Skip to content

Commit

Permalink
etcdserver: support MemberAdd for learner
Browse files Browse the repository at this point in the history
Added IsLearner field to etcdserver internal Member type. Routed
learner MemberAdd request from server API to raft. Apply learner
MemberAdd result to server after the request is passed through Raft.
  • Loading branch information
jingyih committed Mar 19, 2019
1 parent a626ff5 commit 97564fb
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 25 deletions.
6 changes: 4 additions & 2 deletions etcdserver/api/membership/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,12 @@ type RaftCluster struct {
removed map[types.ID]bool
}

// NewClusterFromURLsMap creates a new raft cluster using provided urls map. Currently, it does not support creating
// cluster with raft learner member.
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 := NewMember(name, urls, token, nil, false)
if _, ok := c.members[m.ID]; ok {
return nil, fmt.Errorf("member exists with identical ID %v", m)
}
Expand Down Expand Up @@ -259,7 +261,7 @@ func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error {
return ErrIDRemoved
}
switch cc.Type {
case raftpb.ConfChangeAddNode:
case raftpb.ConfChangeAddNode, raftpb.ConfChangeAddLearnerNode:
if members[id] != nil {
return ErrIDExists
}
Expand Down
23 changes: 23 additions & 0 deletions etcdserver/api/membership/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,29 @@ func TestClusterAddMember(t *testing.T) {
}
}

func TestClusterAddMemberAsLearner(t *testing.T) {
st := mockstore.NewRecorder()
c := newTestCluster(nil)
c.SetStore(st)
c.AddMember(newTestMemberAsLearner(1, nil, "node1", nil))

wactions := []testutil.Action{
{
Name: "Create",
Params: []interface{}{
path.Join(StoreMembersPrefix, "1", "raftAttributes"),
false,
`{"peerURLs":null,"isLearner":true}`,
false,
v2store.TTLOptionSet{ExpireTime: v2store.Permanent},
},
},
}
if g := st.Action(); !reflect.DeepEqual(g, wactions) {
t.Errorf("actions = %v, want %v", g, wactions)
}
}

