From 8eee8c260e3a57af8bc8c0635c2921ddd5f59885 Mon Sep 17 00:00:00 2001 From: Yicheng Qin Date: Thu, 23 Oct 2014 13:22:51 -0700 Subject: [PATCH] etcdserver: rebase on master and code clean --- etcdserver/cluster.go | 23 ++++++++++++----------- etcdserver/cluster_test.go | 2 +- etcdserver/etcdhttp/http.go | 4 ++-- etcdserver/etcdhttp/http_test.go | 24 +++++++++++++----------- etcdserver/server.go | 4 ++-- 5 files changed, 30 insertions(+), 27 deletions(-) diff --git a/etcdserver/cluster.go b/etcdserver/cluster.go index b3d38ef7d..1b799551d 100644 --- a/etcdserver/cluster.go +++ b/etcdserver/cluster.go @@ -41,6 +41,8 @@ const ( type ClusterInfo interface { ID() uint64 ClientURLs() []string + Members() map[uint64]*Member + Member(id uint64) *Member } // Cluster is a list of Members that belong to the same raft cluster @@ -48,14 +50,15 @@ type Cluster struct { id uint64 name string members map[uint64]*Member - // removed indicates ids of removed members in the cluster so far + // removed contains the ids of removed members in the cluster. + // removed id cannot be reused. removed map[uint64]bool store store.Store } // NewClusterFromString returns Cluster through given clusterName and parsing // members from a sets of names to IPs discovery formatted like: -// mach0=http://1.1.1.1,mach0=http://2.2.2.2,mach0=http://1.1.1.1,mach1=http://2.2.2.2,mach1=http://3.3.3.3 +// mach0=http://1.1.1.1,mach0=http://2.2.2.2,mach1=http://3.3.3.3,mach2=http://4.4.4.4 func NewClusterFromString(name string, cluster string) (*Cluster, error) { c := newCluster(name) @@ -91,7 +94,7 @@ func NewClusterFromStore(name string, st store.Store) *Cluster { for _, n := range e.Node.Nodes { m, err := nodeToMember(n) if err != nil { - log.Panicf("unexpected nodeToMember error: %v", err) + log.Panicf("nodeToMember should never fail: %v", err) } c.members[m.ID] = m } @@ -126,6 +129,8 @@ func (c *Cluster) Member(id uint64) *Member { return c.members[id] } +// MemberByName returns a Member with the given name if exists. +// If more than one member has the given name, it will return one randomly. func (c *Cluster) MemberByName(name string) *Member { for _, m := range c.members { if m.Name == name { @@ -144,7 +149,7 @@ func (c Cluster) MemberIDs() []uint64 { return ids } -func (c *Cluster) IsMemberRemoved(id uint64) bool { +func (c *Cluster) IsIDremoved(id uint64) bool { return c.removed[id] } @@ -197,13 +202,9 @@ func (c *Cluster) genID() { c.id = binary.BigEndian.Uint64(hash[:8]) } -func (c *Cluster) SetID(id uint64) { - c.id = id -} +func (c *Cluster) SetID(id uint64) { c.id = id } -func (c *Cluster) SetStore(st store.Store) { - c.store = st -} +func (c *Cluster) SetStore(st store.Store) { c.store = st } // AddMember puts a new Member into the store. // A Member with a matching id must not exist. @@ -228,7 +229,7 @@ func (c *Cluster) AddMember(m *Member) { } // RemoveMember removes a member from the store. -// The given id MUST exist. +// The given id MUST exist, or the function panics. func (c *Cluster) RemoveMember(id uint64) { if _, err := c.store.Delete(memberStoreKey(id), true, true); err != nil { log.Panicf("delete peer should never fail: %v", err) diff --git a/etcdserver/cluster_test.go b/etcdserver/cluster_test.go index 6c64b7b73..1c3de4840 100644 --- a/etcdserver/cluster_test.go +++ b/etcdserver/cluster_test.go @@ -132,7 +132,7 @@ func TestClusterMember(t *testing.T) { } } -func TestClusterMemberFromName(t *testing.T) { +func TestClusterMemberByName(t *testing.T) { membs := []Member{ newTestMember(1, nil, "node1", nil), newTestMember(2, nil, "node2", nil), diff --git a/etcdserver/etcdhttp/http.go b/etcdserver/etcdhttp/http.go index 99e1b316e..797d2e65a 100644 --- a/etcdserver/etcdhttp/http.go +++ b/etcdserver/etcdhttp/http.go @@ -160,7 +160,7 @@ func (h serverHandler) serveAdminMembers(w http.ResponseWriter, r *http.Request) case "GET": idStr := strings.TrimPrefix(r.URL.Path, adminMembersPrefix) if idStr == "" { - msmap := h.clusterStore.Get().Members() + msmap := h.clusterInfo.Members() ms := make(SortableMemberSlice, 0, len(msmap)) for _, m := range msmap { ms = append(ms, m) @@ -177,7 +177,7 @@ func (h serverHandler) serveAdminMembers(w http.ResponseWriter, r *http.Request) http.Error(w, err.Error(), http.StatusBadRequest) return } - m := h.clusterStore.Get().FindID(id) + m := h.clusterInfo.Member(id) if m == nil { http.Error(w, "member not found", http.StatusNotFound) return diff --git a/etcdserver/etcdhttp/http_test.go b/etcdserver/etcdhttp/http_test.go index 8450a7e2c..c399a2144 100644 --- a/etcdserver/etcdhttp/http_test.go +++ b/etcdserver/etcdhttp/http_test.go @@ -1533,24 +1533,23 @@ func (s *serverRecorder) RemoveMember(_ context.Context, id uint64) error { } func TestServeAdminMembersGet(t *testing.T) { + memb1 := etcdserver.Member{ID: 1, Attributes: etcdserver.Attributes{ClientURLs: []string{"http://localhost:8080"}}} + memb2 := etcdserver.Member{ID: 2, Attributes: etcdserver.Attributes{ClientURLs: []string{"http://localhost:8081"}}} cluster := &fakeCluster{ - members: []etcdserver.Member{ - {ID: 1, Attributes: etcdserver.Attributes{ClientURLs: []string{"http://localhost:8080"}}}, - {ID: 2, Attributes: etcdserver.Attributes{ClientURLs: []string{"http://localhost:8081"}}}, - }, + members: map[uint64]*etcdserver.Member{1: &memb1, 2: &memb2}, } h := &serverHandler{ - server: &serverRecorder{}, - clock: clockwork.NewFakeClock(), - clusterStore: cluster, + server: &serverRecorder{}, + clock: clockwork.NewFakeClock(), + clusterInfo: cluster, } - msb, err := json.Marshal(cluster.members) + msb, err := json.Marshal([]etcdserver.Member{memb1, memb2}) if err != nil { t.Fatal(err) } wms := string(msb) + "\n" - mb, err := json.Marshal(cluster.members[0]) + mb, err := json.Marshal(memb1) if err != nil { t.Fatal(err) } @@ -1747,7 +1746,10 @@ func TestTrimNodeExternPrefix(t *testing.T) { type fakeCluster struct { id uint64 clientURLs []string + members map[uint64]*etcdserver.Member } -func (c *fakeCluster) ID() uint64 { return c.id } -func (c *fakeCluster) ClientURLs() []string { return c.clientURLs } +func (c *fakeCluster) ID() uint64 { return c.id } +func (c *fakeCluster) ClientURLs() []string { return c.clientURLs } +func (c *fakeCluster) Members() map[uint64]*etcdserver.Member { return c.members } +func (c *fakeCluster) Member(id uint64) *etcdserver.Member { return c.members[id] } diff --git a/etcdserver/server.go b/etcdserver/server.go index dea551ba7..2a6976ca8 100644 --- a/etcdserver/server.go +++ b/etcdserver/server.go @@ -266,7 +266,7 @@ func (s *EtcdServer) start() { } func (s *EtcdServer) Process(ctx context.Context, m raftpb.Message) error { - if s.Cluster.IsMemberRemoved(m.From) { + if s.Cluster.IsIDremoved(m.From) { return ErrRemoved } return s.node.Step(ctx, m) @@ -612,7 +612,7 @@ func (s *EtcdServer) applyConfChange(cc raftpb.ConfChange, nodes []uint64) error } func (s *EtcdServer) checkConfChange(cc raftpb.ConfChange, nodes []uint64) error { - if s.Cluster.IsMemberRemoved(cc.NodeID) { + if s.Cluster.IsIDremoved(cc.NodeID) { return ErrIDRemoved } switch cc.Type {