kvfollowerreadsccl: deflake TestBoundedStalenessDataDriven #155702
Merged
Conversation
This file contains hidden or 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
Member
|
This change is |
stevendanna
force-pushed
the
ssd/deflake-TestBoundedStalenessDataDriven
branch
from
138ee5e to
6b3786d
Compare
Collaborator
Author
|
Maybe this will help us get through extended CI: #154939 |
When using tenants, this test appears more prone to flakes. The flakes seem to come from two sources: 1. A lease would move off of node 1 and thus we'd see a "local read" succeed. Here we try to make this less likely by turning off the lease, replicate, and split queues and adding an early assertion that all of the leaseholders live on n1. 2. The schema change the test performs would take > 10s, which then invalidates many later assumptions that 10s in the past should be strictly before the schema change. We've kicked the can here and bumped it to 20s. Unfortunately, this still doesn't seem to be enough as we still occasionally see this fail. One note is that this test is intended to be skipped under stress, but in CI it is still run under stress because of changes to the meaning of skip.UnderStress. Informs cockroachdb#154710 Release note: None
stevendanna
force-pushed
the
ssd/deflake-TestBoundedStalenessDataDriven
branch
from
6b3786d to
228c97d
Compare
arulajmani
approved these changes
Oct 27, 2025
Collaborator
arulajmani
left a comment
arulajmani
left a comment
There was a problem hiding this comment.
@arulajmani reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @stevendanna)
Collaborator
Author
|
bors r=arulajmani |
Contributor
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
When using tenants, this test appears more prone to flakes. The flakes seem to come from two sources:
When using tenants, this test appears more prone to flakes. The flakes
seem to come from two sources:
A lease would move off of node 1 and thus we'd see a
"local read" succeed. Here we try to make this less likely by turning
off the lease, replicate, and split queues and adding an early
assertion that all of the leaseholders live on n1.
The schema change the test performs would take > 10s, which then
invalidates many later assumptions that 10s in the past should be
strictly before the schema change. We've kicked the can here and
bumped it to 20s.
Unfortunately, this still doesn't seem to be enough as we still
occasionally see this fail. One note is that this test is intended to be
skipped under stress, but in CI it is still run under stress because of
changes to the meaning of skip.UnderStress.
Informs #154710
Release note: None