-
Notifications
You must be signed in to change notification settings - Fork 3.9k
sql: deflake TestDropDatabaseDeleteData #140960
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
Conversation
This commit deflakes the test by blocking the full reconciliation from starting before we drop the database. This is used to avoid a race condition where the full reconciliation starts after we drop the database, it will ignore the dropped database. Then, there is a race between the SQLWatcher and the GC job where the SQLWatcher might write a zone config of table1 before table2, while the GC queue might not know that this range needs to be split because it has different span config. Release note: None Fixes: cockroachdb#138185
@arulajmani @rafiss for context, this PR fixes the second issue in the test where table2 is GCed with table1, even though we only set the TTL=0 for table1. This seems to happen if:
At (5), the GC queue might not know about table2's zone config, and might delete both table1 and table2. However, if we swap (1) and (2), it won't delete table2's data with table1. |
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 3 of 3 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @rafiss)
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! thanks for the detailed analysis
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @iskettaneh)
TFTR! bors r+ |
Build failed (retrying...):
|
This commit deflakes the test by blocking the full reconciliation from starting before we drop the database. This is used to avoid a race condition where the full reconciliation starts after we drop the database, it will ignore the dropped database. Then, there is a race between the SQLWatcher and the GC job where the SQLWatcher might write a zone config of table1 before table2, while the GC queue might not know that this range needs to be split because it has different span config.
Release note: None
Fixes: #138185