-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
execstats: fix flakey TestTraceAnalyzer #75632
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
With the spanconfig job running in the background, and parts of it using the internal executor, it is incorrect for this test to reach into SetSessionData. This method has a comment indicating that it is not to be used concurrently with query execution. The fix is to create a session bound internal executor in the test. Fixes: cockroachdb#75546 Release note: None
adityamaru
force-pushed
the
fix-data-race-75546
branch
from
January 28, 2022 14:24
e135081
to
16bf5d9
Compare
adityamaru
changed the title
spanconfigsqltranslator: fix data race when using pts provider
execstats: fix flakey TestTraceAnalyzer @adityamaru
Jan 28, 2022
adityamaru
changed the title
execstats: fix flakey TestTraceAnalyzer @adityamaru
execstats: fix flakey TestTraceAnalyzer
Jan 28, 2022
yuzefovich
approved these changes
Jan 28, 2022
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.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @irfansharif)
bors r=yuzefovich |
craig bot
pushed a commit
that referenced
this pull request
Jan 28, 2022
70159: kvclient: switch error in ctx cancel edge case r=andreimatei a=andreimatei This patch changes the error returned by the DistSender on a cancelled ctx. Depending on the exactly who detects the ctx as cancelled, there are a number of different possibilities - too many to enumerate here. Generally, the client is supposed to get a context.Canceled error, wrapped in different layers. The code path that this patch changes is about the cancellation being detected by sendPartialBatch() without it having been previously detected by sendToReplicas(). This path is unusual (sendToReplicas() generally detects the ctx cancelled). Depending on the exact cancellation timing, it's possible though for sendPartialBatch() to detect it instead. In this case, this patch makes it so that, if sendToReplicas() returned a sendError (indicating that a suitable replica could not be reached and that the higher layer is expected to continue trying other replicas), the error returned to the client is a cancellation error instead of the sendError. Touches #69419 Release note: None 75632: execstats: fix flakey TestTraceAnalyzer r=yuzefovich a=adityamaru Fixes: #75546 Release note: None Co-authored-by: Andrei Matei <[email protected]> Co-authored-by: Aditya Maru <[email protected]>
Build failed (retrying...): |
bors r+ |
Already running a review |
Build failed (retrying...): |
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #75546
Release note: None