diff --git a/pkg/raft/quorum/joint.go b/pkg/raft/quorum/joint.go index e806bff5dbcc..14f5c3c2ecb2 100644 --- a/pkg/raft/quorum/joint.go +++ b/pkg/raft/quorum/joint.go @@ -45,6 +45,20 @@ func (c JointConfig) IDs() map[pb.PeerID]struct{} { return m } +// Visit calls the given function for each unique voter ID in the joint +// configuration. +func (c JointConfig) Visit(f func(pb.PeerID)) { + for id := range c[0] { + f(id) + } + for id := range c[1] { + if _, ok := c[0][id]; ok { + continue // skip duplicate + } + f(id) + } +} + // Describe returns a (multi-line) representation of the commit indexes for the // given lookuper. func (c JointConfig) Describe(l AckedIndexer) string { diff --git a/pkg/raft/quorum/quorum_test.go b/pkg/raft/quorum/quorum_test.go index 7c0924b6720a..8da15666ba0e 100644 --- a/pkg/raft/quorum/quorum_test.go +++ b/pkg/raft/quorum/quorum_test.go @@ -6,6 +6,7 @@ package quorum import ( + "slices" "testing" pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb" @@ -134,3 +135,21 @@ func TestLeadSupportExpirationJointConfig(t *testing.T) { require.Equal(t, tc.exp, j.LeadSupportExpiration(tc.support)) } } + +func TestJointConfigVisit(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + j := JointConfig{ + MajorityConfig{1: struct{}{}, 2: struct{}{}, 3: struct{}{}}, + MajorityConfig{2: struct{}{}, 3: struct{}{}, 4: struct{}{}}, + } + + var visited []pb.PeerID + j.Visit(func(id pb.PeerID) { + visited = append(visited, id) + }) + slices.Sort(visited) + + require.Equal(t, []pb.PeerID{1, 2, 3, 4}, visited) +} diff --git a/pkg/raft/tracker/fortificationtracker.go b/pkg/raft/tracker/fortificationtracker.go index b8a889f3f697..2061ea1a4378 100644 --- a/pkg/raft/tracker/fortificationtracker.go +++ b/pkg/raft/tracker/fortificationtracker.go @@ -164,26 +164,31 @@ func (ft *FortificationTracker) computeLeadSupportUntil(state pb.StateType) hlc. if state != pb.StateLeader { panic("computeLeadSupportUntil should only be called by the leader") } + if len(ft.fortification) == 0 { + return hlc.Timestamp{} // fast-path for no fortification + } // TODO(arul): avoid this map allocation as we're calling LeadSupportUntil // from hot paths. supportExpMap := make(map[pb.PeerID]hlc.Timestamp) - for id, supportEpoch := range ft.fortification { - curEpoch, curExp := ft.storeLiveness.SupportFrom(id) - // NB: We can't assert that supportEpoch <= curEpoch because there may be a - // race between a successful MsgFortifyLeaderResp and the store liveness - // heartbeat response that lets the leader know the follower's store is - // supporting the leader's store at the epoch in the MsgFortifyLeaderResp - // message. - if curEpoch == supportEpoch { - supportExpMap[id] = curExp + ft.config.Voters.Visit(func(id pb.PeerID) { + if supportEpoch, ok := ft.fortification[id]; ok { + curEpoch, curExp := ft.storeLiveness.SupportFrom(id) + // NB: We can't assert that supportEpoch <= curEpoch because there may be + // a race between a successful MsgFortifyLeaderResp and the store liveness + // heartbeat response that lets the leader know the follower's store is + // supporting the leader's store at the epoch in the MsgFortifyLeaderResp + // message. + if curEpoch == supportEpoch { + supportExpMap[id] = curExp + } } - } + }) return ft.config.Voters.LeadSupportExpiration(supportExpMap) } // CanDefortify returns whether the caller can safely[1] de-fortify the term -// based on the sate tracked by the FortificationTracker. +// based on the state tracked by the FortificationTracker. // // [1] Without risking regressions in the maximum that's ever been indicated to // the layers above. Or, more simply, without risking regression of leader