kv: hook Raft StoreLiveness into storeliveness package #126533
Conversation
![blathers[crl]](https://avatars.githubusercontent.com/in/59700?s=40&u=373d9101a12edc3847d2fb6ae78e3adcfb4ca41e&v=4){kind=avatar}
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @arulajmani and @nvanbenschoten)
pkg/kv/kvserver/replica_store_liveness.go line 73 at r1 (raw file):
// SupportInPast implements the raftstoreliveness.StoreLiveness interface. func (r *replicaRLockedStoreLiveness) SupportInPast(ts hlc.Timestamp) bool {
The name of this method was a bit confusing until I saw what it actually does. The name on its own seems to imply (to me at least) that it's checking for support at some past timestamp from a history of provided support.
Where will we use this? Anywhere we check if support is still provided? Maybe SupportExpired fits better?
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:
nvanbenschoten
force-pushed
the
nvanbenschoten/storeLivenessHook
branch
from
59a6fe4 to
e1a7df6
Compare
There was a problem hiding this comment.
TFTRs!
bors r+
Reviewable status:
pkg/kv/kvserver/replica_store_liveness.go line 73 at r1 (raw file):
I like SupportExpired. I think it's actually what we had until I convinced @arulajmani to switch to SupportInPast. Switched back here and in the raftstoreliveness.StoreLiveness interface.
Where will we use this? Anywhere we check if support is still provided?
We will use this when determining whether we should step down as the Raft leader. If support is in the past / expired, we step down.
Fixes cockroachdb#125242. This commit adds a `replicaRLockedStoreLiveness` adapter type to hook the raft store liveness into the storeliveness package. This is currently unused. Release note: None
nvanbenschoten
force-pushed
the
nvanbenschoten/storeLivenessHook
branch
from
e1a7df6 to
393d1ee
Compare
|
bors r+ |
Fixes #125242.
This commit adds a
replicaRLockedStoreLivenessadapter type to hook the raft store liveness into the storeliveness package.This is currently unused.
Release note: None