multiregionccl: deflake the multi-region cold start test #103666
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
|
This change is |
jeffswenson
force-pushed
the
jeffswenson-fix-cold-start-latency-test
branch
2 times, most recently
from
01a99cf to
34a125b
Compare
jeffswenson
force-pushed
the
jeffswenson-fix-cold-start-latency-test
branch
2 times, most recently
from
62c1a72 to
9139b57
Compare
jeffswenson
force-pushed
the
jeffswenson-fix-cold-start-latency-test
branch
from
9139b57 to
04ab2fc
Compare
arulajmani
approved these changes
May 19, 2023
There was a problem hiding this comment.
Nice job piecing together the all the history here and deflaking this!
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @jeffswenson and @rafiss)
-- commits line 25 at r1:
nit: "and are not required to achieve conformance as we proactively nudge replicas through the replicate queue after ... "
jeffswenson
force-pushed
the
jeffswenson-fix-cold-start-latency-test
branch
3 times, most recently
from
655c6b8 to
859b6dd
Compare
I ran the test 2500 times with no flakes. The core source of the flakes is the test depends on every range having an available replica in every region. Any amount of replica movement causes the test to fail because many regions only have a single replica and temporary unavailability causes the sql servers to pick a replica that is available, but unreachable due to a networking hook injected by the test. There are a few independent changes in this PR: 1. It includes the settings from cockroachdb#100320. The lease transfer setting improves test performance and disabling load based splitting helps avoid replica migration. 2. Based on the insight from cockroachdb#96334#issuecomment-1492610753 the test checks the span conformance report for all servers instead of only one of the servers. 3. The test no longer triggers scatters while waiting for the span config bounds to conform. The scatters increased test flakiness and are not required to achieve conformance as we proactively nudge replicas through the replicate queue after cockroachdb#100349. 4. WaitForFullReplication is called to ensure replication queues are empty and there are no pending range moves. Fixes cockroachdb#96334 Release Note: None
jeffswenson
force-pushed
the
jeffswenson-fix-cold-start-latency-test
branch
from
859b6dd to
2cbd35e
Compare
|
bors r+ |
|
Thanks for the review! |
|
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.
I ran the test 2500 times with no flakes. The core source of the flakes
is the test depends on every range having an available replica in every
region. Any amount of replica movement causes the test to fail because
many regions only have a single replica and temporary unavailability
causes the sql servers to pick a replica that is available, but
unreachable due to a networking hook injected by the test.
There are a few independent changes in this PR:
improves test performance and disabling load based splitting helps
avoid replica migration.
checks the span conformance report for all servers instead of only
one of the servers.
config bounds to conform. The scatters increased test flakiness
and are not required to achieve conformance as we proactively nudge
replicas through the replicate queue after kvserver: nudge replicate queue on span config update #100349.
empty and there are no pending range moves.
Fixes #96334
Release Note: None