etcdserver: add ability to auto-promote learners to voters

It looks as though the etcd team was planning to implement "auto promote" (see etcd-io#10537). This commit attempts to lay the groundwork for auto-promoting learners to voters by adding an --auto-promote flag to the add member command. One reason for having the ability to automatically promote learners to voters is that it would enable implementing the Raft paper's recommendation handle reconfiguration changes by first adding new members as non-voters until they have caught up with the leader.
maxenglander · Jul 10, 2019 · fc4f4d4 · fc4f4d4
1 parent eb7dd97
commit fc4f4d4
Show file tree

Hide file tree

Showing 22 changed files with 739 additions and 375 deletions.
diff --git a/clientv3/cluster.go b/clientv3/cluster.go
@@ -36,8 +36,12 @@ type Cluster interface {
 	// MemberList lists the current cluster membership.
 	MemberList(ctx context.Context) (*MemberListResponse, error)
 
-	// MemberAdd adds a new member into the cluster.
-	MemberAdd(ctx context.Context, peerAddrs []string) (*MemberAddResponse, error)
+	// MemberAddAsAutoPromoting adds a new member as a learner that is
+	// automatically promoted to a node upon catching up with the leader into the cluster.
+	MemberAddAsAutoPromotingNode(ctx context.Context, peerAddrs []string) (*MemberAddResponse, error)
+
+	// MemberAddAsNode adds a new member as a node into the cluster.
+	MemberAddAsNode(ctx context.Context, peerAddrs []string) (*MemberAddResponse, error)
 
 	// MemberAddAsLearner adds a new learner member into the cluster.
 	MemberAddAsLearner(ctx context.Context, peerAddrs []string) (*MemberAddResponse, error)
@@ -73,23 +77,28 @@ func NewClusterFromClusterClient(remote pb.ClusterClient, c *Client) Cluster {
 	return api
 }
 
-func (c *cluster) MemberAdd(ctx context.Context, peerAddrs []string) (*MemberAddResponse, error) {
-	return c.memberAdd(ctx, peerAddrs, false)
+func (c *cluster) MemberAddAsAutoPromotingNode(ctx context.Context, peerAddrs []string) (*MemberAddResponse, error) {
+	return c.memberAdd(ctx, peerAddrs, true, true)
+}
+
+func (c *cluster) MemberAddAsNode(ctx context.Context, peerAddrs []string) (*MemberAddResponse, error) {
+	return c.memberAdd(ctx, peerAddrs, false, false)
 }
 
 func (c *cluster) MemberAddAsLearner(ctx context.Context, peerAddrs []string) (*MemberAddResponse, error) {
-	return c.memberAdd(ctx, peerAddrs, true)
+	return c.memberAdd(ctx, peerAddrs, true, false)
 }
 
-func (c *cluster) memberAdd(ctx context.Context, peerAddrs []string, isLearner bool) (*MemberAddResponse, error) {
+func (c *cluster) memberAdd(ctx context.Context, peerAddrs []string, isLearner bool, autoPromote bool) (*MemberAddResponse, error) {
 	// fail-fast before panic in rafthttp
 	if _, err := types.NewURLs(peerAddrs); err != nil {
 		return nil, err
 	}
 
 	r := &pb.MemberAddRequest{
-		PeerURLs:  peerAddrs,
-		IsLearner: isLearner,
+		PeerURLs:    peerAddrs,
+		IsLearner:   isLearner,
+		AutoPromote: autoPromote,
 	}
 	resp, err := c.remote.MemberAdd(ctx, r, c.callOpts...)
 	if err != nil {

diff --git a/clientv3/example_cluster_test.go b/clientv3/example_cluster_test.go
@@ -51,7 +51,7 @@ func ExampleCluster_memberAdd() {
 	defer cli.Close()
 
 	peerURLs := endpoints[2:]
-	mresp, err := cli.MemberAdd(context.Background(), peerURLs)
+	mresp, err := cli.MemberAddAsNode(context.Background(), peerURLs)
 	if err != nil {
 		log.Fatal(err)
 	}

diff --git a/clientv3/integration/cluster_test.go b/clientv3/integration/cluster_test.go
@@ -54,7 +54,7 @@ func TestMemberAdd(t *testing.T) {
 	capi := clus.RandClient()
 
 	urls := []string{"http://127.0.0.1:1234"}
-	resp, err := capi.MemberAdd(context.Background(), urls)
+	resp, err := capi.MemberAddAsNode(context.Background(), urls)
 	if err != nil {
 		t.Fatalf("failed to add member %v", err)
 	}
@@ -78,7 +78,7 @@ func TestMemberAddWithExistingURLs(t *testing.T) {
 	}
 
 	existingURL := resp.Members[0].PeerURLs[0]
-	_, err = capi.MemberAdd(context.Background(), []string{existingURL})
+	_, err = capi.MemberAddAsNode(context.Background(), []string{existingURL})
 	expectedErrKeywords := "Peer URLs already exists"
 	if err == nil {
 		t.Fatalf("expecting add member to fail, got no error")
@@ -176,7 +176,7 @@ func TestMemberAddUpdateWrongURLs(t *testing.T) {
 		{"localhost:1234"},
 	}
 	for i := range tt {
-		_, err := capi.MemberAdd(context.Background(), tt[i])
+		_, err := capi.MemberAddAsNode(context.Background(), tt[i])
 		if err == nil {
 			t.Errorf("#%d: MemberAdd err = nil, but error", i)
 		}
@@ -412,7 +412,7 @@ func TestMaxLearnerInCluster(t *testing.T) {
 	}
 
 	// 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"})
+	_, err = clus.Client(0).MemberAddAsNode(context.Background(), []string{"http://127.0.0.1:3456"})
 	if err != nil {
 		t.Errorf("failed to add member %v", err)
 	}

diff --git a/clientv3/snapshot/member_test.go b/clientv3/snapshot/member_test.go
@@ -55,7 +55,7 @@ func TestSnapshotV3RestoreMultiMemberAdd(t *testing.T) {
 
 	urls := newEmbedURLs(2)
 	newCURLs, newPURLs := urls[:1], urls[1:]
-	if _, err = cli.MemberAdd(context.Background(), []string{newPURLs[0].String()}); err != nil {
+	if _, err = cli.MemberAddAsNode(context.Background(), []string{newPURLs[0].String()}); err != nil {
 		t.Fatal(err)
 	}
 

diff --git a/etcdctl/ctlv3/command/member_command.go b/etcdctl/ctlv3/command/member_command.go
@@ -27,6 +27,7 @@ import (
 var (
 	memberPeerURLs string
 	isLearner      bool
+	autoPromote    bool
 )
 
 // NewMemberCommand returns the cobra command for "member".
@@ -56,6 +57,7 @@ func NewMemberAddCommand() *cobra.Command {
 
 	cc.Flags().StringVar(&memberPeerURLs, "peer-urls", "", "comma separated peer URLs for the new member.")
 	cc.Flags().BoolVar(&isLearner, "learner", false, "indicates if the new member is raft learner")
+	cc.Flags().BoolVar(&autoPromote, "auto-promote", false, "indicates if the new learner member will be auto-promoted to voter")
 
 	return cc
 }
@@ -143,9 +145,13 @@ func memberAddCommandFunc(cmd *cobra.Command, args []string) {
 		err  error
 	)
 	if isLearner {
-		resp, err = cli.MemberAddAsLearner(ctx, urls)
+		if autoPromote {
+			resp, err = cli.MemberAddAsAutoPromotingNode(ctx, urls)
+		} else {
+			resp, err = cli.MemberAddAsLearner(ctx, urls)
+		}
 	} else {
-		resp, err = cli.MemberAdd(ctx, urls)
+		resp, err = cli.MemberAddAsNode(ctx, urls)
 	}
 	cancel()
 	if err != nil {

diff --git a/etcdserver/api/membership/cluster.go b/etcdserver/api/membership/cluster.go
@@ -75,7 +75,7 @@ type ConfigChangeContext struct {
 func NewClusterFromURLsMap(lg *zap.Logger, token string, urlsmap types.URLsMap) (*RaftCluster, error) {
 	c := NewCluster(lg, token)
 	for name, urls := range urlsmap {
-		m := NewMember(name, urls, token, nil)
+		m := NewMemberAsNode(name, urls, token, nil)
 		if _, ok := c.members[m.ID]; ok {
 			return nil, fmt.Errorf("member exists with identical ID %v", m)
 		}
@@ -285,7 +285,7 @@ func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error {
 		return ErrIDRemoved
 	}
 	switch cc.Type {
-	case raftpb.ConfChangeAddNode, raftpb.ConfChangeAddLearnerNode:
+	case raftpb.ConfChangeAddAutoPromotingNode, raftpb.ConfChangeAddNode, raftpb.ConfChangeAddLearnerNode:
 		confChangeContext := new(ConfigChangeContext)
 		if err := json.Unmarshal(cc.Context, confChangeContext); err != nil {
 			if c.lg != nil {
@@ -486,6 +486,7 @@ func (c *RaftCluster) PromoteMember(id types.ID) {
 	defer c.Unlock()
 
 	c.members[id].RaftAttributes.IsLearner = false
+	c.members[id].RaftAttributes.AutoPromote = false
 	if c.v2store != nil {
 		mustUpdateMemberInStore(c.v2store, c.members[id])
 	}

diff --git a/etcdserver/api/membership/member.go b/etcdserver/api/membership/member.go
@@ -37,6 +37,9 @@ type RaftAttributes struct {
 	PeerURLs []string `json:"peerURLs"`
 	// IsLearner indicates if the member is raft learner.
 	IsLearner bool `json:"isLearner,omitempty"`
+	// AutoPromote indicates whether the learner should be
+	// promoted to a voter upon catching up with leader.
+	AutoPromote bool `json:"autoPromote,omitempty"`
 }
 
 // Attributes represents all the non-raft related attributes of an etcd member.
@@ -51,23 +54,32 @@ type Member struct {
 	Attributes
 }
 
-// NewMember creates a Member without an ID and generates one based on the
+// NewMember creates a node Member without an ID and generates one based on the
 // cluster name, peer URLs, and time. This is used for bootstrapping/adding new member.
-func NewMember(name string, peerURLs types.URLs, clusterName string, now *time.Time) *Member {
-	return newMember(name, peerURLs, clusterName, now, false)
+func NewMemberAsNode(name string, peerURLs types.URLs, clusterName string, now *time.Time) *Member {
+	return newMember(name, peerURLs, clusterName, now, false /* isLearner */, false /* autoPromote */)
+}
+
+// NewMemberAsAutoPromotingNode creates a learner Member without an ID and generates one
+// based on the cluster name, peer URLs, and time. This is used for bootstrapping/adding
+// new member. Auto-promoting learner members are automatically promoted to nodes upon
+// catching up with the master.
+func NewMemberAsAutoPromotingNode(name string, peerURLs types.URLs, clusterName string, now *time.Time) *Member {
+	return newMember(name, peerURLs, clusterName, now, true /* isLearner */, true /* autoPromote */)
 }
 
 // NewMemberAsLearner creates a learner Member without an ID and generates one based on the
 // cluster name, peer URLs, and time. This is used for adding new learner member.
 func NewMemberAsLearner(name string, peerURLs types.URLs, clusterName string, now *time.Time) *Member {
-	return newMember(name, peerURLs, clusterName, now, true)
+	return newMember(name, peerURLs, clusterName, now, true /* isLearner */, false /* autoPromote */)
 }
 
-func newMember(name string, peerURLs types.URLs, clusterName string, now *time.Time, isLearner bool) *Member {
+func newMember(name string, peerURLs types.URLs, clusterName string, now *time.Time, isLearner bool, autoPromote bool) *Member {
 	m := &Member{
 		RaftAttributes: RaftAttributes{
-			PeerURLs:  peerURLs.StringSlice(),
-			IsLearner: isLearner,
+			PeerURLs:    peerURLs.StringSlice(),
+			IsLearner:   isLearner,
+			AutoPromote: autoPromote,
 		},
 		Attributes: Attributes{Name: name},
 	}
@@ -104,7 +116,8 @@ func (m *Member) Clone() *Member {
 	mm := &Member{
 		ID: m.ID,
 		RaftAttributes: RaftAttributes{
-			IsLearner: m.IsLearner,
+			IsLearner:   m.IsLearner,
+			AutoPromote: m.AutoPromote,
 		},
 		Attributes: Attributes{
 			Name: m.Name,

diff --git a/etcdserver/api/membership/member_test.go b/etcdserver/api/membership/member_test.go
@@ -36,17 +36,17 @@ func TestMemberTime(t *testing.T) {
 		mem *Member
 		id  types.ID
 	}{
-		{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298},
+		{NewMemberAsNode("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298},
 		// Same ID, different name (names shouldn't matter)
-		{NewMember("memfoo", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298},
+		{NewMemberAsNode("memfoo", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298},
 		// Same ID, different Time
-		{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", timeParse("1984-12-23T15:04:05Z")), 2448790162483548276},
+		{NewMemberAsNode("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", timeParse("1984-12-23T15:04:05Z")), 2448790162483548276},
 		// Different cluster name
-		{NewMember("mcm1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "etcd", timeParse("1984-12-23T15:04:05Z")), 6973882743191604649},
-		{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}}, "", timeParse("1984-12-23T15:04:05Z")), 1466075294948436910},
+		{NewMemberAsNode("mcm1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "etcd", timeParse("1984-12-23T15:04:05Z")), 6973882743191604649},
+		{NewMemberAsNode("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}}, "", timeParse("1984-12-23T15:04:05Z")), 1466075294948436910},
 		// Order shouldn't matter
-		{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}, {Scheme: "http", Host: "10.0.0.2:2379"}}, "", nil), 16552244735972308939},
-		{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.2:2379"}, {Scheme: "http", Host: "10.0.0.1:2379"}}, "", nil), 16552244735972308939},
+		{NewMemberAsNode("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}, {Scheme: "http", Host: "10.0.0.2:2379"}}, "", nil), 16552244735972308939},
+		{NewMemberAsNode("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.2:2379"}, {Scheme: "http", Host: "10.0.0.1:2379"}}, "", nil), 16552244735972308939},
 	}
 	for i, tt := range tests {
 		if tt.mem.ID != tt.id {

diff --git a/etcdserver/api/v2http/client.go b/etcdserver/api/v2http/client.go
@@ -238,7 +238,7 @@ func (h *membersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		now := h.clock.Now()
-		m := membership.NewMember("", req.PeerURLs, "", &now)
+		m := membership.NewMemberAsNode("", req.PeerURLs, "", &now)
 		_, err := h.server.AddMember(ctx, *m)
 		switch {
 		case err == membership.ErrIDExists || err == membership.ErrPeerURLexists:

diff --git a/etcdserver/api/v2v3/server.go b/etcdserver/api/v2v3/server.go
@@ -64,7 +64,7 @@ func (s *v2v3Server) Leader() types.ID {
 
 func (s *v2v3Server) AddMember(ctx context.Context, memb membership.Member) ([]*membership.Member, error) {
 	// adding member as learner is not supported by V2 Server.
-	resp, err := s.c.MemberAdd(ctx, memb.PeerURLs)
+	resp, err := s.c.MemberAddAsNode(ctx, memb.PeerURLs)
 	if err != nil {
 		return nil, err
 	}

diff --git a/etcdserver/api/v3rpc/member.go b/etcdserver/api/v3rpc/member.go
@@ -47,9 +47,13 @@ func (cs *ClusterServer) MemberAdd(ctx context.Context, r *pb.MemberAddRequest)
 	now := time.Now()
 	var m *membership.Member
 	if r.IsLearner {
-		m = membership.NewMemberAsLearner("", urls, "", &now)
+		if r.AutoPromote {
+			m = membership.NewMemberAsAutoPromotingNode("", urls, "", &now)
+		} else {
+			m = membership.NewMemberAsLearner("", urls, "", &now)
+		}
 	} else {
-		m = membership.NewMember("", urls, "", &now)
+		m = membership.NewMemberAsNode("", urls, "", &now)
 	}
 	membs, merr := cs.server.AddMember(ctx, *m)
 	if merr != nil {