Skip to content

Commit

Permalink
etcdserver: fix createConfChangeEnts
Browse files Browse the repository at this point in the history
It created a sequence of conf changes that could intermittently cause an
empty set of voters, which Raft asserts against as of etcd-io#10889.

This fixes TestCtlV2BackupSnapshot and TestCtlV2BackupV3Snapshot, see:
etcd-io#10700 (comment)
  • Loading branch information
tbg committed Jul 19, 2019
1 parent 3c5e2f5 commit eb4d9b6
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 20 deletions.
41 changes: 26 additions & 15 deletions etcdserver/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -686,27 +686,18 @@ func getIDs(lg *zap.Logger, snap *raftpb.Snapshot, ents []raftpb.Entry) []uint64
// If `self` is not inside the given ids, it creates a Raft entry to add a
// default member with the given `self`.
func createConfigChangeEnts(lg *zap.Logger, ids []uint64, self uint64, term, index uint64) []raftpb.Entry {
ents := make([]raftpb.Entry, 0)
next := index + 1
found := false
for _, id := range ids {
if id == self {
found = true
continue
}
cc := &raftpb.ConfChange{
Type: raftpb.ConfChangeRemoveNode,
NodeID: id,
}
e := raftpb.Entry{
Type: raftpb.EntryConfChange,
Data: pbutil.MustMarshal(cc),
Term: term,
Index: next,
}
ents = append(ents, e)
next++
}

var ents []raftpb.Entry
next := index + 1

// NB: always add self first, then remove other nodes. Raft will panic if the
// set of voters ever becomes empty.
if !found {
m := membership.Member{
ID: types.ID(self),
Expand All @@ -732,6 +723,26 @@ func createConfigChangeEnts(lg *zap.Logger, ids []uint64, self uint64, term, ind
Index: next,
}
ents = append(ents, e)
next++
}

for _, id := range ids {
if id == self {
continue
}
cc := &raftpb.ConfChange{
Type: raftpb.ConfChangeRemoveNode,
NodeID: id,
}
e := raftpb.Entry{
Type: raftpb.EntryConfChange,
Data: pbutil.MustMarshal(cc),
Term: term,
Index: next,
}
ents = append(ents, e)
next++
}

return ents
}
9 changes: 4 additions & 5 deletions etcdserver/raft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"go.etcd.io/etcd/pkg/types"
"go.etcd.io/etcd/raft"
"go.etcd.io/etcd/raft/raftpb"

"go.uber.org/zap"
)

Expand Down Expand Up @@ -97,7 +96,7 @@ func TestCreateConfigChangeEnts(t *testing.T) {
1,
1, 1,

[]raftpb.Entry{},
nil,
},
{
[]uint64{1, 2},
Expand Down Expand Up @@ -138,9 +137,9 @@ func TestCreateConfigChangeEnts(t *testing.T) {
2, 2,

[]raftpb.Entry{
{Term: 2, Index: 3, Type: raftpb.EntryConfChange, Data: pbutil.MustMarshal(removecc2)},
{Term: 2, Index: 4, Type: raftpb.EntryConfChange, Data: pbutil.MustMarshal(removecc3)},
{Term: 2, Index: 5, Type: raftpb.EntryConfChange, Data: pbutil.MustMarshal(addcc1)},
{Term: 2, Index: 3, Type: raftpb.EntryConfChange, Data: pbutil.MustMarshal(addcc1)},
{Term: 2, Index: 4, Type: raftpb.EntryConfChange, Data: pbutil.MustMarshal(removecc2)},
{Term: 2, Index: 5, Type: raftpb.EntryConfChange, Data: pbutil.MustMarshal(removecc3)},
},
},
}
Expand Down

0 comments on commit eb4d9b6

Please sign in to comment.