From dc3316f703972183006e61988c7f8987d09f0c38 Mon Sep 17 00:00:00 2001 From: Andrew Baptist Date: Mon, 16 Dec 2024 13:54:05 -0500 Subject: [PATCH] kvserver: add store membership to the liveness record This commit only changes the protobuf, it does not explicitly use the field anywhere. Since the protobuf now includes a map, it is necessary to use `Equal` rather than == to compare the liveness record to the empty one. Epic: none Release note: None --- pkg/kv/kvserver/leases/build.go | 2 +- pkg/kv/kvserver/leases/status_test.go | 2 +- pkg/kv/kvserver/liveness/cache.go | 2 +- pkg/kv/kvserver/liveness/liveness.go | 6 +++--- pkg/kv/kvserver/liveness/livenesspb/liveness.go | 2 +- pkg/kv/kvserver/liveness/livenesspb/liveness.proto | 4 ++++ pkg/kv/kvserver/replica_rangefeed_test.go | 4 ++-- pkg/testutils/testcluster/testcluster.go | 2 +- 8 files changed, 14 insertions(+), 10 deletions(-) diff --git a/pkg/kv/kvserver/leases/build.go b/pkg/kv/kvserver/leases/build.go index e621ab605f96..37c3e3ee3715 100644 --- a/pkg/kv/kvserver/leases/build.go +++ b/pkg/kv/kvserver/leases/build.go @@ -223,7 +223,7 @@ func (i BuildInput) validate() error { func (i BuildInput) validatePrevLeaseNodeLiveness() error { epochLease := i.PrevLease.Type() == roachpb.LeaseEpoch - livenessSet := i.PrevLeaseNodeLiveness != livenesspb.Liveness{} + livenessSet := !i.PrevLeaseNodeLiveness.Equal(livenesspb.Liveness{}) if epochLease != livenessSet { return errors.AssertionFailedf("previous lease is epoch-based: %t, "+ "but liveness is set: %t", epochLease, livenessSet) diff --git a/pkg/kv/kvserver/leases/status_test.go b/pkg/kv/kvserver/leases/status_test.go index 41f2be2532d9..46f54521a123 100644 --- a/pkg/kv/kvserver/leases/status_test.go +++ b/pkg/kv/kvserver/leases/status_test.go @@ -182,7 +182,7 @@ func TestStatus(t *testing.T) { t.Run("", func(t *testing.T) { nl := &mockNodeLiveness{ record: liveness.Record{Liveness: tc.liveness}, - missing: tc.liveness == livenesspb.Liveness{}, + missing: tc.liveness.Equal(livenesspb.Liveness{}), } in := StatusInput{ LocalStoreID: repl1.StoreID, diff --git a/pkg/kv/kvserver/liveness/cache.go b/pkg/kv/kvserver/liveness/cache.go index f424d0da46a8..4bcdc2f597fa 100644 --- a/pkg/kv/kvserver/liveness/cache.go +++ b/pkg/kv/kvserver/liveness/cache.go @@ -144,7 +144,7 @@ func (c *Cache) storeGossipUpdate(_ string, content roachpb.Value) { // maybeUpdate replaces the liveness (if it appears newer) and invokes the // registered callbacks if the node became live in the process. func (c *Cache) maybeUpdate(ctx context.Context, newLivenessRec Record) { - if newLivenessRec.Liveness == (livenesspb.Liveness{}) { + if newLivenessRec.Liveness.Equal(livenesspb.Liveness{}) { log.Fatal(ctx, "invalid new liveness record; found to be empty") } diff --git a/pkg/kv/kvserver/liveness/liveness.go b/pkg/kv/kvserver/liveness/liveness.go index 8bf0ec034fa9..286d3cb56d4c 100644 --- a/pkg/kv/kvserver/liveness/liveness.go +++ b/pkg/kv/kvserver/liveness/liveness.go @@ -478,7 +478,7 @@ func (nl *NodeLiveness) setDrainingInternal( <-sem }() - if oldLivenessRec.Liveness == (livenesspb.Liveness{}) { + if oldLivenessRec.Liveness.Equal(livenesspb.Liveness{}) { return errors.AssertionFailedf("invalid old liveness record; found to be empty") } @@ -796,7 +796,7 @@ func (nl *NodeLiveness) heartbeatInternal( } } - if oldLiveness == (livenesspb.Liveness{}) { + if oldLiveness.Equal(livenesspb.Liveness{}) { return errors.AssertionFailedf("invalid old liveness record; found to be empty") } @@ -1122,7 +1122,7 @@ func (nl *NodeLiveness) updateLivenessAttempt( // ErrMissingRecord instead. return Record{}, ErrRecordCacheMiss } - if l.Liveness != update.oldLiveness { + if !l.Liveness.Equal(update.oldLiveness) { return Record{}, handleCondFailed(l) } update.oldRaw = l.raw diff --git a/pkg/kv/kvserver/liveness/livenesspb/liveness.go b/pkg/kv/kvserver/liveness/livenesspb/liveness.go index 0b216fb41b36..ce1aeb3659a7 100644 --- a/pkg/kv/kvserver/liveness/livenesspb/liveness.go +++ b/pkg/kv/kvserver/liveness/livenesspb/liveness.go @@ -114,7 +114,7 @@ func (c MembershipStatus) String() string { // This returns an error if the transition is invalid, and false if the // transition is unnecessary (since it would be a no-op). func ValidateTransition(old Liveness, newStatus MembershipStatus) (bool, error) { - if (old == Liveness{}) { + if (old.Equal(Liveness{})) { return false, errors.AssertionFailedf("invalid old liveness record; found to be empty") } diff --git a/pkg/kv/kvserver/liveness/livenesspb/liveness.proto b/pkg/kv/kvserver/liveness/livenesspb/liveness.proto index 15fbf97a8c86..6a2027eb6b44 100644 --- a/pkg/kv/kvserver/liveness/livenesspb/liveness.proto +++ b/pkg/kv/kvserver/liveness/livenesspb/liveness.proto @@ -50,6 +50,10 @@ message Liveness { // the defining MembershipStatus to be on-the-wire compatible with the boolean // representation. MembershipStatus membership = 5; + + // Membership status of each of the stores. For backwards compatibility, any + // store not listed is assumed to be ACTIVE. + map storeMembership = 6; } // MembershipStatus enumerates the possible membership states a node could in. diff --git a/pkg/kv/kvserver/replica_rangefeed_test.go b/pkg/kv/kvserver/replica_rangefeed_test.go index 1c57547933c3..1f0985820cef 100644 --- a/pkg/kv/kvserver/replica_rangefeed_test.go +++ b/pkg/kv/kvserver/replica_rangefeed_test.go @@ -1594,7 +1594,7 @@ func TestRangefeedCheckpointsRecoverFromLeaseExpiration(t *testing.T) { testutils.SucceedsSoon(t, func() error { nl1 := n1.NodeLiveness().(*liveness.NodeLiveness) n2LivenessFromN1, _ := nl1.GetLiveness(n2.NodeID()) - if n2Liveness != n2LivenessFromN1.Liveness { + if !n2Liveness.Equal(n2LivenessFromN1.Liveness) { return errors.Errorf("waiting for node 2 liveness to converge on both nodes 1 and 2") } return nil @@ -1812,7 +1812,7 @@ func TestNewRangefeedForceLeaseRetry(t *testing.T) { testutils.SucceedsSoon(t, func() error { nl1 := n1.NodeLiveness().(*liveness.NodeLiveness) n2LivenessFromN1, _ := nl1.GetLiveness(n2.NodeID()) - if n2Liveness != n2LivenessFromN1.Liveness { + if !n2Liveness.Equal(n2LivenessFromN1.Liveness) { return errors.Errorf("waiting for node 2 liveness to converge on both nodes 1 and 2") } return nil diff --git a/pkg/testutils/testcluster/testcluster.go b/pkg/testutils/testcluster/testcluster.go index 93adda29e143..c8605adda89d 100644 --- a/pkg/testutils/testcluster/testcluster.go +++ b/pkg/testutils/testcluster/testcluster.go @@ -1610,7 +1610,7 @@ func (tc *TestCluster) WaitForNodeLiveness(t serverutils.TestFataler) { if err := db.GetProto(context.Background(), key, &liveness); err != nil { return err } - if (liveness == livenesspb.Liveness{}) { + if (liveness.Equal(livenesspb.Liveness{})) { return fmt.Errorf("no liveness record") } if liveness.Epoch < 1 {