Browse Source

etcdserver: stop if removed along with multiple conf changes

shouldstop would get clobbered when several conf changes are in an apply
Anthony Romano 10 years ago
parent
commit
d7ad721ede
2 changed files with 40 additions and 2 deletions
  1. 2 2
      etcdserver/server.go
  2. 38 0
      etcdserver/server_test.go

+ 2 - 2
etcdserver/server.go

@@ -919,7 +919,6 @@ func (s *EtcdServer) send(ms []raftpb.Message) {
 func (s *EtcdServer) apply(es []raftpb.Entry, confState *raftpb.ConfState) (uint64, bool) {
 func (s *EtcdServer) apply(es []raftpb.Entry, confState *raftpb.ConfState) (uint64, bool) {
 	var applied uint64
 	var applied uint64
 	var shouldstop bool
 	var shouldstop bool
-	var err error
 	for i := range es {
 	for i := range es {
 		e := es[i]
 		e := es[i]
 		// set the consistent index of current executing entry
 		// set the consistent index of current executing entry
@@ -953,7 +952,8 @@ func (s *EtcdServer) apply(es []raftpb.Entry, confState *raftpb.ConfState) (uint
 		case raftpb.EntryConfChange:
 		case raftpb.EntryConfChange:
 			var cc raftpb.ConfChange
 			var cc raftpb.ConfChange
 			pbutil.MustUnmarshal(&cc, e.Data)
 			pbutil.MustUnmarshal(&cc, e.Data)
-			shouldstop, err = s.applyConfChange(cc, confState)
+			removedSelf, err := s.applyConfChange(cc, confState)
+			shouldstop = shouldstop || removedSelf
 			s.w.Trigger(cc.ID, err)
 			s.w.Trigger(cc.ID, err)
 		default:
 		default:
 			plog.Panicf("entry type should be either EntryNormal or EntryConfChange")
 			plog.Panicf("entry type should be either EntryNormal or EntryConfChange")

+ 38 - 0
etcdserver/server_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/coreos/etcd/pkg/pbutil"
 	"github.com/coreos/etcd/pkg/pbutil"
 	"github.com/coreos/etcd/pkg/testutil"
 	"github.com/coreos/etcd/pkg/testutil"
 	"github.com/coreos/etcd/pkg/types"
 	"github.com/coreos/etcd/pkg/types"
+	"github.com/coreos/etcd/pkg/wait"
 	"github.com/coreos/etcd/raft"
 	"github.com/coreos/etcd/raft"
 	"github.com/coreos/etcd/raft/raftpb"
 	"github.com/coreos/etcd/raft/raftpb"
 	"github.com/coreos/etcd/snap"
 	"github.com/coreos/etcd/snap"
@@ -509,6 +510,43 @@ func TestApplyConfChangeShouldStop(t *testing.T) {
 	}
 	}
 }
 }
 
 
+// TestApplyMultiConfChangeShouldStop ensures that apply will return shouldStop
+// if the local member is removed along with other conf updates.
+func TestApplyMultiConfChangeShouldStop(t *testing.T) {
+	cl := newCluster("")
+	cl.SetStore(store.New())
+	for i := 1; i <= 5; i++ {
+		cl.AddMember(&Member{ID: types.ID(i)})
+	}
+	srv := &EtcdServer{
+		id: 2,
+		r: raftNode{
+			Node:      &nodeRecorder{},
+			transport: &nopTransporter{},
+		},
+		cluster: cl,
+		w:       wait.New(),
+	}
+	ents := []raftpb.Entry{}
+	for i := 1; i <= 4; i++ {
+		ent := raftpb.Entry{
+			Term:  1,
+			Index: uint64(i),
+			Type:  raftpb.EntryConfChange,
+			Data: pbutil.MustMarshal(
+				&raftpb.ConfChange{
+					Type:   raftpb.ConfChangeRemoveNode,
+					NodeID: uint64(i)}),
+		}
+		ents = append(ents, ent)
+	}
+
+	_, shouldStop := srv.apply(ents, &raftpb.ConfState{})
+	if shouldStop == false {
+		t.Errorf("shouldStop = %t, want %t", shouldStop, true)
+	}
+}
+
 func TestDoProposal(t *testing.T) {
 func TestDoProposal(t *testing.T) {
 	tests := []pb.Request{
 	tests := []pb.Request{
 		{Method: "POST", ID: 1},
 		{Method: "POST", ID: 1},