func TestClusterMembers(t *testing.T) {
cls := &RaftCluster{
members: map[types.ID]*Member{
Expand Down
14 changes: 11 additions & 3 deletions etcdserver/api/membership/member.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ type RaftAttributes struct {
// PeerURLs is the list of peers in the raft cluster.
// TODO(philips): ensure these are URLs
PeerURLs []string `json:"peerURLs"`
// IsLearner indicates if the member is raft learner.
IsLearner bool `json:"isLearner,omitempty"`
}

// Attributes represents all the non-raft related attributes of an etcd member.
Expand All @@ -51,10 +53,13 @@ type Member struct {

// NewMember creates a 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 {
func NewMember(name string, peerURLs types.URLs, clusterName string, now *time.Time, isLearner bool) *Member {
m := &Member{
RaftAttributes: RaftAttributes{PeerURLs: peerURLs.StringSlice()},
Attributes: Attributes{Name: name},
RaftAttributes: RaftAttributes{
PeerURLs: peerURLs.StringSlice(),
IsLearner: isLearner,
},
Attributes: Attributes{Name: name},
}

var b []byte
Expand Down Expand Up @@ -88,6 +93,9 @@ func (m *Member) Clone() *Member {
}
mm := &Member{
ID: m.ID,
RaftAttributes: RaftAttributes{
IsLearner: m.IsLearner,
},
Attributes: Attributes{
Name: m.Name,
},
Expand Down
22 changes: 15 additions & 7 deletions etcdserver/api/membership/member_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil, false), 14544069596553697298},
// Same ID, different name (names shouldn't matter)
{NewMember("memfoo", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298},
{NewMember("memfoo", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil, false), 14544069596553697298},
// Same ID, different Time
{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", timeParse("1984-12-23T15:04:05Z")), 2448790162483548276},
{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", timeParse("1984-12-23T15:04:05Z"), false), 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},
{NewMember("mcm1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "etcd", timeParse("1984-12-23T15:04:05Z"), false), 6973882743191604649},
{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}}, "", timeParse("1984-12-23T15:04:05Z"), false), 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},
{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}, {Scheme: "http", Host: "10.0.0.2:2379"}}, "", nil, false), 16552244735972308939},
{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.2:2379"}, {Scheme: "http", Host: "10.0.0.1:2379"}}, "", nil, false), 16552244735972308939},
}
for i, tt := range tests {
if tt.mem.ID != tt.id {
Expand Down Expand Up @@ -113,3 +113,11 @@ func newTestMember(id uint64, peerURLs []string, name string, clientURLs []strin
Attributes: Attributes{Name: name, ClientURLs: clientURLs},
}
}

func newTestMemberAsLearner(id uint64, peerURLs []string, name string, clientURLs []string) *Member {
return &Member{
ID: types.ID(id),
RaftAttributes: RaftAttributes{PeerURLs: peerURLs, IsLearner: true},
Attributes: Attributes{Name: name, ClientURLs: clientURLs},
}
}
2 changes: 1 addition & 1 deletion etcdserver/api/v2http/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.NewMember("", req.PeerURLs, "", &now, false) // does not support adding learner via v2http
_, err := h.server.AddMember(ctx, *m)
switch {
case err == membership.ErrIDExists || err == membership.ErrPeerURLexists:
Expand Down
11 changes: 8 additions & 3 deletions etcdserver/api/v3rpc/member.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,19 @@ func (cs *ClusterServer) MemberAdd(ctx context.Context, r *pb.MemberAddRequest)
}

now := time.Now()
m := membership.NewMember("", urls, "", &now)
m := membership.NewMember("", urls, "", &now, r.IsLearner)
membs, merr := cs.server.AddMember(ctx, *m)
if merr != nil {
return nil, togRPCError(merr)
}

return &pb.MemberAddResponse{
Header: cs.header(),
Member: &pb.Member{ID: uint64(m.ID), PeerURLs: m.PeerURLs},
Header: cs.header(),
Member: &pb.Member{
ID: uint64(m.ID),
PeerURLs: m.PeerURLs,
IsLearner: m.IsLearner,
},
Members: membersToProtoMembers(membs),
}, nil
}
Expand Down Expand Up @@ -101,6 +105,7 @@ func membersToProtoMembers(membs []*membership.Member) []*pb.Member {
ID: uint64(membs[i].ID),
PeerURLs: membs[i].PeerURLs,
ClientURLs: membs[i].ClientURLs,
IsLearner: membs[i].IsLearner,
}
}
return protoMembs
Expand Down
8 changes: 7 additions & 1 deletion etcdserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1544,6 +1544,7 @@ func (s *EtcdServer) AddMember(ctx context.Context, memb membership.Member) ([]*
return nil, err
}

// TODO: might switch to less strict check when adding raft learner
if s.Cfg.StrictReconfigCheck {
// by default StrictReconfigCheck is enabled; reject new members if unhealthy
if !s.cluster.IsReadyToAddNewMember() {
Expand Down Expand Up @@ -1585,6 +1586,11 @@ func (s *EtcdServer) AddMember(ctx context.Context, memb membership.Member) ([]*
NodeID: uint64(memb.ID),
Context: b,
}

if memb.IsLearner {
cc.Type = raftpb.ConfChangeAddLearnerNode
}

return s.configure(ctx, cc)
}

Expand Down Expand Up @@ -2054,7 +2060,7 @@ func (s *EtcdServer) applyConfChange(cc raftpb.ConfChange, confState *raftpb.Con
lg := s.getLogger()
*confState = *s.r.ApplyConfChange(cc)
switch cc.Type {
case raftpb.ConfChangeAddNode:
case raftpb.ConfChangeAddNode, raftpb.ConfChangeAddLearnerNode:
m := new(membership.Member)
if err := json.Unmarshal(cc.Context, m); err != nil {
if lg != nil {
Expand Down
16 changes: 8 additions & 8 deletions etcdserver/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ func TestApplyConfigChangeUpdatesConsistIndex(t *testing.T) {
if err != nil {
t.Fatal(err)
}
m := membership.NewMember("", urls, "", &now)
m := membership.NewMember("", urls, "", &now, false)
m.ID = types.ID(2)
b, err := json.Marshal(m)
if err != nil {
Expand Down Expand Up @@ -1564,23 +1564,23 @@ func TestGetOtherPeerURLs(t *testing.T) {
}{
{
[]*membership.Member{
membership.NewMember("1", types.MustNewURLs([]string{"http://10.0.0.1:1"}), "a", nil),
membership.NewMember("1", types.MustNewURLs([]string{"http://10.0.0.1:1"}), "a", nil, false),
},
[]string{},
},
{
[]*membership.Member{
membership.NewMember("1", types.MustNewURLs([]string{"http://10.0.0.1:1"}), "a", nil),
membership.NewMember("2", types.MustNewURLs([]string{"http://10.0.0.2:2"}), "a", nil),
membership.NewMember("3", types.MustNewURLs([]string{"http://10.0.0.3:3"}), "a", nil),
membership.NewMember("1", types.MustNewURLs([]string{"http://10.0.0.1:1"}), "a", nil, false),
membership.NewMember("2", types.MustNewURLs([]string{"http://10.0.0.2:2"}), "a", nil, false),
membership.NewMember("3", types.MustNewURLs([]string{"http://10.0.0.3:3"}), "a", nil, false),
},
[]string{"http://10.0.0.2:2", "http://10.0.0.3:3"},
},
{
[]*membership.Member{
membership.NewMember("1", types.MustNewURLs([]string{"http://10.0.0.1:1"}), "a", nil),
membership.NewMember("3", types.MustNewURLs([]string{"http://10.0.0.3:3"}), "a", nil),
membership.NewMember("2", types.MustNewURLs([]string{"http://10.0.0.2:2"}), "a", nil),
membership.NewMember("1", types.MustNewURLs([]string{"http://10.0.0.1:1"}), "a", nil, false),
membership.NewMember("3", types.MustNewURLs([]string{"http://10.0.0.3:3"}), "a", nil, false),
membership.NewMember("2", types.MustNewURLs([]string{"http://10.0.0.2:2"}), "a", nil, false),
},
[]string{"http://10.0.0.2:2", "http://10.0.0.3:3"},
},
Expand Down

0 comments on commit 97564fb

Please sign in to comment.