From aa4cda2f5cc99836db89d2c372212c49ec03c647 Mon Sep 17 00:00:00 2001 From: Jingyi Hu Date: Fri, 26 Apr 2019 18:16:51 -0700 Subject: [PATCH] etcdserver: allow 1 learner in cluster Hard-coded the maximum number of learners to 1. --- clientv3/integration/cluster_test.go | 40 ++++++++++++++++++++++++++ etcdserver/api/membership/cluster.go | 21 +++++++++++--- etcdserver/api/membership/errors.go | 1 + etcdserver/api/v3rpc/rpctypes/error.go | 3 ++ etcdserver/api/v3rpc/util.go | 1 + 5 files changed, 62 insertions(+), 4 deletions(-) diff --git a/clientv3/integration/cluster_test.go b/clientv3/integration/cluster_test.go index 6e01842bf..ebe0b9705 100644 --- a/clientv3/integration/cluster_test.go +++ b/clientv3/integration/cluster_test.go @@ -259,3 +259,43 @@ func TestMemberPromoteForLearner(t *testing.T) { t.Errorf("learner promoted, expect 0 learner, got %d", numberOfLearners) } } + +// TestMaxLearnerInCluster verifies that the maximum number of learners allowed in a cluster is 1 +func TestMaxLearnerInCluster(t *testing.T) { + defer testutil.AfterTest(t) + + // 1. start with a cluster with 3 voting member and 0 learner member + clus := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 3}) + defer clus.Terminate(t) + + // 2. adding a learner member should succeed + resp1, err := clus.Client(0).MemberAddAsLearner(context.Background(), []string{"http://127.0.0.1:1234"}) + if err != nil { + t.Fatalf("failed to add learner member %v", err) + } + numberOfLearners := 0 + for _, m := range resp1.Members { + if m.IsLearner { + numberOfLearners++ + } + } + if numberOfLearners != 1 { + t.Fatalf("Added 1 learner node to cluster, got %d", numberOfLearners) + } + + // 3. cluster has 3 voting member and 1 learner, adding another learner should fail + _, err = clus.Client(0).MemberAddAsLearner(context.Background(), []string{"http://127.0.0.1:2345"}) + if err == nil { + t.Fatalf("expect member add to fail, got no error") + } + expectedErrKeywords := "too many learner members in cluster" + if !strings.Contains(err.Error(), expectedErrKeywords) { + t.Fatalf("expecting error to contain %s, got %s", expectedErrKeywords, err.Error()) + } + + // 4. cluster has 3 voting member and 1 learner, adding a voting member should succeed + _, err = clus.Client(0).MemberAdd(context.Background(), []string{"http://127.0.0.1:3456"}) + if err != nil { + t.Errorf("failed to add member %v", err) + } +} diff --git a/etcdserver/api/membership/cluster.go b/etcdserver/api/membership/cluster.go index c7e626918..3c48bc4e4 100644 --- a/etcdserver/api/membership/cluster.go +++ b/etcdserver/api/membership/cluster.go @@ -40,6 +40,8 @@ import ( "go.uber.org/zap" ) +const maxLearners = 1 + // RaftCluster is a list of Members that belong to the same raft cluster type RaftCluster struct { lg *zap.Logger @@ -292,16 +294,15 @@ func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error { plog.Panicf("unmarshal confChangeContext should never fail: %v", err) } } - // A ConfChangeAddNode to a existing learner node promotes it to a voting member. - if confChangeContext.IsPromote { + + if confChangeContext.IsPromote { // promoting a learner member to voting member if members[id] == nil { return ErrIDNotFound } if !members[id].IsLearner { return ErrMemberNotLearner } - } else { - // add a learner or a follower case + } else { // adding a new member if members[id] != nil { return ErrIDExists } @@ -317,6 +318,18 @@ func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error { return ErrPeerURLexists } } + + if confChangeContext.Member.IsLearner { // the new member is a learner + numLearners := 0 + for _, m := range members { + if m.IsLearner { + numLearners++ + } + } + if numLearners+1 > maxLearners { + return ErrTooManyLearners + } + } } case raftpb.ConfChangeRemoveNode: if members[id] == nil { diff --git a/etcdserver/api/membership/errors.go b/etcdserver/api/membership/errors.go index 6c8b033ca..143068fe6 100644 --- a/etcdserver/api/membership/errors.go +++ b/etcdserver/api/membership/errors.go @@ -27,6 +27,7 @@ var ( ErrPeerURLexists = errors.New("membership: peerURL exists") ErrMemberNotLearner = errors.New("membership: can only promote a learner member") ErrLearnerNotReady = errors.New("membership: can only promote a learner member which is in sync with leader") + ErrTooManyLearners = errors.New("membership: too many learner members in cluster") ) func isKeyNotFound(err error) bool { diff --git a/etcdserver/api/v3rpc/rpctypes/error.go b/etcdserver/api/v3rpc/rpctypes/error.go index 12021daf6..3bbc26b8f 100644 --- a/etcdserver/api/v3rpc/rpctypes/error.go +++ b/etcdserver/api/v3rpc/rpctypes/error.go @@ -42,6 +42,7 @@ var ( ErrGRPCMemberNotFound = status.New(codes.NotFound, "etcdserver: member not found").Err() ErrGRPCMemberNotLearner = status.New(codes.FailedPrecondition, "etcdserver: can only promote a learner member").Err() ErrGRPCLearnerNotReady = status.New(codes.FailedPrecondition, "etcdserver: can only promote a learner member which is in sync with leader").Err() + ErrGRPCTooManyLearners = status.New(codes.FailedPrecondition, "etcdserver: too many learner members in cluster").Err() ErrGRPCRequestTooLarge = status.New(codes.InvalidArgument, "etcdserver: request is too large").Err() ErrGRPCRequestTooManyRequests = status.New(codes.ResourceExhausted, "etcdserver: too many requests").Err() @@ -97,6 +98,7 @@ var ( ErrorDesc(ErrGRPCMemberNotFound): ErrGRPCMemberNotFound, ErrorDesc(ErrGRPCMemberNotLearner): ErrGRPCMemberNotLearner, ErrorDesc(ErrGRPCLearnerNotReady): ErrGRPCLearnerNotReady, + ErrorDesc(ErrGRPCTooManyLearners): ErrGRPCTooManyLearners, ErrorDesc(ErrGRPCRequestTooLarge): ErrGRPCRequestTooLarge, ErrorDesc(ErrGRPCRequestTooManyRequests): ErrGRPCRequestTooManyRequests, @@ -154,6 +156,7 @@ var ( ErrMemberNotFound = Error(ErrGRPCMemberNotFound) ErrMemberNotLearner = Error(ErrGRPCMemberNotLearner) ErrMemberLearnerNotReady = Error(ErrGRPCLearnerNotReady) + ErrTooManyLearners = Error(ErrGRPCTooManyLearners) ErrRequestTooLarge = Error(ErrGRPCRequestTooLarge) ErrTooManyRequests = Error(ErrGRPCRequestTooManyRequests) diff --git a/etcdserver/api/v3rpc/util.go b/etcdserver/api/v3rpc/util.go index b6d8c43c9..d9cf32dcd 100644 --- a/etcdserver/api/v3rpc/util.go +++ b/etcdserver/api/v3rpc/util.go @@ -37,6 +37,7 @@ var toGRPCErrorMap = map[error]error{ membership.ErrPeerURLexists: rpctypes.ErrGRPCPeerURLExist, membership.ErrMemberNotLearner: rpctypes.ErrGRPCMemberNotLearner, membership.ErrLearnerNotReady: rpctypes.ErrGRPCLearnerNotReady, + membership.ErrTooManyLearners: rpctypes.ErrGRPCTooManyLearners, etcdserver.ErrNotEnoughStartedMembers: rpctypes.ErrMemberNotEnoughStarted, mvcc.ErrCompacted: rpctypes.ErrGRPCCompacted,