Skip to content

Comments

kvserver: use leader leases in various flow control tests#136182

Merged
craig[bot] merged 6 commits intocockroachdb:masterfrom
arulajmani:ll-TestFlowControlCrashedNode
Nov 26, 2024
Merged

kvserver: use leader leases in various flow control tests#136182
craig[bot] merged 6 commits intocockroachdb:masterfrom
arulajmani:ll-TestFlowControlCrashedNode

Conversation

@arulajmani
Copy link
Collaborator

See individual commits for details.

This test sets a very large number for RaftElectionTimeoutTicks to
keep leadership sticky. Naively, this prevents us from using leader
leases in this test, as replicas won't campaign if they don't have
StoreLiveness support, which isn't kicked off until the first tick.
Conveniently, raft fortification is yet another way to keep leadership
sticky -- so explicitly using leader leases should achieve the same
goal.

References cockroachdb#133763

Release note: None
This test sets a very large number for RaftElectionTimeoutTicks to
keep leadership sticky. Naively, this prevents us from using leader
leases in this test, as replicas won't campaign if they don't have
StoreLiveness support, which isn't kicked off until the first tick.
Conveniently, raft fortification is yet another way to keep leadership
sticky -- so explicitly using leader leases should achieve the same
goal.

References cockroachdb#133763

Release note: None
This test sets a very large number for RaftElectionTimeoutTicks to
keep leadership sticky. Naively, this prevents us from using leader
leases in this test, as replicas won't campaign if they don't have
StoreLiveness support, which isn't kicked off until the first tick.
Conveniently, raft fortification is yet another way to keep leadership
sticky -- so explicitly using leader leases should achieve the same
goal.

References cockroachdb#133763

Release note: None
This test sets a very large number for RaftElectionTimeoutTicks to
keep leadership sticky. Naively, this prevents us from using leader
leases in this test, as replicas won't campaign if they don't have
StoreLiveness support, which isn't kicked off until the first tick.
Conveniently, raft fortification is yet another way to keep leadership
sticky -- so explicitly using leader leases should achieve the same
goal.

References cockroachdb#133763
Release note: None
We'll soon enable leader leases metamorphically. There's a few tests
that  want to always use a lease type that allows for quiescence --
so instead of disabling expiration based leases, they should instead
explicitly set its lease type to be epoch based.

The tests are:
- TestFlowControlUnquiescedRangeV2
- TestFlowControlUnquiescedRange
- TestFlowControlQuiescedRange

References cockroachdb#133763
Release note: None
Much like the previous commits, a bunch of other tests were setting
a very large number for RaftElectionTimeoutTicks to keep leadership
sticky. Naively, this prevents us from using leader leases in these
tests, as replicas won't campaign unless they have StoreLiveness
support, which they won't until the first tick kicks off a request.
Conveniently, raft fortification is yet another way to keep leadership
sticky -- so explicitly using leader leases should achieve the same
goal.

The tests modified here are:
- TestFlowControlSendQueue
- TestFlowControlRepeatedlySwitchMode
- TestFlowControlRaftSnapshot
- TestFlowControlRaftTransportBreak
- TestFlowControlAdmissionPostSplitMergeV2
- TestFlowControlV1ToV2Transition

References cockroachdb#133763
Release note: None
@arulajmani arulajmani requested a review from a team as a code owner November 26, 2024 02:44
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@arulajmani
Copy link
Collaborator Author

At one point, I got tired of pulling things out into their own commits and decided to batch. I was too lazy to go back and batch the first few commits, so unless you (@kvoli, @iskettaneh) care about the lack of uniformity, here we are.

Copy link
Contributor

@kvoli kvoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No testing diffs in .../kvserver/testdata/flow_control_integration_v2/* + passing tests is good indication that these changes had little discernible impact on rac2.

:lgtm:

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @iskettaneh)

@arulajmani
Copy link
Collaborator Author

TFTR!

bors r=kvoli

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants