-
Notifications
You must be signed in to change notification settings - Fork 4.1k
sql: deflake TestAbortedTxnLocks #160589
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
sql: deflake TestAbortedTxnLocks #160589
Conversation
5a1c5a7 enabled multitenant testing for this test. One of the subtests is flaky on secondary tenants, so we skip it in that mode. Release note: None
spilchen
left a comment
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.
@spilchen reviewed all commit messages and made 2 comments.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @fqazi and @rafiss).
-- commits line 4 at r1:
nit: this may be slightly misleading? AFAIK, we already ran the test with multitenant, since it didn't use the flag to disable it. This change looks like we are just skipping one test that has proven to be flaky.
rafiss
left a comment
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.
@rafiss made 1 comment.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @fqazi and @spilchen).
Previously, spilchen wrote…
nit: this may be slightly misleading? AFAIK, we already ran the test with multitenant, since it didn't use the flag to disable it. This change looks like we are just skipping one test that has proven to be flaky.
I may not follow your question. 5a1c5a7 was recently merged and removed the flag that disables multitenant testing for this test.
now we are skipping one of the subtests that became flaky after that.
spilchen
left a comment
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.
@spilchen made 1 comment and resolved 1 discussion.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @fqazi).
Previously, rafiss (Rafi Shamim) wrote…
I may not follow your question. 5a1c5a7 was recently merged and removed the flag that disables multitenant testing for this test.
now we are skipping one of the subtests that became flaky after that.
ack, I read this too quick and misunderstood it. I thought this was the complete fix and was looking for where the enablement happened. I understand now. You can disregard this comment.
|
TFTR! bors r+ |
160408: rttanalysis: deflake pause/cancel job benchmark r=jeffswenson a=msbutler This patch changes the benchmark to pause/cancel jobs which are guaranteed to be pausable/cancellable. While here, I added rows to the job_progress and job_status tables. Fixes #157116 Release note: none 160416: restore: remove incremental_location from SQL read syntaxes r=msbutler a=kev-cao This patch removes support for the `incremental_location` syntax from our read paths. Resolves: #159172 Release note (backwards-incompatible change): The `incremental_location` option has been removed from `SHOW BACKUP` and `RESTORE`. 160589: sql: deflake TestAbortedTxnLocks r=rafiss a=rafiss 5a1c5a7 enabled multitenant testing for this test. One of the subtests is flaky on secondary tenants, so we skip it in that mode. fixes #160585 Release note: None Co-authored-by: Michael Butler <[email protected]> Co-authored-by: Kevin Cao <[email protected]> Co-authored-by: Rafi Shamim <[email protected]>
|
This PR was included in a batch that successfully built, but then failed to merge into master (it was a non-fast-forward update). It will be automatically retried. |
|
Build succeeded: |
5a1c5a7 enabled multitenant testing for this test. One of the subtests is flaky on secondary tenants, so we skip it in that mode.
fixes #160585
Release note: None