(do not review - look at etcd-io#11002 etcd-io#11003 etcd-io#11004 in…

…stead)

raft/confchange/confchange.go

@@ -62,10 +62,7 @@ func (c Changer) EnterJoint(autoLeave bool, ccs ...pb.ConfChangeSingle) (tracker
 		return c.err(err)
 	}
 	// Clear the outgoing config.
-	{
-		*outgoingPtr(&cfg.Voters) = quorum.MajorityConfig{}
-
-	}
+	*outgoingPtr(&cfg.Voters) = quorum.MajorityConfig{}
 	// Copy incoming to outgoing.
 	for id := range incoming(cfg.Voters) {
 		outgoing(cfg.Voters)[id] = struct{}{}

raft/confchange/datadriven_test.go

@@ -17,8 +17,6 @@ package confchange
 import (
 	"errors"
 	"fmt"
-	"strconv"
-	"strings"
 	"testing"
 
 	"github.com/cockroachdb/datadriven"
@@ -48,39 +46,14 @@ func TestConfChangeDataDriven(t *testing.T) {
 			defer func() {
 				c.LastIndex++
 			}()
-			var ccs []pb.ConfChangeSingle
-			toks := strings.Split(strings.TrimSpace(d.Input), " ")
-			if toks[0] == "" {
-				toks = nil
-			}
-			for _, tok := range toks {
-				if len(tok) < 2 {
-					return fmt.Sprintf("unknown token %s", tok)
-				}
-				var cc pb.ConfChangeSingle
-				switch tok[0] {
-				case 'v':
-					cc.Type = pb.ConfChangeAddNode
-				case 'l':
-					cc.Type = pb.ConfChangeAddLearnerNode
-				case 'r':
-					cc.Type = pb.ConfChangeRemoveNode
-				case 'u':
-					cc.Type = pb.ConfChangeUpdateNode
-				default:
-					return fmt.Sprintf("unknown input: %s", tok)
-				}
-				id, err := strconv.ParseUint(tok[1:], 10, 64)
-				if err != nil {
-					return err.Error()
-				}
-				cc.NodeID = id
-				ccs = append(ccs, cc)
+
+			ccs, err := pb.ConfChangeFromString(d.Input)
+			if err != nil {
+				t.Fatal(err)
 			}
 
 			var cfg tracker.Config
 			var prs tracker.ProgressMap
-			var err error
 			switch d.Cmd {
 			case "simple":
 				cfg, prs, err = c.Simple(ccs...)

raft/confchange/restore.go

@@ -0,0 +1,124 @@
+// Copyright 2019 The etcd Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package confchange
+
+import (
+	pb "go.etcd.io/etcd/raft/raftpb"
+	"go.etcd.io/etcd/raft/tracker"
+)
+
+// toConfChangeSingle translates a conf state into 1) a slice of operations creating
+// first the config that will become the outgoing one, and then the incoming one, and
+// b) another slice that, when applied to the config resulted from 1), represents the
+// ConfState.
+func toConfChangeSingle(cs pb.ConfState) (out []pb.ConfChangeSingle, in []pb.ConfChangeSingle) {
+	for _, id := range cs.VotersOutgoing {
+		// If there are outgoing voters, first add them one by one so that the
+		// (non-joint) config has them all.
+		out = append(out, pb.ConfChangeSingle{
+			Type:   pb.ConfChangeAddNode,
+			NodeID: id,
+		})
+
+	}
+
+	// NB: we're done constructing the outgoing slice, now on to the incoming
+	// one (which will apply on top of the config created by the outgoing slice).
+
+	// First, we'll remove all of the outgoing voters.
+	for _, id := range cs.VotersOutgoing {
+		in = append(in, pb.ConfChangeSingle{
+			Type:   pb.ConfChangeRemoveNode,
+			NodeID: id,
+		})
+	}
+	// Then we'll add the incoming voters and learners.
+	for _, id := range cs.Voters {
+		in = append(in, pb.ConfChangeSingle{
+			Type:   pb.ConfChangeAddNode,
+			NodeID: id,
+		})
+	}
+	for _, id := range cs.Learners {
+		in = append(in, pb.ConfChangeSingle{
+			Type:   pb.ConfChangeAddLearnerNode,
+			NodeID: id,
+		})
+	}
+	// Same for LearnersNext; these are nodes we want to be learners but which
+	// are currently voters in the outgoing config.
+	for _, id := range cs.LearnersNext {
+		in = append(in, pb.ConfChangeSingle{
+			Type:   pb.ConfChangeAddLearnerNode,
+			NodeID: id,
+		})
+	}
+	return out, in
+}
+
+func chain(chg Changer, ops ...func(Changer) (tracker.Config, tracker.ProgressMap, error)) (tracker.Config, tracker.ProgressMap, error) {
+	for _, op := range ops {
+		cfg, prs, err := op(chg)
+		if err != nil {
+			return tracker.Config{}, nil, err
+		}
+		chg.Tracker.Config = cfg
+		chg.Tracker.Progress = prs
+	}
+	return chg.Tracker.Config, chg.Tracker.Progress, nil
+}
+
+// Restore takes a Changer (which must represent an empty configuration), and
+// brings returns the state described by the supplied ConfState.
+//
+// TODO(tbg) it's silly that this takes a Changer. Unravel this by making sure
+// 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) {
+	outgoing, incoming := toConfChangeSingle(cs)
+
+	var ops []func(Changer) (tracker.Config, tracker.ProgressMap, error)
+
+	// First, apply all of the changes of the outgoing config one by one (so that
+	// it temporarily becomes the incoming active config. For example, 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) {
+			return chg.Simple(cc)
+		})
+	}
+
+	if len(outgoing) > 0 {
+		// If we added anything above, enter the joint state, which rotates the
+		// above additions into the outgoing config, and adds the incoming config
+		// in. Continuing the 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.
+		ops = append(ops, func(chg Changer) (tracker.Config, tracker.ProgressMap, error) {
+			return chg.EnterJoint(cs.AutoLeave, incoming...)
+		})
+	} else {
+		// 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) {
+				return chg.Simple(cc)
+			})
+		}
+	}
+
+	return chain(chg, ops...)
+}

