diff --git a/pkg/kv/kvserver/client_atomic_membership_change_test.go b/pkg/kv/kvserver/client_atomic_membership_change_test.go index 6410ec78ef3f..ba0ce5bc6700 100644 --- a/pkg/kv/kvserver/client_atomic_membership_change_test.go +++ b/pkg/kv/kvserver/client_atomic_membership_change_test.go @@ -89,7 +89,7 @@ func TestAtomicReplicationChange(t *testing.T) { // a hair earlier. cfg := quorum.MakeEmptyConfig() cfg, _, err = confchange.Restore(confchange.Changer{ - ProgressMap: tracker.MakeProgressTracker(&cfg).Progress, + ProgressMap: tracker.MakeEmptyProgressMap(), Config: cfg, MaxInflight: 1, MaxInflightBytes: 0, diff --git a/pkg/raft/confchange/datadriven_test.go b/pkg/raft/confchange/datadriven_test.go index 9bf2cd0d151a..2aeef89f6200 100644 --- a/pkg/raft/confchange/datadriven_test.go +++ b/pkg/raft/confchange/datadriven_test.go @@ -32,10 +32,9 @@ import ( func TestConfChangeDataDriven(t *testing.T) { datadriven.Walk(t, "testdata", func(t *testing.T, path string) { - cfg := quorum.MakeEmptyConfig() c := Changer{ - Config: cfg, - ProgressMap: tracker.MakeProgressTracker(&cfg).Progress, + Config: quorum.MakeEmptyConfig(), + ProgressMap: tracker.MakeEmptyProgressMap(), MaxInflight: 10, MaxInflightBytes: 0, LastIndex: 0, // incremented in this test with each cmd diff --git a/pkg/raft/confchange/quick_test.go b/pkg/raft/confchange/quick_test.go index 72ca0b4aea28..30e0ea496dbc 100644 --- a/pkg/raft/confchange/quick_test.go +++ b/pkg/raft/confchange/quick_test.go @@ -93,10 +93,9 @@ func TestConfChangeQuick(t *testing.T) { wrapper := func(invoke testFunc) func(setup initialChanges, ccs confChanges) (*Changer, error) { return func(setup initialChanges, ccs confChanges) (*Changer, error) { - cfg := quorum.MakeEmptyConfig() c := &Changer{ - Config: cfg, - ProgressMap: tracker.MakeProgressTracker(&cfg).Progress, + Config: quorum.MakeEmptyConfig(), + ProgressMap: tracker.MakeEmptyProgressMap(), MaxInflight: 10, MaxInflightBytes: 0, LastIndex: 10, diff --git a/pkg/raft/confchange/restore_test.go b/pkg/raft/confchange/restore_test.go index 58e4dbc30ceb..0114a787867d 100644 --- a/pkg/raft/confchange/restore_test.go +++ b/pkg/raft/confchange/restore_test.go @@ -89,10 +89,9 @@ func TestRestore(t *testing.T) { cfg := quick.Config{MaxCount: 1000} f := func(cs pb.ConfState) bool { - cfg := quorum.MakeEmptyConfig() chg := Changer{ - Config: cfg, - ProgressMap: tracker.MakeProgressTracker(&cfg).Progress, + Config: quorum.MakeEmptyConfig(), + ProgressMap: tracker.MakeEmptyProgressMap(), LastIndex: 10, } cfg, progressMap, err := Restore(chg, cs) diff --git a/pkg/raft/raft.go b/pkg/raft/raft.go index 348c8ba1311c..bf6555177b41 100644 --- a/pkg/raft/raft.go +++ b/pkg/raft/raft.go @@ -420,7 +420,6 @@ func newRaft(c *Config) *raft { raftLog: raftlog, maxMsgSize: entryEncodingSize(c.MaxSizePerMsg), maxUncommittedSize: entryPayloadSize(c.MaxUncommittedEntriesSize), - config: quorum.MakeEmptyConfig(), electionTimeout: c.ElectionTick, heartbeatTimeout: c.HeartbeatTick, logger: c.Logger, @@ -435,11 +434,9 @@ func newRaft(c *Config) *raft { } lastID := r.raftLog.lastEntryID() - r.trk = tracker.MakeProgressTracker(&r.config) - - cfg, trk, err := confchange.Restore(confchange.Changer{ - Config: r.config, - ProgressMap: r.trk.Progress, + cfg, progressMap, err := confchange.Restore(confchange.Changer{ + Config: quorum.MakeEmptyConfig(), + ProgressMap: tracker.MakeEmptyProgressMap(), MaxInflight: r.maxInflight, MaxInflightBytes: r.maxInflightBytes, LastIndex: lastID.index, @@ -447,7 +444,7 @@ func newRaft(c *Config) *raft { 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) @@ -1908,12 +1905,10 @@ func (r *raft) restore(s snapshot) bool { return false } - // Reset the configuration and add the (potentially updated) peers in anew. - r.config = quorum.MakeEmptyConfig() - r.trk = tracker.MakeProgressTracker(&r.config) - cfg, trk, err := confchange.Restore(confchange.Changer{ - Config: r.config, - ProgressMap: r.trk.Progress, + 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(), @@ -1925,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]", @@ -1941,7 +1936,7 @@ func (r *raft) promotable() bool { } func (r *raft) applyConfChange(cc pb.ConfChangeV2) pb.ConfState { - cfg, trk, err := func() (quorum.Config, tracker.ProgressMap, error) { + cfg, progressMap, err := func() (quorum.Config, tracker.ProgressMap, error) { changer := confchange.Changer{ Config: r.config, ProgressMap: r.trk.Progress, @@ -1962,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 @@ -1971,9 +1966,9 @@ 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 quorum.Config, trk tracker.ProgressMap) pb.ConfState { +func (r *raft) switchToConfig(cfg quorum.Config, progressMap tracker.ProgressMap) pb.ConfState { r.config = cfg - r.trk.Progress = trk + r.trk = tracker.MakeProgressTracker(&r.config, progressMap) r.logger.Infof("%x switched to configuration %s", r.id, r.config) cs := r.config.ConfState() diff --git a/pkg/raft/raft_test.go b/pkg/raft/raft_test.go index f1ab34cc93bd..b4e30cf718f8 100644 --- a/pkg/raft/raft_test.go +++ b/pkg/raft/raft_test.go @@ -3750,7 +3750,7 @@ func newNetworkWithConfig(configFunc func(*Config), peers ...stateMachine) *netw learners[i] = true } v.id = id - v.trk = tracker.MakeProgressTracker(&v.config) + v.trk = tracker.MakeProgressTracker(&v.config, tracker.MakeEmptyProgressMap()) if len(learners) > 0 { v.config.Learners = map[pb.PeerID]struct{}{} } 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 4393e65a2f06..f21e769fbd5f 100644 --- a/pkg/raft/tracker/tracker.go +++ b/pkg/raft/tracker/tracker.go @@ -37,11 +37,11 @@ type ProgressTracker struct { } // MakeProgressTracker initializes a ProgressTracker. -func MakeProgressTracker(config *quorum.Config) ProgressTracker { +func MakeProgressTracker(config *quorum.Config, progressMap ProgressMap) ProgressTracker { p := ProgressTracker{ Config: config, + Progress: progressMap, Votes: map[pb.PeerID]bool{}, - Progress: map[pb.PeerID]*Progress{}, } return p } diff --git a/pkg/roachpb/metadata_replicas_test.go b/pkg/roachpb/metadata_replicas_test.go index 2a950629fe7d..479766fcbf3b 100644 --- a/pkg/roachpb/metadata_replicas_test.go +++ b/pkg/roachpb/metadata_replicas_test.go @@ -337,11 +337,10 @@ func TestReplicaDescriptorsCanMakeProgressRandom(t *testing.T) { }) raftCanMakeProgress, skip := func() (res bool, skip bool) { - emptyCfg := quorum.MakeEmptyConfig() cfg, _, err := confchange.Restore( confchange.Changer{ - Config: emptyCfg, - ProgressMap: tracker.MakeProgressTracker(&emptyCfg).Progress, + Config: quorum.MakeEmptyConfig(), + ProgressMap: tracker.MakeEmptyProgressMap(), MaxInflight: 1, MaxInflightBytes: 0, },