base: decrease store liveness durations #137608
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 11 of 11 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @miraradeva and @nvanbenschoten)
pkg/kv/kvserver/storeliveness/requester_state.go line 336 at r1 (raw file):
// updateMaxRequested forwards the current MaxRequested timestamp to now + // interval, where now is the node's clock timestamp and support duration is the
nit: interval is the support duration or, better yet, "now + support duration, where now is the node's clock timestamp".
miraradeva
force-pushed
the
mira-133613-tune
branch
from
3b35d19 to
d4244b9
Compare
There was a problem hiding this comment.
Reviewed 10 of 11 files at r1, 4 of 4 files at r3, 3 of 3 files at r4, all commit messages.
Reviewable status:
pkg/base/testdata/raft_config_recovery line 3 at r4 (raw file):
# Any changes in this result should be copied to the comment on # defaultRangeLeaseRaftElectionTimeoutMultiplier, and the corresponding # reasoning should be adjusted.
We'll want to update the comment that this is referencing as well.
miraradeva
force-pushed
the
mira-133613-tune
branch
from
d4244b9 to
78902a2
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/base/testdata/raft_config_recovery line 3 at r4 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
We'll want to update the comment that this is referencing as well.
Looks like defaultRangeLeaseRaftElectionTimeoutMultiplier doesn't exist as of 40cb075, so I updated the comment to point to the right place (which is defaultRangeLeaseDuration). I had already updated that one.
|
The CI failure ( |
miraradeva
force-pushed
the
mira-133613-tune
branch
from
78902a2 to
4af056e
Compare
|
The CI extended failure is |
The liveness interval indicates the length of each period of requested (or extended) support in store liveness. It is not really an interval as much as a duration. This commit renames "liveness interval" to "support duration", which also aligns with the notion of a lease duration. Part of: cockroachdb#133613 Release note: None
Previously, store liveness used a heartbeat interval and support duration of 3s and 6s, respectively. This matched the lease extension and lease duration, respectively. However, these values were not well aligned with Raft's election timeout (4s) and jitter (up to 2s), so when a follower had to campaign after withdrawing support from the leader, the store liveness durations added up to the Raft timeout and jitter, instead of being subsumed by them. This commit reduces the store liveness heartbeat interval and support duration to 1s and 3s, respectively. Fixes: cockroachdb#133613 Release note: None
miraradeva
force-pushed
the
mira-133613-tune
branch
from
4af056e to
ffa4eb1
Compare
|
CI is finally green. TFTRs! bors r+ |
137608: base: decrease store liveness durations r=miraradeva a=miraradeva Previously, store liveness used a heartbeat interval and support duration of 3s and 6s, respectively. This matched the lease extension and lease duration, respectively. However, these values were not well aligned with Raft's election timeout (4s) and jitter (up to 2s), so when a follower had to campaign after withdrawing support from the leader, the store liveness durations added up to the Raft timeout and jitter, instead of being subsumed by them. This commit reduces the store liveness heartbeat interval and support duration to 1s and 3s, respectively. Fixes: [#133613](#133613) Release note: None 137845: diagnostics: honor stopper quiescence when reporting diagnostics r=xinhaoz a=dhartunian Previously, we would block on the HTTP diagnostics request until the preset timeout regardless of stopper quiescence. This change ensures that the `ctx` passed to the diagnostics reporter and the HTTP client is cancelled if the stopper is quiescing. Resolves: #136739 Release note: None Co-authored-by: Mira Radeva <[email protected]> Co-authored-by: David Hartunian <[email protected]>
|
This PR was included in a batch that was canceled, it will be automatically retried |
Previously, store liveness used a heartbeat interval and support duration of 3s and 6s, respectively. This matched the lease extension and lease duration, respectively. However, these values were not well aligned with Raft's election timeout (4s) and jitter (up to 2s), so when a follower had to campaign after withdrawing support from the leader, the store liveness durations added up to the Raft timeout and jitter, instead of being subsumed by them.
This commit reduces the store liveness heartbeat interval and support duration to 1s and 3s, respectively.
Fixes: #133613
Release note: None