diff --git a/pkg/kv/kvserver/BUILD.bazel b/pkg/kv/kvserver/BUILD.bazel index fb88985ce8bc..5029da4ce368 100644 --- a/pkg/kv/kvserver/BUILD.bazel +++ b/pkg/kv/kvserver/BUILD.bazel @@ -443,6 +443,7 @@ go_test( "//pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer", "//pkg/raft", "//pkg/raft/confchange", + "//pkg/raft/quorum", "//pkg/raft/raftpb", "//pkg/raft/raftstoreliveness", "//pkg/raft/tracker", diff --git a/pkg/kv/kvserver/client_atomic_membership_change_test.go b/pkg/kv/kvserver/client_atomic_membership_change_test.go index 0c4dc299dc23..ba0ce5bc6700 100644 --- a/pkg/kv/kvserver/client_atomic_membership_change_test.go +++ b/pkg/kv/kvserver/client_atomic_membership_change_test.go @@ -19,6 +19,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/kv/kvpb" "github.com/cockroachdb/cockroach/pkg/kv/kvserver" "github.com/cockroachdb/cockroach/pkg/raft/confchange" + "github.com/cockroachdb/cockroach/pkg/raft/quorum" "github.com/cockroachdb/cockroach/pkg/raft/tracker" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/testutils" @@ -86,9 +87,13 @@ func TestAtomicReplicationChange(t *testing.T) { // Check that conf state is up to date. This can fail even though // the descriptor already matches since the descriptor is updated // a hair earlier. - cfg, _, err := confchange.Restore(confchange.Changer{ - Tracker: tracker.MakeProgressTracker(1, 0), - LastIndex: 1, + cfg := quorum.MakeEmptyConfig() + cfg, _, err = confchange.Restore(confchange.Changer{ + ProgressMap: tracker.MakeEmptyProgressMap(), + Config: cfg, + MaxInflight: 1, + MaxInflightBytes: 0, + LastIndex: 1, }, desc.Replicas().ConfState()) require.NoError(t, err) act := r.RaftStatus().Config.Voters diff --git a/pkg/raft/confchange/BUILD.bazel b/pkg/raft/confchange/BUILD.bazel index f3869cbead2d..6042995d6f67 100644 --- a/pkg/raft/confchange/BUILD.bazel +++ b/pkg/raft/confchange/BUILD.bazel @@ -25,6 +25,7 @@ go_test( data = glob(["testdata/**"]), embed = [":confchange"], deps = [ + "//pkg/raft/quorum", "//pkg/raft/raftpb", "//pkg/raft/tracker", "@com_github_cockroachdb_datadriven//:datadriven", diff --git a/pkg/raft/confchange/confchange.go b/pkg/raft/confchange/confchange.go index 01dfd1c75b24..8e204b4ffc3d 100644 --- a/pkg/raft/confchange/confchange.go +++ b/pkg/raft/confchange/confchange.go @@ -32,8 +32,11 @@ import ( // refusing invalid configuration changes before they affect the active // configuration. type Changer struct { - Tracker tracker.ProgressTracker - LastIndex uint64 + Config quorum.Config + ProgressMap tracker.ProgressMap + MaxInflight int + MaxInflightBytes uint64 + LastIndex uint64 } // EnterJoint verifies that the outgoing (=right) majority config of the joint @@ -53,7 +56,7 @@ type Changer struct { // [1]: https://github.com/ongardie/dissertation/blob/master/online-trim.pdf func (c Changer) EnterJoint( autoLeave bool, ccs ...pb.ConfChangeSingle, -) (tracker.Config, tracker.ProgressMap, error) { +) (quorum.Config, tracker.ProgressMap, error) { cfg, trk, err := c.checkAndCopy() if err != nil { return c.err(err) @@ -96,7 +99,7 @@ func (c Changer) EnterJoint( // inserted into Learners. // // [1]: https://github.com/ongardie/dissertation/blob/master/online-trim.pdf -func (c Changer) LeaveJoint() (tracker.Config, tracker.ProgressMap, error) { +func (c Changer) LeaveJoint() (quorum.Config, tracker.ProgressMap, error) { cfg, trk, err := c.checkAndCopy() if err != nil { return c.err(err) @@ -130,7 +133,7 @@ func (c Changer) LeaveJoint() (tracker.Config, tracker.ProgressMap, error) { // will return an error if that is not the case, if the resulting quorum is // zero, or if the configuration is in a joint state (i.e. if there is an // outgoing configuration). -func (c Changer) Simple(ccs ...pb.ConfChangeSingle) (tracker.Config, tracker.ProgressMap, error) { +func (c Changer) Simple(ccs ...pb.ConfChangeSingle) (quorum.Config, tracker.ProgressMap, error) { cfg, trk, err := c.checkAndCopy() if err != nil { return c.err(err) @@ -142,8 +145,8 @@ func (c Changer) Simple(ccs ...pb.ConfChangeSingle) (tracker.Config, tracker.Pro if err := c.apply(&cfg, trk, ccs...); err != nil { return c.err(err) } - if n := symdiff(incoming(c.Tracker.Voters), incoming(cfg.Voters)); n > 1 { - return tracker.Config{}, nil, errors.New("more than one voter changed without entering joint config") + if n := symdiff(incoming(c.Config.Voters), incoming(cfg.Voters)); n > 1 { + return quorum.Config{}, nil, errors.New("more than one voter changed without entering joint config") } return checkAndReturn(cfg, trk) @@ -153,7 +156,7 @@ func (c Changer) Simple(ccs ...pb.ConfChangeSingle) (tracker.Config, tracker.Pro // always made to the incoming majority config Voters[0]. Voters[1] is either // empty or preserves the outgoing majority configuration while in a joint state. func (c Changer) apply( - cfg *tracker.Config, trk tracker.ProgressMap, ccs ...pb.ConfChangeSingle, + cfg *quorum.Config, trk tracker.ProgressMap, ccs ...pb.ConfChangeSingle, ) error { for _, cc := range ccs { if cc.NodeID == 0 { @@ -182,7 +185,7 @@ func (c Changer) apply( // makeVoter adds or promotes the given ID to be a voter in the incoming // majority config. -func (c Changer) makeVoter(cfg *tracker.Config, trk tracker.ProgressMap, id pb.PeerID) { +func (c Changer) makeVoter(cfg *quorum.Config, trk tracker.ProgressMap, id pb.PeerID) { pr := trk[id] if pr == nil { c.initProgress(cfg, trk, id, false /* isLearner */) @@ -208,7 +211,7 @@ func (c Changer) makeVoter(cfg *tracker.Config, trk tracker.ProgressMap, id pb.P // simultaneously. Instead, we add the learner to LearnersNext, so that it will // be added to Learners the moment the outgoing config is removed by // LeaveJoint(). -func (c Changer) makeLearner(cfg *tracker.Config, trk tracker.ProgressMap, id pb.PeerID) { +func (c Changer) makeLearner(cfg *quorum.Config, trk tracker.ProgressMap, id pb.PeerID) { pr := trk[id] if pr == nil { c.initProgress(cfg, trk, id, true /* isLearner */) @@ -235,7 +238,7 @@ func (c Changer) makeLearner(cfg *tracker.Config, trk tracker.ProgressMap, id pb } // remove this peer as a voter or learner from the incoming config. -func (c Changer) remove(cfg *tracker.Config, trk tracker.ProgressMap, id pb.PeerID) { +func (c Changer) remove(cfg *quorum.Config, trk tracker.ProgressMap, id pb.PeerID) { if _, ok := trk[id]; !ok { return } @@ -252,7 +255,7 @@ func (c Changer) remove(cfg *tracker.Config, trk tracker.ProgressMap, id pb.Peer // initProgress initializes a new progress for the given node or learner. func (c Changer) initProgress( - cfg *tracker.Config, trk tracker.ProgressMap, id pb.PeerID, isLearner bool, + cfg *quorum.Config, trk tracker.ProgressMap, id pb.PeerID, isLearner bool, ) { if !isLearner { incoming(cfg.Voters)[id] = struct{}{} @@ -270,7 +273,7 @@ func (c Changer) initProgress( // making the first index the better choice). Match: 0, Next: max(c.LastIndex, 1), // invariant: Match < Next - Inflights: tracker.NewInflights(c.Tracker.MaxInflight, c.Tracker.MaxInflightBytes), + Inflights: tracker.NewInflights(c.MaxInflight, c.MaxInflightBytes), IsLearner: isLearner, // When a node is first added, we should mark it as recently active. // Otherwise, CheckQuorum may cause us to step down if it is invoked @@ -282,7 +285,7 @@ func (c Changer) initProgress( // checkInvariants makes sure that the config and progress are compatible with // each other. This is used to check both what the Changer is initialized with, // as well as what it returns. -func checkInvariants(cfg tracker.Config, trk tracker.ProgressMap) error { +func checkInvariants(cfg quorum.Config, trk tracker.ProgressMap) error { // NB: intentionally allow the empty config. In production we'll never see a // non-empty config (we prevent it from being created) but we will need to // be able to *create* an initial config, for example during bootstrap (or @@ -343,11 +346,11 @@ func checkInvariants(cfg tracker.Config, trk tracker.ProgressMap) error { // checkAndCopy copies the tracker's config and progress map (deeply enough for // the purposes of the Changer) and returns those copies. It returns an error // if checkInvariants does. -func (c Changer) checkAndCopy() (tracker.Config, tracker.ProgressMap, error) { - cfg := c.Tracker.Config.Clone() +func (c Changer) checkAndCopy() (quorum.Config, tracker.ProgressMap, error) { + cfg := c.Config.Clone() trk := tracker.ProgressMap{} - for id, pr := range c.Tracker.Progress { + for id, pr := range c.ProgressMap { // A shallow copy is enough because we only mutate the Learner field. ppr := *pr trk[id] = &ppr @@ -358,17 +361,17 @@ func (c Changer) checkAndCopy() (tracker.Config, tracker.ProgressMap, error) { // checkAndReturn calls checkInvariants on the input and returns either the // resulting error or the input. func checkAndReturn( - cfg tracker.Config, trk tracker.ProgressMap, -) (tracker.Config, tracker.ProgressMap, error) { + cfg quorum.Config, trk tracker.ProgressMap, +) (quorum.Config, tracker.ProgressMap, error) { if err := checkInvariants(cfg, trk); err != nil { - return tracker.Config{}, tracker.ProgressMap{}, err + return quorum.Config{}, tracker.ProgressMap{}, err } return cfg, trk, nil } // err returns zero values and an error. -func (c Changer) err(err error) (tracker.Config, tracker.ProgressMap, error) { - return tracker.Config{}, nil, err +func (c Changer) err(err error) (quorum.Config, tracker.ProgressMap, error) { + return quorum.Config{}, nil, err } // nilAwareAdd populates a map entry, creating the map if necessary. @@ -408,7 +411,7 @@ func symdiff(l, r map[pb.PeerID]struct{}) int { return n } -func joint(cfg tracker.Config) bool { +func joint(cfg quorum.Config) bool { return len(outgoing(cfg.Voters)) > 0 } diff --git a/pkg/raft/confchange/datadriven_test.go b/pkg/raft/confchange/datadriven_test.go index df2451c822f6..2aeef89f6200 100644 --- a/pkg/raft/confchange/datadriven_test.go +++ b/pkg/raft/confchange/datadriven_test.go @@ -24,6 +24,7 @@ import ( "strings" "testing" + "github.com/cockroachdb/cockroach/pkg/raft/quorum" pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb" "github.com/cockroachdb/cockroach/pkg/raft/tracker" "github.com/cockroachdb/datadriven" @@ -31,10 +32,12 @@ import ( func TestConfChangeDataDriven(t *testing.T) { datadriven.Walk(t, "testdata", func(t *testing.T, path string) { - tr := tracker.MakeProgressTracker(10, 0) c := Changer{ - Tracker: tr, - LastIndex: 0, // incremented in this test with each cmd + Config: quorum.MakeEmptyConfig(), + ProgressMap: tracker.MakeEmptyProgressMap(), + MaxInflight: 10, + MaxInflightBytes: 0, + LastIndex: 0, // incremented in this test with each cmd } // The test files use the commands @@ -81,23 +84,23 @@ func TestConfChangeDataDriven(t *testing.T) { ccs = append(ccs, cc) } - var cfg tracker.Config - var trk tracker.ProgressMap + var cfg quorum.Config + var progressMap tracker.ProgressMap var err error switch d.Cmd { case "simple": - cfg, trk, err = c.Simple(ccs...) + cfg, progressMap, err = c.Simple(ccs...) case "enter-joint": var autoLeave bool if len(d.CmdArgs) > 0 { d.ScanArgs(t, "autoleave", &autoLeave) } - cfg, trk, err = c.EnterJoint(autoLeave, ccs...) + cfg, progressMap, err = c.EnterJoint(autoLeave, ccs...) case "leave-joint": if len(ccs) > 0 { err = errors.New("this command takes no input") } else { - cfg, trk, err = c.LeaveJoint() + cfg, progressMap, err = c.LeaveJoint() } default: return "unknown command" @@ -105,8 +108,8 @@ func TestConfChangeDataDriven(t *testing.T) { if err != nil { return err.Error() + "\n" } - c.Tracker.Config, c.Tracker.Progress = cfg, trk - return fmt.Sprintf("%s\n%s", c.Tracker.Config, c.Tracker.Progress) + c.Config, c.ProgressMap = cfg, progressMap + return fmt.Sprintf("%s\n%s", c.Config, c.ProgressMap) }) }) } diff --git a/pkg/raft/confchange/quick_test.go b/pkg/raft/confchange/quick_test.go index e09d875290ff..30e0ea496dbc 100644 --- a/pkg/raft/confchange/quick_test.go +++ b/pkg/raft/confchange/quick_test.go @@ -24,6 +24,7 @@ import ( "testing" "testing/quick" + "github.com/cockroachdb/cockroach/pkg/raft/quorum" pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb" "github.com/cockroachdb/cockroach/pkg/raft/tracker" ) @@ -40,7 +41,7 @@ func TestConfChangeQuick(t *testing.T) { const infoCount = 5 runWithJoint := func(c *Changer, ccs []pb.ConfChangeSingle) error { - cfg, trk, err := c.EnterJoint(false /* autoLeave */, ccs...) + cfg, progressMap, err := c.EnterJoint(false /* autoLeave */, ccs...) if err != nil { return err } @@ -51,29 +52,29 @@ func TestConfChangeQuick(t *testing.T) { return err } cfg2a.AutoLeave = false - if !reflect.DeepEqual(cfg, cfg2a) || !reflect.DeepEqual(trk, trk2a) { - return fmt.Errorf("cfg: %+v\ncfg2a: %+v\ntrk: %+v\ntrk2a: %+v", - cfg, cfg2a, trk, trk2a) + if !reflect.DeepEqual(cfg, cfg2a) || !reflect.DeepEqual(progressMap, trk2a) { + return fmt.Errorf("cfg: %+v\ncfg2a: %+v\nprogressMap: %+v\ntrk2a: %+v", + cfg, cfg2a, progressMap, trk2a) } - c.Tracker.Config = cfg - c.Tracker.Progress = trk + c.Config = cfg + c.ProgressMap = progressMap cfg2b, trk2b, err := c.LeaveJoint() if err != nil { return err } // Reset back to the main branch with autoLeave=false. - c.Tracker.Config = cfg - c.Tracker.Progress = trk - cfg, trk, err = c.LeaveJoint() + c.Config = cfg + c.ProgressMap = progressMap + cfg, progressMap, err = c.LeaveJoint() if err != nil { return err } - if !reflect.DeepEqual(cfg, cfg2b) || !reflect.DeepEqual(trk, trk2b) { - return fmt.Errorf("cfg: %+v\ncfg2b: %+v\ntrk: %+v\ntrk2b: %+v", - cfg, cfg2b, trk, trk2b) + if !reflect.DeepEqual(cfg, cfg2b) || !reflect.DeepEqual(progressMap, trk2b) { + return fmt.Errorf("cfg: %+v\ncfg2b: %+v\nprogressMap: %+v\ntrk2b: %+v", + cfg, cfg2b, progressMap, trk2b) } - c.Tracker.Config = cfg - c.Tracker.Progress = trk + c.Config = cfg + c.ProgressMap = progressMap return nil } @@ -83,7 +84,7 @@ func TestConfChangeQuick(t *testing.T) { if err != nil { return err } - c.Tracker.Config, c.Tracker.Progress = cfg, trk + c.Config, c.ProgressMap = cfg, trk } return nil } @@ -92,10 +93,12 @@ func TestConfChangeQuick(t *testing.T) { wrapper := func(invoke testFunc) func(setup initialChanges, ccs confChanges) (*Changer, error) { return func(setup initialChanges, ccs confChanges) (*Changer, error) { - tr := tracker.MakeProgressTracker(10, 0) c := &Changer{ - Tracker: tr, - LastIndex: 10, + Config: quorum.MakeEmptyConfig(), + ProgressMap: tracker.MakeEmptyProgressMap(), + MaxInflight: 10, + MaxInflightBytes: 0, + LastIndex: 10, } if err := runWithSimple(c, setup); err != nil { @@ -116,8 +119,8 @@ func TestConfChangeQuick(t *testing.T) { if n < infoCount { t.Log("initial setup:", Describe(setup...)) t.Log("changes:", Describe(ccs...)) - t.Log(c.Tracker.Config) - t.Log(c.Tracker.Progress) + t.Log(c.Config) + t.Log(c.ProgressMap) } n++ return c diff --git a/pkg/raft/confchange/restore.go b/pkg/raft/confchange/restore.go index ba312debaf58..5ac80a3c25b4 100644 --- a/pkg/raft/confchange/restore.go +++ b/pkg/raft/confchange/restore.go @@ -18,6 +18,7 @@ package confchange import ( + "github.com/cockroachdb/cockroach/pkg/raft/quorum" pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb" "github.com/cockroachdb/cockroach/pkg/raft/tracker" ) @@ -100,17 +101,17 @@ func toConfChangeSingle(cs pb.ConfState) (out []pb.ConfChangeSingle, in []pb.Con } func chain( - chg Changer, ops ...func(Changer) (tracker.Config, tracker.ProgressMap, error), -) (tracker.Config, tracker.ProgressMap, error) { + chg Changer, ops ...func(Changer) (quorum.Config, tracker.ProgressMap, error), +) (quorum.Config, tracker.ProgressMap, error) { for _, op := range ops { - cfg, trk, err := op(chg) + cfg, progressMap, err := op(chg) if err != nil { - return tracker.Config{}, nil, err + return quorum.Config{}, nil, err } - chg.Tracker.Config = cfg - chg.Tracker.Progress = trk + chg.Config = cfg + chg.ProgressMap = progressMap } - return chg.Tracker.Config, chg.Tracker.Progress, nil + return chg.Config, chg.ProgressMap, nil } // Restore takes a Changer (which must represent an empty configuration), and @@ -121,16 +122,16 @@ func chain( // the Changer only needs a ProgressMap (not a whole Tracker) at which point // this can just take LastIndex and MaxInflight directly instead and cook up // the results from that alone. -func Restore(chg Changer, cs pb.ConfState) (tracker.Config, tracker.ProgressMap, error) { +func Restore(chg Changer, cs pb.ConfState) (quorum.Config, tracker.ProgressMap, error) { outgoing, incoming := toConfChangeSingle(cs) - var ops []func(Changer) (tracker.Config, tracker.ProgressMap, error) + var ops []func(Changer) (quorum.Config, tracker.ProgressMap, error) if len(outgoing) == 0 { // No outgoing config, so just apply the incoming changes one by one. for _, cc := range incoming { cc := cc // loop-local copy - ops = append(ops, func(chg Changer) (tracker.Config, tracker.ProgressMap, error) { + ops = append(ops, func(chg Changer) (quorum.Config, tracker.ProgressMap, error) { return chg.Simple(cc) }) } @@ -142,7 +143,7 @@ func Restore(chg Changer, cs pb.ConfState) (tracker.Config, tracker.ProgressMap, // if the config is (1 2 3)&(2 3 4), this will establish (2 3 4)&(). for _, cc := range outgoing { cc := cc // loop-local copy - ops = append(ops, func(chg Changer) (tracker.Config, tracker.ProgressMap, error) { + ops = append(ops, func(chg Changer) (quorum.Config, tracker.ProgressMap, error) { return chg.Simple(cc) }) } @@ -151,7 +152,7 @@ func Restore(chg Changer, cs pb.ConfState) (tracker.Config, tracker.ProgressMap, // example above, we'd get (1 2 3)&(2 3 4), i.e. the incoming operations // would be removing 2,3,4 and then adding in 1,2,3 while transitioning // into a joint state. - ops = append(ops, func(chg Changer) (tracker.Config, tracker.ProgressMap, error) { + ops = append(ops, func(chg Changer) (quorum.Config, tracker.ProgressMap, error) { return chg.EnterJoint(cs.AutoLeave, incoming...) }) } diff --git a/pkg/raft/confchange/restore_test.go b/pkg/raft/confchange/restore_test.go index 474c07b2a9a5..0114a787867d 100644 --- a/pkg/raft/confchange/restore_test.go +++ b/pkg/raft/confchange/restore_test.go @@ -24,6 +24,7 @@ import ( "testing" "testing/quick" + "github.com/cockroachdb/cockroach/pkg/raft/quorum" pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb" "github.com/cockroachdb/cockroach/pkg/raft/tracker" ) @@ -89,16 +90,17 @@ func TestRestore(t *testing.T) { f := func(cs pb.ConfState) bool { chg := Changer{ - Tracker: tracker.MakeProgressTracker(20, 0), - LastIndex: 10, + Config: quorum.MakeEmptyConfig(), + ProgressMap: tracker.MakeEmptyProgressMap(), + LastIndex: 10, } - cfg, trk, err := Restore(chg, cs) + cfg, progressMap, err := Restore(chg, cs) if err != nil { t.Error(err) return false } - chg.Tracker.Config = cfg - chg.Tracker.Progress = trk + chg.Config = cfg + chg.ProgressMap = progressMap for _, sl := range [][]pb.PeerID{ cs.Voters, @@ -109,7 +111,7 @@ func TestRestore(t *testing.T) { sort.Slice(sl, func(i, j int) bool { return sl[i] < sl[j] }) } - cs2 := chg.Tracker.ConfState() + cs2 := chg.Config.ConfState() // NB: cs.Equivalent does the same "sorting" dance internally, but let's // test it a bit here instead of relying on it. if reflect.DeepEqual(cs, cs2) && cs.Equivalent(cs2) == nil && cs2.Equivalent(cs) == nil { diff --git a/pkg/raft/quorum/BUILD.bazel b/pkg/raft/quorum/BUILD.bazel index acfca753499d..662c5aaea5c6 100644 --- a/pkg/raft/quorum/BUILD.bazel +++ b/pkg/raft/quorum/BUILD.bazel @@ -4,6 +4,7 @@ load("//build:STRINGER.bzl", "stringer") go_library( name = "quorum", srcs = [ + "config.go", "joint.go", "majority.go", "quorum.go", diff --git a/pkg/raft/quorum/config.go b/pkg/raft/quorum/config.go new file mode 100644 index 000000000000..0529bebaa52c --- /dev/null +++ b/pkg/raft/quorum/config.go @@ -0,0 +1,130 @@ +// Copyright 2024 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package quorum + +import ( + "fmt" + "strings" + + pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb" +) + +// Config reflects the configuration of a raft group. It is used to make +// quorum decisions and perform configuration changes. +type Config struct { + Voters JointConfig + // AutoLeave is true if the configuration is joint and a transition to the + // incoming configuration should be carried out automatically by Raft when + // this is possible. If false, the configuration will be joint until the + // application initiates the transition manually. + AutoLeave bool + // Learners is a set of IDs corresponding to the learners active in the + // current configuration. + // + // Invariant: Learners and Voters does not intersect, i.e. if a peer is in + // either half of the joint config, it can't be a learner; if it is a + // learner it can't be in either half of the joint config. This invariant + // simplifies the implementation since it allows peers to have clarity about + // its current role without taking into account joint consensus. + Learners map[pb.PeerID]struct{} + // When we turn a voter into a learner during a joint consensus transition, + // we cannot add the learner directly when entering the joint state. This is + // because this would violate the invariant that the intersection of + // voters and learners is empty. For example, assume a Voter is removed and + // immediately re-added as a learner (or in other words, it is demoted): + // + // Initially, the configuration will be + // + // voters: {1 2 3} + // learners: {} + // + // and we want to demote 3. Entering the joint configuration, we naively get + // + // voters: {1 2} & {1 2 3} + // learners: {3} + // + // but this violates the invariant (3 is both voter and learner). Instead, + // we get + // + // voters: {1 2} & {1 2 3} + // learners: {} + // next_learners: {3} + // + // Where 3 is now still purely a voter, but we are remembering the intention + // to make it a learner upon transitioning into the final configuration: + // + // voters: {1 2} + // learners: {3} + // next_learners: {} + // + // Note that next_learners is not used while adding a learner that is not + // also a voter in the joint config. In this case, the learner is added + // right away when entering the joint configuration, so that it is caught up + // as soon as possible. + LearnersNext map[pb.PeerID]struct{} +} + +// MakeEmptyConfig constructs and returns an empty Config. +func MakeEmptyConfig() Config { + return Config{ + Voters: JointConfig{ + MajorityConfig{}, + nil, // only populated when used + }, + Learners: nil, // only populated when used + LearnersNext: nil, // only populated when used + } +} + +func (c Config) String() string { + var buf strings.Builder + fmt.Fprintf(&buf, "voters=%s", c.Voters) + if c.Learners != nil { + fmt.Fprintf(&buf, " learners=%s", MajorityConfig(c.Learners).String()) + } + if c.LearnersNext != nil { + fmt.Fprintf(&buf, " learners_next=%s", MajorityConfig(c.LearnersNext).String()) + } + if c.AutoLeave { + fmt.Fprint(&buf, " autoleave") + } + return buf.String() +} + +// Clone returns a copy of the Config that shares no memory with the original. +func (c *Config) Clone() Config { + clone := func(m map[pb.PeerID]struct{}) map[pb.PeerID]struct{} { + if m == nil { + return nil + } + mm := make(map[pb.PeerID]struct{}, len(m)) + for k := range m { + mm[k] = struct{}{} + } + return mm + } + return Config{ + Voters: JointConfig{clone(c.Voters[0]), clone(c.Voters[1])}, + Learners: clone(c.Learners), + LearnersNext: clone(c.LearnersNext), + } +} + +// ConfState returns a ConfState representing the active configuration. +func (c *Config) ConfState() pb.ConfState { + return pb.ConfState{ + Voters: c.Voters[0].Slice(), + VotersOutgoing: c.Voters[1].Slice(), + Learners: MajorityConfig(c.Learners).Slice(), + LearnersNext: MajorityConfig(c.LearnersNext).Slice(), + AutoLeave: c.AutoLeave, + } +} diff --git a/pkg/raft/raft.go b/pkg/raft/raft.go index 6b5c6540b10c..bf6555177b41 100644 --- a/pkg/raft/raft.go +++ b/pkg/raft/raft.go @@ -325,7 +325,8 @@ type raft struct { maxMsgSize entryEncodingSize maxUncommittedSize entryPayloadSize - trk tracker.ProgressTracker + config quorum.Config + trk tracker.ProgressTracker state StateType @@ -381,8 +382,10 @@ type raft struct { // only leader keeps heartbeatElapsed. heartbeatElapsed int - checkQuorum bool - preVote bool + maxInflight int + maxInflightBytes uint64 + checkQuorum bool + preVote bool heartbeatTimeout int electionTimeout int @@ -417,10 +420,11 @@ func newRaft(c *Config) *raft { raftLog: raftlog, maxMsgSize: entryEncodingSize(c.MaxSizePerMsg), maxUncommittedSize: entryPayloadSize(c.MaxUncommittedEntriesSize), - trk: tracker.MakeProgressTracker(c.MaxInflightMsgs, c.MaxInflightBytes), electionTimeout: c.ElectionTick, heartbeatTimeout: c.HeartbeatTick, logger: c.Logger, + maxInflight: c.MaxInflightMsgs, + maxInflightBytes: c.MaxInflightBytes, checkQuorum: c.CheckQuorum, preVote: c.PreVote, disableProposalForwarding: c.DisableProposalForwarding, @@ -430,14 +434,17 @@ func newRaft(c *Config) *raft { } lastID := r.raftLog.lastEntryID() - cfg, trk, err := confchange.Restore(confchange.Changer{ - Tracker: r.trk, - LastIndex: lastID.index, + cfg, progressMap, err := confchange.Restore(confchange.Changer{ + Config: quorum.MakeEmptyConfig(), + ProgressMap: tracker.MakeEmptyProgressMap(), + MaxInflight: r.maxInflight, + MaxInflightBytes: r.maxInflightBytes, + LastIndex: lastID.index, }, cs) if err != nil { panic(err) } - assertConfStatesEquivalent(r.logger, cs, r.switchToConfig(cfg, trk)) + assertConfStatesEquivalent(r.logger, cs, r.switchToConfig(cfg, progressMap)) if !IsEmptyHardState(hs) { r.loadState(hs) @@ -685,7 +692,7 @@ func (r *raft) appliedTo(index uint64, size entryEncodingSize) { newApplied := max(index, oldApplied) r.raftLog.appliedTo(newApplied, size) - if r.trk.Config.AutoLeave && newApplied >= r.pendingConfIndex && r.state == StateLeader { + if r.config.AutoLeave && newApplied >= r.pendingConfIndex && r.state == StateLeader { // If the current (and most recent, at least for this leader's term) // configuration should be auto-left, initiate that now. We use a // nil Data which unmarshals into an empty ConfChangeV2 and has the @@ -702,9 +709,9 @@ func (r *raft) appliedTo(index uint64, size entryEncodingSize) { // the joint configuration, or the leadership transfer will fail, // and we will propose the config change on the next advance. if err := r.Step(m); err != nil { - r.logger.Debugf("not initiating automatic transition out of joint configuration %s: %v", r.trk.Config, err) + r.logger.Debugf("not initiating automatic transition out of joint configuration %s: %v", r.config, err) } else { - r.logger.Infof("initiating automatic transition out of joint configuration %s", r.trk.Config) + r.logger.Infof("initiating automatic transition out of joint configuration %s", r.config) } } } @@ -758,7 +765,7 @@ func (r *raft) reset(term uint64) { *pr = tracker.Progress{ Match: 0, Next: r.raftLog.lastIndex() + 1, - Inflights: tracker.NewInflights(r.trk.MaxInflight, r.trk.MaxInflightBytes), + Inflights: tracker.NewInflights(r.maxInflight, r.maxInflightBytes), IsLearner: pr.IsLearner, } if id == r.id { @@ -1010,7 +1017,7 @@ func (r *raft) campaign(t CampaignType) { } var ids []pb.PeerID { - idMap := r.trk.Voters.IDs() + idMap := r.config.Voters.IDs() ids = make([]pb.PeerID, 0, len(idMap)) for id := range idMap { ids = append(ids, id) @@ -1296,7 +1303,7 @@ func stepLeader(r *raft, m pb.Message) error { // [^1]: https://github.com/etcd-io/etcd/issues/7625#issuecomment-489232411 alreadyPending := r.pendingConfIndex > r.raftLog.applied - alreadyJoint := len(r.trk.Config.Voters[1]) > 0 + alreadyJoint := len(r.config.Voters[1]) > 0 wantsLeaveJoint := len(cc.AsV2().Changes) == 0 var failedCheck string @@ -1313,7 +1320,7 @@ func stepLeader(r *raft, m pb.Message) error { // // NB: !alreadyPending requirement is always respected, for safety. if alreadyPending || (failedCheck != "" && !r.disableConfChangeValidation) { - r.logger.Infof("%x ignoring conf change %v at config %s: %s", r.id, cc, r.trk.Config, failedCheck) + r.logger.Infof("%x ignoring conf change %v at config %s: %s", r.id, cc, r.config, failedCheck) m.Entries[i] = pb.Entry{Type: pb.EntryNormal} } else { r.pendingConfIndex = r.raftLog.lastIndex() + uint64(i) + 1 @@ -1898,11 +1905,13 @@ func (r *raft) restore(s snapshot) bool { return false } - // Reset the configuration and add the (potentially updated) peers in anew. - r.trk = tracker.MakeProgressTracker(r.trk.MaxInflight, r.trk.MaxInflightBytes) - cfg, trk, err := confchange.Restore(confchange.Changer{ - Tracker: r.trk, - LastIndex: r.raftLog.lastIndex(), + cfg, progressMap, err := confchange.Restore(confchange.Changer{ + // Reset the configuration and add the (potentially updated) peers in anew. + Config: quorum.MakeEmptyConfig(), + ProgressMap: tracker.MakeEmptyProgressMap(), // empty ProgressMap to go with our empty config + MaxInflight: r.maxInflight, + MaxInflightBytes: r.maxInflightBytes, + LastIndex: r.raftLog.lastIndex(), }, cs) if err != nil { @@ -1911,7 +1920,7 @@ func (r *raft) restore(s snapshot) bool { panic(fmt.Sprintf("unable to restore config %+v: %s", cs, err)) } - assertConfStatesEquivalent(r.logger, cs, r.switchToConfig(cfg, trk)) + assertConfStatesEquivalent(r.logger, cs, r.switchToConfig(cfg, progressMap)) last := r.raftLog.lastEntryID() r.logger.Infof("%x [commit: %d, lastindex: %d, lastterm: %d] restored snapshot [index: %d, term: %d]", @@ -1927,10 +1936,13 @@ func (r *raft) promotable() bool { } func (r *raft) applyConfChange(cc pb.ConfChangeV2) pb.ConfState { - cfg, trk, err := func() (tracker.Config, tracker.ProgressMap, error) { + cfg, progressMap, err := func() (quorum.Config, tracker.ProgressMap, error) { changer := confchange.Changer{ - Tracker: r.trk, - LastIndex: r.raftLog.lastIndex(), + Config: r.config, + ProgressMap: r.trk.Progress, + MaxInflight: r.maxInflight, + MaxInflightBytes: r.maxInflightBytes, + LastIndex: r.raftLog.lastIndex(), } if cc.LeaveJoint() { return changer.LeaveJoint() @@ -1945,7 +1957,7 @@ func (r *raft) applyConfChange(cc pb.ConfChangeV2) pb.ConfState { panic(err) } - return r.switchToConfig(cfg, trk) + return r.switchToConfig(cfg, progressMap) } // switchToConfig reconfigures this node to use the provided configuration. It @@ -1954,12 +1966,12 @@ func (r *raft) applyConfChange(cc pb.ConfChangeV2) pb.ConfState { // requirements. // // The inputs usually result from restoring a ConfState or applying a ConfChange. -func (r *raft) switchToConfig(cfg tracker.Config, trk tracker.ProgressMap) pb.ConfState { - r.trk.Config = cfg - r.trk.Progress = trk +func (r *raft) switchToConfig(cfg quorum.Config, progressMap tracker.ProgressMap) pb.ConfState { + r.config = cfg + r.trk = tracker.MakeProgressTracker(&r.config, progressMap) - r.logger.Infof("%x switched to configuration %s", r.id, r.trk.Config) - cs := r.trk.ConfState() + r.logger.Infof("%x switched to configuration %s", r.id, r.config) + cs := r.config.ConfState() pr, ok := r.trk.Progress[r.id] // Update whether the node itself is a learner, resetting to false when the @@ -1998,7 +2010,7 @@ func (r *raft) switchToConfig(cfg tracker.Config, trk tracker.ProgressMap) pb.Co r.bcastAppend() // If the leadTransferee was removed or demoted, abort the leadership transfer. - if _, tOK := r.trk.Config.Voters.IDs()[r.leadTransferee]; !tOK && r.leadTransferee != 0 { + if _, tOK := r.config.Voters.IDs()[r.leadTransferee]; !tOK && r.leadTransferee != 0 { r.abortLeaderTransfer() } diff --git a/pkg/raft/raft_flow_control_test.go b/pkg/raft/raft_flow_control_test.go index 191431c7055d..acd6aa6c858f 100644 --- a/pkg/raft/raft_flow_control_test.go +++ b/pkg/raft/raft_flow_control_test.go @@ -36,7 +36,7 @@ func TestMsgAppFlowControlFull(t *testing.T) { // force the progress to be in replicate state pr2.BecomeReplicate() // fill in the inflights window - for i := 0; i < r.trk.MaxInflight; i++ { + for i := 0; i < r.maxInflight; i++ { r.Step(pb.Message{From: 1, To: 1, Type: pb.MsgProp, Entries: []pb.Entry{{Data: []byte("somedata")}}}) ms := r.readMessages() if len(ms) != 1 || ms[0].Type != pb.MsgApp { @@ -72,14 +72,14 @@ func TestMsgAppFlowControlMoveForward(t *testing.T) { // force the progress to be in replicate state pr2.BecomeReplicate() // fill in the inflights window - for i := 0; i < r.trk.MaxInflight; i++ { + for i := 0; i < r.maxInflight; i++ { r.Step(pb.Message{From: 1, To: 1, Type: pb.MsgProp, Entries: []pb.Entry{{Data: []byte("somedata")}}}) r.readMessages() } // 1 is noop, 2 is the first proposal we just sent. // so we start with 2. - for tt := 2; tt < r.trk.MaxInflight; tt++ { + for tt := 2; tt < r.maxInflight; tt++ { // move forward the window r.Step(pb.Message{From: 2, To: 1, Type: pb.MsgAppResp, Index: uint64(tt)}) r.readMessages() @@ -117,7 +117,7 @@ func TestMsgAppFlowControlRecvHeartbeat(t *testing.T) { // force the progress to be in replicate state pr2.BecomeReplicate() // fill in the inflights window - for i := 0; i < r.trk.MaxInflight; i++ { + for i := 0; i < r.maxInflight; i++ { r.Step(pb.Message{From: 1, To: 1, Type: pb.MsgProp, Entries: []pb.Entry{{Data: []byte("somedata")}}}) r.readMessages() } diff --git a/pkg/raft/raft_test.go b/pkg/raft/raft_test.go index 927ab2cf3eaa..b4e30cf718f8 100644 --- a/pkg/raft/raft_test.go +++ b/pkg/raft/raft_test.go @@ -3745,22 +3745,22 @@ func newNetworkWithConfig(configFunc func(*Config), peers ...stateMachine) *netw npeers[id] = sm case *raft: // TODO(tbg): this is all pretty confused. Clean this up. - learners := make(map[pb.PeerID]bool, len(v.trk.Learners)) - for i := range v.trk.Learners { + learners := make(map[pb.PeerID]bool, len(v.config.Learners)) + for i := range v.config.Learners { learners[i] = true } v.id = id - v.trk = tracker.MakeProgressTracker(v.trk.MaxInflight, v.trk.MaxInflightBytes) + v.trk = tracker.MakeProgressTracker(&v.config, tracker.MakeEmptyProgressMap()) if len(learners) > 0 { - v.trk.Learners = map[pb.PeerID]struct{}{} + v.config.Learners = map[pb.PeerID]struct{}{} } for i := 0; i < size; i++ { pr := &tracker.Progress{} if _, ok := learners[peerAddrs[i]]; ok { pr.IsLearner = true - v.trk.Learners[peerAddrs[i]] = struct{}{} + v.config.Learners[peerAddrs[i]] = struct{}{} } else { - v.trk.Voters[0][peerAddrs[i]] = struct{}{} + v.config.Voters[0][peerAddrs[i]] = struct{}{} } v.trk.Progress[peerAddrs[i]] = pr } diff --git a/pkg/raft/rawnode_test.go b/pkg/raft/rawnode_test.go index 7b218ca695e0..6e345eabcb48 100644 --- a/pkg/raft/rawnode_test.go +++ b/pkg/raft/rawnode_test.go @@ -694,7 +694,7 @@ func TestRawNodeStatus(t *testing.T) { require.Equal(t, StateLeader, status.RaftState) require.Equal(t, *rn.raft.trk.Progress[1], status.Progress[1]) - expCfg := tracker.Config{Voters: quorum.JointConfig{ + expCfg := quorum.Config{Voters: quorum.JointConfig{ quorum.MajorityConfig{1: {}}, nil, }} diff --git a/pkg/raft/status.go b/pkg/raft/status.go index 41e7946e6bd1..6acd9be0f9f1 100644 --- a/pkg/raft/status.go +++ b/pkg/raft/status.go @@ -20,6 +20,7 @@ package raft import ( "fmt" + "github.com/cockroachdb/cockroach/pkg/raft/quorum" pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb" "github.com/cockroachdb/cockroach/pkg/raft/tracker" "github.com/cockroachdb/cockroach/pkg/util/hlc" @@ -29,7 +30,7 @@ import ( // The Progress is only populated on the leader. type Status struct { BasicStatus - Config tracker.Config + Config quorum.Config Progress map[pb.PeerID]tracker.Progress LeadSupportUntil hlc.Timestamp } @@ -126,7 +127,7 @@ func getStatus(r *raft) Status { if s.RaftState == StateLeader { s.Progress = getProgressCopy(r) } - s.Config = r.trk.Config.Clone() + s.Config = r.config.Clone() // NOTE: we assign to LeadSupportUntil even if RaftState is not currently // StateLeader. The replica may have been the leader and stepped down to a // follower before its lead support ran out. diff --git a/pkg/raft/tracker/progress.go b/pkg/raft/tracker/progress.go index 0d35738d8ce8..bb0c896789ca 100644 --- a/pkg/raft/tracker/progress.go +++ b/pkg/raft/tracker/progress.go @@ -356,6 +356,11 @@ func (pr *Progress) String() string { // ProgressMap is a map of *Progress. type ProgressMap map[pb.PeerID]*Progress +// MakeEmptyProgressMap constructs and returns an empty ProgressMap. +func MakeEmptyProgressMap() ProgressMap { + return make(ProgressMap) +} + // String prints the ProgressMap in sorted key order, one Progress per line. func (m ProgressMap) String() string { ids := make([]pb.PeerID, 0, len(m)) diff --git a/pkg/raft/tracker/tracker.go b/pkg/raft/tracker/tracker.go index 4fa55921f7de..f21e769fbd5f 100644 --- a/pkg/raft/tracker/tracker.go +++ b/pkg/raft/tracker/tracker.go @@ -18,153 +18,34 @@ package tracker import ( - "fmt" "slices" "sort" - "strings" "github.com/cockroachdb/cockroach/pkg/raft/quorum" pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb" ) -// Config reflects the configuration tracked in a ProgressTracker. -type Config struct { - Voters quorum.JointConfig - // AutoLeave is true if the configuration is joint and a transition to the - // incoming configuration should be carried out automatically by Raft when - // this is possible. If false, the configuration will be joint until the - // application initiates the transition manually. - AutoLeave bool - // Learners is a set of IDs corresponding to the learners active in the - // current configuration. - // - // Invariant: Learners and Voters does not intersect, i.e. if a peer is in - // either half of the joint config, it can't be a learner; if it is a - // learner it can't be in either half of the joint config. This invariant - // simplifies the implementation since it allows peers to have clarity about - // its current role without taking into account joint consensus. - Learners map[pb.PeerID]struct{} - // When we turn a voter into a learner during a joint consensus transition, - // we cannot add the learner directly when entering the joint state. This is - // because this would violate the invariant that the intersection of - // voters and learners is empty. For example, assume a Voter is removed and - // immediately re-added as a learner (or in other words, it is demoted): - // - // Initially, the configuration will be - // - // voters: {1 2 3} - // learners: {} - // - // and we want to demote 3. Entering the joint configuration, we naively get - // - // voters: {1 2} & {1 2 3} - // learners: {3} - // - // but this violates the invariant (3 is both voter and learner). Instead, - // we get - // - // voters: {1 2} & {1 2 3} - // learners: {} - // next_learners: {3} - // - // Where 3 is now still purely a voter, but we are remembering the intention - // to make it a learner upon transitioning into the final configuration: - // - // voters: {1 2} - // learners: {3} - // next_learners: {} - // - // Note that next_learners is not used while adding a learner that is not - // also a voter in the joint config. In this case, the learner is added - // right away when entering the joint configuration, so that it is caught up - // as soon as possible. - LearnersNext map[pb.PeerID]struct{} -} - -func (c Config) String() string { - var buf strings.Builder - fmt.Fprintf(&buf, "voters=%s", c.Voters) - if c.Learners != nil { - fmt.Fprintf(&buf, " learners=%s", quorum.MajorityConfig(c.Learners).String()) - } - if c.LearnersNext != nil { - fmt.Fprintf(&buf, " learners_next=%s", quorum.MajorityConfig(c.LearnersNext).String()) - } - if c.AutoLeave { - fmt.Fprint(&buf, " autoleave") - } - return buf.String() -} - -// Clone returns a copy of the Config that shares no memory with the original. -func (c *Config) Clone() Config { - clone := func(m map[pb.PeerID]struct{}) map[pb.PeerID]struct{} { - if m == nil { - return nil - } - mm := make(map[pb.PeerID]struct{}, len(m)) - for k := range m { - mm[k] = struct{}{} - } - return mm - } - return Config{ - Voters: quorum.JointConfig{clone(c.Voters[0]), clone(c.Voters[1])}, - Learners: clone(c.Learners), - LearnersNext: clone(c.LearnersNext), - } -} - -// ProgressTracker tracks the currently active configuration and the information -// known about the nodes and learners in it. In particular, it tracks the match -// index for each peer which in turn allows reasoning about the committed index. +// ProgressTracker tracks the progress made by each peer in the currently active +// configuration. In particular, it tracks the match index for each peer, which +// in-turn allows for reasoning about the committed index. type ProgressTracker struct { - Config + Config *quorum.Config Progress ProgressMap Votes map[pb.PeerID]bool - - MaxInflight int - MaxInflightBytes uint64 } // MakeProgressTracker initializes a ProgressTracker. -func MakeProgressTracker(maxInflight int, maxBytes uint64) ProgressTracker { +func MakeProgressTracker(config *quorum.Config, progressMap ProgressMap) ProgressTracker { p := ProgressTracker{ - MaxInflight: maxInflight, - MaxInflightBytes: maxBytes, - Config: Config{ - Voters: quorum.JointConfig{ - quorum.MajorityConfig{}, - nil, // only populated when used - }, - Learners: nil, // only populated when used - LearnersNext: nil, // only populated when used - }, + Config: config, + Progress: progressMap, Votes: map[pb.PeerID]bool{}, - Progress: map[pb.PeerID]*Progress{}, } return p } -// ConfState returns a ConfState representing the active configuration. -func (p *ProgressTracker) ConfState() pb.ConfState { - return pb.ConfState{ - Voters: p.Voters[0].Slice(), - VotersOutgoing: p.Voters[1].Slice(), - Learners: quorum.MajorityConfig(p.Learners).Slice(), - LearnersNext: quorum.MajorityConfig(p.LearnersNext).Slice(), - AutoLeave: p.AutoLeave, - } -} - -// IsSingleton returns true if (and only if) there is only one voting member -// (i.e. the leader) in the current configuration. -func (p *ProgressTracker) IsSingleton() bool { - return len(p.Voters[0]) == 1 && len(p.Voters[1]) == 0 -} - type matchAckIndexer map[pb.PeerID]*Progress var _ quorum.AckedIndexer = matchAckIndexer(nil) @@ -181,7 +62,7 @@ func (l matchAckIndexer) AckedIndex(id pb.PeerID) (quorum.Index, bool) { // Committed returns the largest log index known to be committed based on what // the voting members of the group have acknowledged. func (p *ProgressTracker) Committed() uint64 { - return uint64(p.Voters.CommittedIndex(matchAckIndexer(p.Progress))) + return uint64(p.Config.Voters.CommittedIndex(matchAckIndexer(p.Progress))) } // Visit invokes the supplied closure for all tracked progresses in stable order. @@ -218,12 +99,12 @@ func (p *ProgressTracker) QuorumActive() bool { votes[id] = pr.RecentActive }) - return p.Voters.VoteResult(votes) == quorum.VoteWon + return p.Config.Voters.VoteResult(votes) == quorum.VoteWon } // VoterNodes returns a sorted slice of voters. func (p *ProgressTracker) VoterNodes() []pb.PeerID { - m := p.Voters.IDs() + m := p.Config.Voters.IDs() nodes := make([]pb.PeerID, 0, len(m)) for id := range m { nodes = append(nodes, id) @@ -234,11 +115,11 @@ func (p *ProgressTracker) VoterNodes() []pb.PeerID { // LearnerNodes returns a sorted slice of learners. func (p *ProgressTracker) LearnerNodes() []pb.PeerID { - if len(p.Learners) == 0 { + if len(p.Config.Learners) == 0 { return nil } - nodes := make([]pb.PeerID, 0, len(p.Learners)) - for id := range p.Learners { + nodes := make([]pb.PeerID, 0, len(p.Config.Learners)) + for id := range p.Config.Learners { nodes = append(nodes, id) } sort.Slice(nodes, func(i, j int) bool { return nodes[i] < nodes[j] }) @@ -280,6 +161,6 @@ func (p *ProgressTracker) TallyVotes() (granted int, rejected int, _ quorum.Vote rejected++ } } - result := p.Voters.VoteResult(p.Votes) + result := p.Config.Voters.VoteResult(p.Votes) return granted, rejected, result } diff --git a/pkg/roachpb/metadata_replicas_test.go b/pkg/roachpb/metadata_replicas_test.go index 3ad76e953857..479766fcbf3b 100644 --- a/pkg/roachpb/metadata_replicas_test.go +++ b/pkg/roachpb/metadata_replicas_test.go @@ -338,7 +338,12 @@ func TestReplicaDescriptorsCanMakeProgressRandom(t *testing.T) { raftCanMakeProgress, skip := func() (res bool, skip bool) { cfg, _, err := confchange.Restore( - confchange.Changer{Tracker: tracker.MakeProgressTracker(1, 0)}, + confchange.Changer{ + Config: quorum.MakeEmptyConfig(), + ProgressMap: tracker.MakeEmptyProgressMap(), + MaxInflight: 1, + MaxInflightBytes: 0, + }, rng.ConfState(), ) if err != nil {