Skip to content

release-23.1: ccl/multiregionccl: disable transfers in multiregion test#110052

Merged
kvoli merged 2 commits intocockroachdb:release-23.1from
kvoli:backport23.1-109819
Sep 6, 2023
Merged

release-23.1: ccl/multiregionccl: disable transfers in multiregion test#110052
kvoli merged 2 commits intocockroachdb:release-23.1from
kvoli:backport23.1-109819

Conversation

@kvoli
Copy link
Copy Markdown
Contributor

@kvoli kvoli commented Sep 5, 2023

Backport 2/2 commits from #109819.

/cc @cockroachdb/release


Previously, TestMultiRegionDataDriven allowed 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_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: #109215
Release note: None


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: #109215
Release note: None


Release justification: Test only change.

@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Sep 5, 2023

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Patches should only be created for serious issues or test-only changes.
  • Patches should not break backwards-compatibility.
  • Patches should change as little code as possible.
  • Patches should not change on-disk formats or node communication protocols.
  • Patches should not add new functionality.
  • Patches must not add, edit, or otherwise modify cluster versions; or add version gates.
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

  • What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
  • Will this work in a cluster of mixed patch versions? Did we test that?
  • If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

@blathers-crl blathers-crl bot added the backport Label PR's that are backports to older release branches label Sep 5, 2023
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@kvoli kvoli requested a review from rafiss September 5, 2023 18:51
@kvoli kvoli marked this pull request as ready for review September 5, 2023 18:51
@kvoli kvoli requested a review from a team as a code owner September 5, 2023 18:51
@kvoli kvoli self-assigned this Sep 5, 2023
@kvoli kvoli removed the request for review from rafiss September 5, 2023 19:00
@kvoli kvoli marked this pull request as draft September 5, 2023 19:00
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
`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
@kvoli kvoli force-pushed the backport23.1-109819 branch from afe873f to 0afcba0 Compare September 5, 2023 19:28
@kvoli
Copy link
Copy Markdown
Contributor Author

kvoli commented Sep 5, 2023

global_tables is still flaky on 23.1 -- I can reproduce the failure in about 1k runs (20-30 mins).

@kvoli kvoli marked this pull request as ready for review September 6, 2023 15:17
@kvoli kvoli requested a review from rafiss September 6, 2023 15:17
@kvoli
Copy link
Copy Markdown
Contributor Author

kvoli commented Sep 6, 2023

TYFTR

@kvoli kvoli merged commit 344f994 into cockroachdb:release-23.1 Sep 6, 2023
kvoli added a commit to kvoli/cockroach that referenced this pull request Sep 6, 2023
Update the cluster setting statement to correctly surround the value
duration in quotations.`5m` to `'5m'`. This was fixed in backports to
release-23.1 (cockroachdb#110052) and release-22.2 (cockroachdb#110053). However, this was not
caught on master due to `TestMultiRegionDataDriven` being skipped.

Informs: cockroachdb#98020
Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Label PR's that are backports to older release branches v23.1.12

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants