From 88f5561733eb345793e743bc2cf45f3612a6886c Mon Sep 17 00:00:00 2001 From: Tobias Schottdorf Date: Sat, 20 Jul 2019 23:42:04 +0200 Subject: [PATCH] raft: use ConfChangeSingle internally --- raft/confchange/confchange.go | 14 +++++++------- raft/confchange/datadriven_test.go | 4 ++-- raft/confchange/quick_test.go | 22 +++++++++++----------- raft/raft.go | 5 ++++- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/raft/confchange/confchange.go b/raft/confchange/confchange.go index fd75aedc8..8f1ed0bee 100644 --- a/raft/confchange/confchange.go +++ b/raft/confchange/confchange.go @@ -41,12 +41,12 @@ type Changer struct { // to // (1 2 3)&&(1 2 3). // -// The supplied ConfChanges are then applied to the incoming majority config, +// The supplied changes are then applied to the incoming majority config, // resulting in a joint configuration that in terms of the Raft thesis[1] // (Section 4.3) corresponds to `C_{new,old}`. // // [1]: https://github.com/ongardie/dissertation/blob/master/online-trim.pdf -func (c Changer) EnterJoint(ccs ...pb.ConfChange) (tracker.Config, tracker.ProgressMap, error) { +func (c Changer) EnterJoint(ccs ...pb.ConfChangeSingle) (tracker.Config, tracker.ProgressMap, error) { cfg, prs, err := c.checkAndCopy() if err != nil { return c.err(err) @@ -129,7 +129,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.ConfChange) (tracker.Config, tracker.ProgressMap, error) { +func (c Changer) Simple(ccs ...pb.ConfChangeSingle) (tracker.Config, tracker.ProgressMap, error) { cfg, prs, err := c.checkAndCopy() if err != nil { return c.err(err) @@ -151,14 +151,14 @@ func (c Changer) Simple(ccs ...pb.ConfChange) (tracker.Config, tracker.ProgressM return checkAndReturn(cfg, prs) } -// apply a ConfChange to the configuration. By convention, changes to voters are +// apply a change to the configuration. By convention, changes to voters are // 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, prs tracker.ProgressMap, ccs ...pb.ConfChange) error { +func (c Changer) apply(cfg *tracker.Config, prs tracker.ProgressMap, ccs ...pb.ConfChangeSingle) error { for _, cc := range ccs { if cc.NodeID == 0 { // etcd replaces the NodeID with zero if it decides (downstream of - // raft) to not apply a ConfChange, so we have to have explicit code + // raft) to not apply a change, so we have to have explicit code // here to ignore these. continue } @@ -408,7 +408,7 @@ func outgoingPtr(voters *quorum.JointConfig) *quorum.MajorityConfig { return &vo // Describe prints the type and NodeID of the configuration changes as a // space-delimited string. -func Describe(ccs ...pb.ConfChange) string { +func Describe(ccs ...pb.ConfChangeSingle) string { var buf strings.Builder for _, cc := range ccs { if buf.Len() > 0 { diff --git a/raft/confchange/datadriven_test.go b/raft/confchange/datadriven_test.go index 7d5428f17..dbbb4d95b 100644 --- a/raft/confchange/datadriven_test.go +++ b/raft/confchange/datadriven_test.go @@ -48,7 +48,7 @@ func TestConfChangeDataDriven(t *testing.T) { defer func() { c.LastIndex++ }() - var ccs []pb.ConfChange + var ccs []pb.ConfChangeSingle toks := strings.Split(strings.TrimSpace(d.Input), " ") if toks[0] == "" { toks = nil @@ -57,7 +57,7 @@ func TestConfChangeDataDriven(t *testing.T) { if len(tok) < 2 { return fmt.Sprintf("unknown token %s", tok) } - var cc pb.ConfChange + var cc pb.ConfChangeSingle switch tok[0] { case 'v': cc.Type = pb.ConfChangeAddNode diff --git a/raft/confchange/quick_test.go b/raft/confchange/quick_test.go index 7a4c112ca..f2dc9a635 100644 --- a/raft/confchange/quick_test.go +++ b/raft/confchange/quick_test.go @@ -35,7 +35,7 @@ func TestConfChangeQuick(t *testing.T) { // as intended. const infoCount = 5 - runWithJoint := func(c *Changer, ccs []pb.ConfChange) error { + runWithJoint := func(c *Changer, ccs []pb.ConfChangeSingle) error { cfg, prs, err := c.EnterJoint(ccs...) if err != nil { return err @@ -51,7 +51,7 @@ func TestConfChangeQuick(t *testing.T) { return nil } - runWithSimple := func(c *Changer, ccs []pb.ConfChange) error { + runWithSimple := func(c *Changer, ccs []pb.ConfChangeSingle) error { for _, cc := range ccs { cfg, prs, err := c.Simple(cc) if err != nil { @@ -62,7 +62,7 @@ func TestConfChangeQuick(t *testing.T) { return nil } - type testFunc func(*Changer, []pb.ConfChange) error + type testFunc func(*Changer, []pb.ConfChangeSingle) error wrapper := func(invoke testFunc) func(setup initialChanges, ccs confChanges) (*Changer, error) { return func(setup initialChanges, ccs confChanges) (*Changer, error) { @@ -112,8 +112,8 @@ func TestConfChangeQuick(t *testing.T) { t.Fatal(err) } - t.Error("setup:", Describe(cErr.In[0].([]pb.ConfChange)...)) - t.Error("ccs:", Describe(cErr.In[1].([]pb.ConfChange)...)) + t.Error("setup:", Describe(cErr.In[0].([]pb.ConfChangeSingle)...)) + t.Error("ccs:", Describe(cErr.In[1].([]pb.ConfChangeSingle)...)) t.Errorf("out1: %+v\nout2: %+v", cErr.Out1, cErr.Out2) } @@ -123,13 +123,13 @@ func (confChangeTyp) Generate(rand *rand.Rand, _ int) reflect.Value { return reflect.ValueOf(confChangeTyp(rand.Intn(4))) } -type confChanges []pb.ConfChange +type confChanges []pb.ConfChangeSingle -func genCC(num func() int, id func() uint64, typ func() pb.ConfChangeType) []pb.ConfChange { - var ccs []pb.ConfChange +func genCC(num func() int, id func() uint64, typ func() pb.ConfChangeType) []pb.ConfChangeSingle { + var ccs []pb.ConfChangeSingle n := num() for i := 0; i < n; i++ { - ccs = append(ccs, pb.ConfChange{Type: typ(), NodeID: id()}) + ccs = append(ccs, pb.ConfChangeSingle{Type: typ(), NodeID: id()}) } return ccs } @@ -150,7 +150,7 @@ func (confChanges) Generate(rand *rand.Rand, _ int) reflect.Value { return reflect.ValueOf(genCC(num, id, typ)) } -type initialChanges []pb.ConfChange +type initialChanges []pb.ConfChangeSingle func (initialChanges) Generate(rand *rand.Rand, _ int) reflect.Value { num := func() int { @@ -163,6 +163,6 @@ func (initialChanges) Generate(rand *rand.Rand, _ int) reflect.Value { // NodeID one is special - it's in the initial config and will be a voter // always (this is to avoid uninteresting edge cases where the simple conf // changes can't easily make progress). - ccs := append([]pb.ConfChange{{Type: pb.ConfChangeAddNode, NodeID: 1}}, genCC(num, id, typ)...) + ccs := append([]pb.ConfChangeSingle{{Type: pb.ConfChangeAddNode, NodeID: 1}}, genCC(num, id, typ)...) return reflect.ValueOf(ccs) } diff --git a/raft/raft.go b/raft/raft.go index 0b2c42c59..61ea892eb 100644 --- a/raft/raft.go +++ b/raft/raft.go @@ -1401,7 +1401,10 @@ func (r *raft) applyConfChange(cc pb.ConfChange) pb.ConfState { cfg, prs, err := confchange.Changer{ Tracker: r.prs, LastIndex: r.raftLog.lastIndex(), - }.Simple(cc) + }.Simple(pb.ConfChangeSingle{ + Type: cc.Type, + NodeID: cc.NodeID, + }) if err != nil { panic(err) }