-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rework timeout chain element #520
Rework timeout chain element #520
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general looks good
6510d77
to
37f6aed
Compare
Issueall values from the current context are copied including time.AfterFunc(duration, func() {
newCtx := extend.WithValuesFromContext(context.Background(), ctx)
if _, err := (*t.onTimeout).Close(newCtx, conn); err != nil {
trace.Log(newCtx).Errorf("Error attempting to close timed out connection: %s: %+v", request.GetConnection().GetId(), err)
}
} for the Close chain it looks like it was injected between the timeout and the after-timeout chain elements if n.index == 0 && ctx != nil {
if np := Server(ctx); np != nil {
nextParent = np
}
} Solutiontime.AfterFunc(duration, func() {
newCtx := extend.WithValuesFromContext(context.Background(), ctx)
closeServer := chain.NewNetworkServiceServer(*t.onTimeout, next.TailServer())
if _, err := closeServer.Close(newCtx, conn); err != nil {
trace.Log(newCtx).Errorf("Error attempting to close timed out connection: %s: %+v", conn.GetId(), err)
}
}) |
d3398dc
to
545e030
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is the wrong way for chains.
We should not add new kinds of chains like a breakchain that means a chain that doesn't call parent chains... I feel it will make the system more complex.
Let's just rework timeout with the next approach:
package timeout
// Timeout just closes all next elements from the context on timeout
func NewServer() {
}
I think this may be simpler: #547 |
|
Signed-off-by: Vladimir Popov <[email protected]>
Signed-off-by: Vladimir Popov <[email protected]>
Signed-off-by: Vladimir Popov <[email protected]>
Signed-off-by: Vladimir Popov <[email protected]>
Signed-off-by: Vladimir Popov <[email protected]>
Signed-off-by: Vladimir Popov <[email protected]>
Signed-off-by: Vladimir Popov <[email protected]>
Signed-off-by: Vladimir Popov <[email protected]>
Signed-off-by: Vladimir Popov <[email protected]>
Signed-off-by: Vladimir Popov <[email protected]>
Signed-off-by: Vladimir Popov <[email protected]>
Signed-off-by: Vladimir Popov <[email protected]>
Signed-off-by: Vladimir Popov <[email protected]>
Signed-off-by: Vladimir Popov <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@edwarnicke Do you have any comments on this PR?
@edwarnicke Let us know if you have any comments on this then we'll fix them in separate PRs |
…k@master networkservicemesh/sdk#520 networkservicemesh/sdk PR link: networkservicemesh/sdk#520 networkservicemesh/sdk commit message: commit 3905c3e75ef750222630040f18c06eb683568c75 Author: Vladimir Popov <[email protected]> Date: Tue Nov 3 14:07:14 2020 +0700 Rework timeout chain element (#520) * Fix issues with timeout server Signed-off-by: Vladimir Popov <[email protected]> * Fix issue with invalid chain element close Signed-off-by: Vladimir Popov <[email protected]> * Add chainbreak Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout_test Signed-off-by: Vladimir Popov <[email protected]> * Replace chain with next Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout to work correct with no server passed Signed-off-by: Vladimir Popov <[email protected]> * Timeout shoult close only subsequent chain elements Signed-off-by: Vladimir Popov <[email protected]> * Revert non-need changes Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with mapexecutor Signed-off-by: Vladimir Popov <[email protected]> * Replace map with sync.Map in timeout Signed-off-by: Vladimir Popov <[email protected]> * Rework multiexecutor Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with executor map Signed-off-by: Vladimir Popov <[email protected]> * Fix issues Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout Close 'next error' issue, add comments Signed-off-by: Vladimir Popov <[email protected]> * Add timeout/DESIGN.md Signed-off-by: Vladimir Popov <[email protected]> * Rename DESIGN.md to README.md Signed-off-by: Vladimir Popov <[email protected]> * Add test for timer.closeCh Signed-off-by: Vladimir Popov <[email protected]> * Fix double Close issue, add stress test Signed-off-by: Vladimir Popov <[email protected]> * Upgrade go-syncmap Signed-off-by: Vladimir Popov <[email protected]> Signed-off-by: NSMBot <[email protected]>
…k@master networkservicemesh/sdk#520 networkservicemesh/sdk PR link: networkservicemesh/sdk#520 networkservicemesh/sdk commit message: commit 3905c3e75ef750222630040f18c06eb683568c75 Author: Vladimir Popov <[email protected]> Date: Tue Nov 3 14:07:14 2020 +0700 Rework timeout chain element (#520) * Fix issues with timeout server Signed-off-by: Vladimir Popov <[email protected]> * Fix issue with invalid chain element close Signed-off-by: Vladimir Popov <[email protected]> * Add chainbreak Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout_test Signed-off-by: Vladimir Popov <[email protected]> * Replace chain with next Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout to work correct with no server passed Signed-off-by: Vladimir Popov <[email protected]> * Timeout shoult close only subsequent chain elements Signed-off-by: Vladimir Popov <[email protected]> * Revert non-need changes Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with mapexecutor Signed-off-by: Vladimir Popov <[email protected]> * Replace map with sync.Map in timeout Signed-off-by: Vladimir Popov <[email protected]> * Rework multiexecutor Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with executor map Signed-off-by: Vladimir Popov <[email protected]> * Fix issues Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout Close 'next error' issue, add comments Signed-off-by: Vladimir Popov <[email protected]> * Add timeout/DESIGN.md Signed-off-by: Vladimir Popov <[email protected]> * Rename DESIGN.md to README.md Signed-off-by: Vladimir Popov <[email protected]> * Add test for timer.closeCh Signed-off-by: Vladimir Popov <[email protected]> * Fix double Close issue, add stress test Signed-off-by: Vladimir Popov <[email protected]> * Upgrade go-syncmap Signed-off-by: Vladimir Popov <[email protected]> Signed-off-by: NSMBot <[email protected]>
…k@master networkservicemesh/sdk#520 networkservicemesh/sdk PR link: networkservicemesh/sdk#520 networkservicemesh/sdk commit message: commit 3905c3e75ef750222630040f18c06eb683568c75 Author: Vladimir Popov <[email protected]> Date: Tue Nov 3 14:07:14 2020 +0700 Rework timeout chain element (#520) * Fix issues with timeout server Signed-off-by: Vladimir Popov <[email protected]> * Fix issue with invalid chain element close Signed-off-by: Vladimir Popov <[email protected]> * Add chainbreak Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout_test Signed-off-by: Vladimir Popov <[email protected]> * Replace chain with next Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout to work correct with no server passed Signed-off-by: Vladimir Popov <[email protected]> * Timeout shoult close only subsequent chain elements Signed-off-by: Vladimir Popov <[email protected]> * Revert non-need changes Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with mapexecutor Signed-off-by: Vladimir Popov <[email protected]> * Replace map with sync.Map in timeout Signed-off-by: Vladimir Popov <[email protected]> * Rework multiexecutor Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with executor map Signed-off-by: Vladimir Popov <[email protected]> * Fix issues Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout Close 'next error' issue, add comments Signed-off-by: Vladimir Popov <[email protected]> * Add timeout/DESIGN.md Signed-off-by: Vladimir Popov <[email protected]> * Rename DESIGN.md to README.md Signed-off-by: Vladimir Popov <[email protected]> * Add test for timer.closeCh Signed-off-by: Vladimir Popov <[email protected]> * Fix double Close issue, add stress test Signed-off-by: Vladimir Popov <[email protected]> * Upgrade go-syncmap Signed-off-by: Vladimir Popov <[email protected]> Signed-off-by: NSMBot <[email protected]>
…k@master networkservicemesh/sdk#520 networkservicemesh/sdk PR link: networkservicemesh/sdk#520 networkservicemesh/sdk commit message: commit 3905c3e75ef750222630040f18c06eb683568c75 Author: Vladimir Popov <[email protected]> Date: Tue Nov 3 14:07:14 2020 +0700 Rework timeout chain element (#520) * Fix issues with timeout server Signed-off-by: Vladimir Popov <[email protected]> * Fix issue with invalid chain element close Signed-off-by: Vladimir Popov <[email protected]> * Add chainbreak Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout_test Signed-off-by: Vladimir Popov <[email protected]> * Replace chain with next Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout to work correct with no server passed Signed-off-by: Vladimir Popov <[email protected]> * Timeout shoult close only subsequent chain elements Signed-off-by: Vladimir Popov <[email protected]> * Revert non-need changes Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with mapexecutor Signed-off-by: Vladimir Popov <[email protected]> * Replace map with sync.Map in timeout Signed-off-by: Vladimir Popov <[email protected]> * Rework multiexecutor Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with executor map Signed-off-by: Vladimir Popov <[email protected]> * Fix issues Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout Close 'next error' issue, add comments Signed-off-by: Vladimir Popov <[email protected]> * Add timeout/DESIGN.md Signed-off-by: Vladimir Popov <[email protected]> * Rename DESIGN.md to README.md Signed-off-by: Vladimir Popov <[email protected]> * Add test for timer.closeCh Signed-off-by: Vladimir Popov <[email protected]> * Fix double Close issue, add stress test Signed-off-by: Vladimir Popov <[email protected]> * Upgrade go-syncmap Signed-off-by: Vladimir Popov <[email protected]> Signed-off-by: NSMBot <[email protected]>
…k@master networkservicemesh/sdk#520 networkservicemesh/sdk PR link: networkservicemesh/sdk#520 networkservicemesh/sdk commit message: commit 3905c3e75ef750222630040f18c06eb683568c75 Author: Vladimir Popov <[email protected]> Date: Tue Nov 3 14:07:14 2020 +0700 Rework timeout chain element (#520) * Fix issues with timeout server Signed-off-by: Vladimir Popov <[email protected]> * Fix issue with invalid chain element close Signed-off-by: Vladimir Popov <[email protected]> * Add chainbreak Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout_test Signed-off-by: Vladimir Popov <[email protected]> * Replace chain with next Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout to work correct with no server passed Signed-off-by: Vladimir Popov <[email protected]> * Timeout shoult close only subsequent chain elements Signed-off-by: Vladimir Popov <[email protected]> * Revert non-need changes Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with mapexecutor Signed-off-by: Vladimir Popov <[email protected]> * Replace map with sync.Map in timeout Signed-off-by: Vladimir Popov <[email protected]> * Rework multiexecutor Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with executor map Signed-off-by: Vladimir Popov <[email protected]> * Fix issues Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout Close 'next error' issue, add comments Signed-off-by: Vladimir Popov <[email protected]> * Add timeout/DESIGN.md Signed-off-by: Vladimir Popov <[email protected]> * Rename DESIGN.md to README.md Signed-off-by: Vladimir Popov <[email protected]> * Add test for timer.closeCh Signed-off-by: Vladimir Popov <[email protected]> * Fix double Close issue, add stress test Signed-off-by: Vladimir Popov <[email protected]> * Upgrade go-syncmap Signed-off-by: Vladimir Popov <[email protected]> Signed-off-by: NSMBot <[email protected]>
…k@master networkservicemesh/sdk#520 networkservicemesh/sdk PR link: networkservicemesh/sdk#520 networkservicemesh/sdk commit message: commit 3905c3e75ef750222630040f18c06eb683568c75 Author: Vladimir Popov <[email protected]> Date: Tue Nov 3 14:07:14 2020 +0700 Rework timeout chain element (#520) * Fix issues with timeout server Signed-off-by: Vladimir Popov <[email protected]> * Fix issue with invalid chain element close Signed-off-by: Vladimir Popov <[email protected]> * Add chainbreak Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout_test Signed-off-by: Vladimir Popov <[email protected]> * Replace chain with next Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout to work correct with no server passed Signed-off-by: Vladimir Popov <[email protected]> * Timeout shoult close only subsequent chain elements Signed-off-by: Vladimir Popov <[email protected]> * Revert non-need changes Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with mapexecutor Signed-off-by: Vladimir Popov <[email protected]> * Replace map with sync.Map in timeout Signed-off-by: Vladimir Popov <[email protected]> * Rework multiexecutor Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with executor map Signed-off-by: Vladimir Popov <[email protected]> * Fix issues Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout Close 'next error' issue, add comments Signed-off-by: Vladimir Popov <[email protected]> * Add timeout/DESIGN.md Signed-off-by: Vladimir Popov <[email protected]> * Rename DESIGN.md to README.md Signed-off-by: Vladimir Popov <[email protected]> * Add test for timer.closeCh Signed-off-by: Vladimir Popov <[email protected]> * Fix double Close issue, add stress test Signed-off-by: Vladimir Popov <[email protected]> * Upgrade go-syncmap Signed-off-by: Vladimir Popov <[email protected]> Signed-off-by: NSMBot <[email protected]>
…k@master networkservicemesh/sdk#520 networkservicemesh/sdk PR link: networkservicemesh/sdk#520 networkservicemesh/sdk commit message: commit 3905c3e75ef750222630040f18c06eb683568c75 Author: Vladimir Popov <[email protected]> Date: Tue Nov 3 14:07:14 2020 +0700 Rework timeout chain element (#520) * Fix issues with timeout server Signed-off-by: Vladimir Popov <[email protected]> * Fix issue with invalid chain element close Signed-off-by: Vladimir Popov <[email protected]> * Add chainbreak Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout_test Signed-off-by: Vladimir Popov <[email protected]> * Replace chain with next Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout to work correct with no server passed Signed-off-by: Vladimir Popov <[email protected]> * Timeout shoult close only subsequent chain elements Signed-off-by: Vladimir Popov <[email protected]> * Revert non-need changes Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with mapexecutor Signed-off-by: Vladimir Popov <[email protected]> * Replace map with sync.Map in timeout Signed-off-by: Vladimir Popov <[email protected]> * Rework multiexecutor Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with executor map Signed-off-by: Vladimir Popov <[email protected]> * Fix issues Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout Close 'next error' issue, add comments Signed-off-by: Vladimir Popov <[email protected]> * Add timeout/DESIGN.md Signed-off-by: Vladimir Popov <[email protected]> * Rename DESIGN.md to README.md Signed-off-by: Vladimir Popov <[email protected]> * Add test for timer.closeCh Signed-off-by: Vladimir Popov <[email protected]> * Fix double Close issue, add stress test Signed-off-by: Vladimir Popov <[email protected]> * Upgrade go-syncmap Signed-off-by: Vladimir Popov <[email protected]> Signed-off-by: NSMBot <[email protected]>
* Fix issues with timeout server Signed-off-by: Vladimir Popov <[email protected]> * Fix issue with invalid chain element close Signed-off-by: Vladimir Popov <[email protected]> * Add chainbreak Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout_test Signed-off-by: Vladimir Popov <[email protected]> * Replace chain with next Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout to work correct with no server passed Signed-off-by: Vladimir Popov <[email protected]> * Timeout shoult close only subsequent chain elements Signed-off-by: Vladimir Popov <[email protected]> * Revert non-need changes Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with mapexecutor Signed-off-by: Vladimir Popov <[email protected]> * Replace map with sync.Map in timeout Signed-off-by: Vladimir Popov <[email protected]> * Rework multiexecutor Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with executor map Signed-off-by: Vladimir Popov <[email protected]> * Fix issues Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout Close 'next error' issue, add comments Signed-off-by: Vladimir Popov <[email protected]> * Add timeout/DESIGN.md Signed-off-by: Vladimir Popov <[email protected]> * Rename DESIGN.md to README.md Signed-off-by: Vladimir Popov <[email protected]> * Add test for timer.closeCh Signed-off-by: Vladimir Popov <[email protected]> * Fix double Close issue, add stress test Signed-off-by: Vladimir Popov <[email protected]> * Upgrade go-syncmap Signed-off-by: Vladimir Popov <[email protected]> Signed-off-by: Sergey Ershov <[email protected]>
* Fix issues with timeout server Signed-off-by: Vladimir Popov <[email protected]> * Fix issue with invalid chain element close Signed-off-by: Vladimir Popov <[email protected]> * Add chainbreak Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout_test Signed-off-by: Vladimir Popov <[email protected]> * Replace chain with next Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout to work correct with no server passed Signed-off-by: Vladimir Popov <[email protected]> * Timeout shoult close only subsequent chain elements Signed-off-by: Vladimir Popov <[email protected]> * Revert non-need changes Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with mapexecutor Signed-off-by: Vladimir Popov <[email protected]> * Replace map with sync.Map in timeout Signed-off-by: Vladimir Popov <[email protected]> * Rework multiexecutor Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with executor map Signed-off-by: Vladimir Popov <[email protected]> * Fix issues Signed-off-by: Vladimir Popov <[email protected]> * Fix timeout Close 'next error' issue, add comments Signed-off-by: Vladimir Popov <[email protected]> * Add timeout/DESIGN.md Signed-off-by: Vladimir Popov <[email protected]> * Rename DESIGN.md to README.md Signed-off-by: Vladimir Popov <[email protected]> * Add test for timer.closeCh Signed-off-by: Vladimir Popov <[email protected]> * Fix double Close issue, add stress test Signed-off-by: Vladimir Popov <[email protected]> * Upgrade go-syncmap Signed-off-by: Vladimir Popov <[email protected]> Signed-off-by: Sergey Ershov <[email protected]>
Timeout chain element use not returned connection value, but incoming - it is wrong because subsequent chain elements can edit connection and fail to close not edited connection.
UPD: #551