raft/confchange/restore_test.go

@@ -0,0 +1,110 @@
+// Copyright 2019 The etcd Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package confchange
+
+import (
+	"math/rand"
+	"reflect"
+	"sort"
+	"testing"
+	"testing/quick"
+
+	pb "go.etcd.io/etcd/raft/raftpb"
+	"go.etcd.io/etcd/raft/tracker"
+)
+
+type rndConfChange pb.ConfState
+
+// Generate creates a random (valid) ConfState for use with quickcheck.
+func (_ rndConfChange) Generate(rand *rand.Rand, _ int) reflect.Value {
+	conv := func(sl []int) []uint64 {
+		out := make([]uint64, len(sl))
+		for i := range sl {
+			out[i] = uint64(sl[i] + 1)
+		}
+		return out
+	}
+	var cs pb.ConfState
+	nVoters := 1 + rand.Intn(5) // at least one voter
+	nLearners := rand.Intn(5)
+	nRemovedVoters := rand.Intn(3)
+
+	ids := conv(rand.Perm(2 * (nVoters + nLearners + nRemovedVoters)))
+
+	cs.Voters = ids[:nVoters]
+	ids = ids[nVoters:]
+
+	if nLearners > 0 {
+		cs.Learners = ids[:nLearners]
+		ids = ids[nLearners:]
+	}
+
+	nOutgoingRetainedVoters := rand.Intn(nVoters)
+	if nOutgoingRetainedVoters > 0 || nRemovedVoters > 0 {
+		cs.VotersOutgoing = append([]uint64(nil), cs.Voters[:nOutgoingRetainedVoters]...)
+		cs.VotersOutgoing = append(cs.VotersOutgoing, ids[:nRemovedVoters]...)
+	}
+	// Next learners must be removed voters (i.e. demotions).
+	if nRemovedVoters > 0 {
+		nLearnersNext := rand.Intn(nRemovedVoters)
+		if nLearnersNext > 0 {
+			cs.LearnersNext = ids[:nLearnersNext]
+		}
+	}
+
+	cs.AutoLeave = len(cs.VotersOutgoing) > 0 && rand.Intn(2) == 1
+	return reflect.ValueOf(rndConfChange(cs))
+}
+
+func TestRestore(t *testing.T) {
+	cfg := quick.Config{MaxCount: 1000}
+
+	f := func(rndCS rndConfChange) bool {
+		cs := pb.ConfState(rndCS)
+		chg := Changer{
+			Tracker:   tracker.MakeProgressTracker(20),
+			LastIndex: 10,
+		}
+		cfg, prs, err := Restore(chg, cs)
+		if err != nil {
+			t.Error(err)
+			return false
+		}
+		chg.Tracker.Config = cfg
+		chg.Tracker.Progress = prs
+
+		for _, sl := range [][]uint64{
+			cs.Voters,
+			cs.Learners,
+			cs.VotersOutgoing,
+			cs.LearnersNext,
+		} {
+			sort.Slice(sl, func(i, j int) bool { return sl[i] < sl[j] })
+		}
+
+		cs2 := chg.Tracker.ConfState()
+		if reflect.DeepEqual(cs, cs2) {
+			return true // success
+		}
+		t.Errorf(`
+before: %+#v
+after:  %+#v`, cs, cs2)
+		return false
+	}
+
+	if err := quick.Check(f, &cfg); err != nil {
+		t.Error(err)
+	}
+}

raft/quorum/majority.go

@@ -112,9 +112,7 @@ func (c MajorityConfig) Slice() []uint64 {
 	return sl
 }
 
-type uint64Slice []uint64
-
-func insertionSort(sl uint64Slice) {
+func insertionSort(sl []uint64) {
 	a, b := 0, len(sl)
 	for i := a + 1; i < b; i++ {
 		for j := i; j > a && sl[j] < sl[j-1]; j-- {
@@ -141,7 +139,7 @@ func (c MajorityConfig) CommittedIndex(l AckedIndexer) Index {
 	// performance is a lesser concern (additionally the performance
 	// implications of an allocation here are far from drastic).
 	var stk [7]uint64
-	srt := uint64Slice(stk[:])
+	srt := []uint64(stk[:])
 
 	if cap(srt) < n {
 		srt = make([]uint64, n)