ccl/multiregionccl: deflake multi-region datadriven test#109819
Conversation
Previously, `TestMultiRegionDataDriven` allowed kvserver (replicate queue) lease transfers to occur for lease count rebalancing. Load based rebalancing was disabled in an earlier patch cockroachdb#109050, however we saw in interleaving replication changes -- leading to flaky behavior. Bump the `kv.allocator.min_lease_transfer_interval` to 5 minutes, effectively disabling internal lease transfers. Lease transfers which are required for lease preference satisfaction, and manual transfers are still permitted. Fixes: cockroachdb#109215 Release note: None
|
This change is |
rafiss
left a comment
There was a problem hiding this comment.
nice improvements, ty
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @kvoli)
pkg/ccl/multiregionccl/datadriven_test.go line 117 at r2 (raw file):
skip.UnderRace(t, "flaky test") skip.UnderDeadlock(t, "flaky test")
super nit: the commit message has a nice explanation. that (or a summary) could be included here in the code so that a future reader can more easily understand why it's legitimate to skip under deadlock
`TestMultiRegionDataDriven` speeds up replication changes by proactively enqueuing replicas into various queues. This has the benefit of reducing the time taken after zone config changes, however the downside of added overhead. Disable the test under deadlock builds, as the test is slow and susceptible to (legitimate) timing issues on a deadlock build. Informs: cockroachdb#109215 Release note: None
c156fb7 to
374ab7b
Compare
kvoli
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/ccl/multiregionccl/datadriven_test.go line 117 at r2 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
super nit: the commit message has a nice explanation. that (or a summary) could be included here in the code so that a future reader can more easily understand why it's legitimate to skip under deadlock
Good idea, I've added a slightly modified commit message summary here.
|
TYFTR bors r=rafiss |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 374ab7b to blathers/backport-release-22.2-109819: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.2.x failed. See errors above. error creating merge commit from 374ab7b to blathers/backport-release-23.1-109819: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 23.1.x failed. See errors above. error creating merge commit from 374ab7b to blathers/backport-release-23.1.9-rc-109819: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 23.1.9-rc failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Previously,
TestMultiRegionDataDrivenallowed kvserver (replicate queue) lease transfers to occur for lease count rebalancing. Load based rebalancing was disabled in an earlier patch #109050, however we saw in interleaving replication changes -- leading to flaky behavior.Bump the
kv.allocator.min_lease_transfer_intervalto 5 minutes, effectively disabling internal lease transfers. Lease transfers which are required for lease preference satisfaction, and manual transfers are still permitted.Fixes: #109215
Release note: None
TestMultiRegionDataDrivenspeeds up replication changes by proactivelyenqueuing replicas into various queues. This has the benefit of reducing
the time taken after zone config changes, however the downside of added
overhead.
Disable the test under deadlock builds, as the test is slow and
susceptible to (legitimate) timing issues on a deadlock build.
Informs: #109215
Release note: None