-
-
Notifications
You must be signed in to change notification settings - Fork 614
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
go1.21: use context.WithoutCancel to avoid propagating user-initiated cancelations #5506
Comments
When go1.21 or go1.22 is released, we should use the upcoming context.Detach API to accomplish this. |
Note that this impact on connection churn is still hypothesized - we haven't yet been able to prove that this is a cause of connection churn. Also, an update: we now "detach" contexts at the WFE instead of the SA. |
go1.21 is shipping with We may also be able to handle some of our use-cases by using the new |
This new standard library method returns a context with all of the original metadata (e.g. tracing spans) still attached, but which will not be canceled by any cancel funcs, deadlines, or timeouts set on the parent context. We do this manually in a few places to prevent client cancellations (usually disconnects) from disrupting our work, so this just makes that code slightly simpler. Fixes #5506
As of #5459, requests which arrive at the SA have their
ctx
cleared and replaced after the SA starts processing the request. This is to work around an issue where context cancellation (e.g. due to a deadline/timeout coming up, or due to the initiating gRPC request being closed) propagating into sql query cancellation causes sql connections to be marked as bad and dropped from the pool, resulting in connection churn.However, this prevents the context object from carrying trace information as it moves through the SA; the SA cannot add custom trace data or additional spans, because the context object has been cleared of that data.
We should investigate other ways to prevent cancellation of sql queries that would allow the incoming context object to be propagated throughout the SA to a greater degree.
The text was updated successfully, but these errors were encountered: