-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
spanner: canceled context is used to record stats #2660
Comments
@olavloite, if you could also take a look at this, that would be great. CC @hengfengli |
The cancel() method should be called once all code that is executing in the given context have finished. In this case, that also means after the stats recording that uses the same context has finished. Fixes #2660
@nodirg Thank you for your report. In this specific case it is as far as I can tell not really a problem, but I do agree with you that it looks a little bit suspicious. The reason that it is not really a problem in this case, is that the context is only used to include the tags that should be included with the statistics that are recorded. |
The cancel() method should be called once all code that is executing in the given context have finished. In this case, that also means after the stats recording that uses the same context has finished. Fixes #2660
…pis#2728) The cancel() method should be called once all code that is executing in the given context have finished. In this case, that also means after the stats recording that uses the same context has finished. Fixes googleapis#2660
I have been reading Spanner client code and stumbled on this:
google-cloud-go/spanner/session.go
Lines 1502 to 1517 in 626dcfc
note here that
ctx
is canceled in L1504 and then that context is used in L1517.I don't have concrete problem that this caused, but this code does not look correct. Perhaps
ctx
variable in L1502 should be renamedThe text was updated successfully, but these errors were encountered: