From 55c92ad456a41443b431d9aeb27c625d00f08bcc Mon Sep 17 00:00:00 2001 From: Jonathan Boulle Date: Thu, 30 Oct 2014 17:31:14 -0700 Subject: [PATCH] *: create ID type This creates a simple ID type (wrapped around uint64) to provide for standard serialization/deserialization to a string (i.e. base 16 encoded). This replaces strutil so now that package is removed. --- discovery/discovery.go | 8 +-- etcdserver/cluster.go | 38 ++++++------ etcdserver/cluster_test.go | 17 +++--- etcdserver/config.go | 2 +- etcdserver/etcdhttp/client.go | 18 +++--- etcdserver/etcdhttp/client_test.go | 24 ++++---- etcdserver/etcdhttp/http_test.go | 5 +- etcdserver/etcdhttp/peer.go | 12 ++-- etcdserver/etcdhttp/peer_test.go | 3 +- etcdserver/member.go | 17 +++--- etcdserver/member_test.go | 4 +- etcdserver/sender.go | 19 +++--- etcdserver/server.go | 58 ++++++++++--------- etcdserver/server_test.go | 5 +- pkg/strutil/hex.go | 29 ---------- pkg/types/id.go | 43 ++++++++++++++ pkg/{strutil/hex_test.go => types/id_test.go} | 46 +++++++++------ test | 2 +- 18 files changed, 188 insertions(+), 162 deletions(-) delete mode 100644 pkg/strutil/hex.go create mode 100644 pkg/types/id.go rename pkg/{strutil/hex_test.go => types/id_test.go} (57%) diff --git a/discovery/discovery.go b/discovery/discovery.go index 91a475fd6..a4d1089bd 100644 --- a/discovery/discovery.go +++ b/discovery/discovery.go @@ -31,7 +31,7 @@ import ( "github.com/coreos/etcd/Godeps/_workspace/src/github.com/jonboulle/clockwork" "github.com/coreos/etcd/client" - "github.com/coreos/etcd/pkg/strutil" + "github.com/coreos/etcd/pkg/types" ) var ( @@ -57,7 +57,7 @@ type Discoverer interface { type discovery struct { cluster string - id uint64 + id types.ID config string c client.KeysAPI retries uint @@ -95,7 +95,7 @@ func proxyFuncFromEnv() (func(*http.Request) (*url.URL, error), error) { return http.ProxyURL(proxyURL), nil } -func New(durl string, id uint64, config string) (Discoverer, error) { +func New(durl string, id types.ID, config string) (Discoverer, error) { u, err := url.Parse(durl) if err != nil { return nil, err @@ -268,7 +268,7 @@ func (d *discovery) waitNodes(nodes client.Nodes, size int) (client.Nodes, error } func (d *discovery) selfKey() string { - return path.Join("/", d.cluster, strutil.IDAsHex(d.id)) + return path.Join("/", d.cluster, d.id.String()) } func nodesToCluster(ns client.Nodes) string { diff --git a/etcdserver/cluster.go b/etcdserver/cluster.go index ebbd59875..f48246ad1 100644 --- a/etcdserver/cluster.go +++ b/etcdserver/cluster.go @@ -40,21 +40,21 @@ const ( ) type ClusterInfo interface { - ID() uint64 + ID() types.ID ClientURLs() []string // Members returns a slice of members sorted by their ID Members() []*Member - Member(id uint64) *Member + Member(id types.ID) *Member } // Cluster is a list of Members that belong to the same raft cluster type Cluster struct { - id uint64 + id types.ID token string - members map[uint64]*Member + members map[types.ID]*Member // removed contains the ids of removed members in the cluster. // removed id cannot be reused. - removed map[uint64]bool + removed map[types.ID]bool store store.Store } @@ -119,7 +119,7 @@ func NewClusterFromStore(token string, st store.Store) *Cluster { return c } -func NewClusterFromMembers(token string, id uint64, membs []*Member) *Cluster { +func NewClusterFromMembers(token string, id types.ID, membs []*Member) *Cluster { c := newCluster(token) c.id = id for _, m := range membs { @@ -131,12 +131,12 @@ func NewClusterFromMembers(token string, id uint64, membs []*Member) *Cluster { func newCluster(token string) *Cluster { return &Cluster{ token: token, - members: make(map[uint64]*Member), - removed: make(map[uint64]bool), + members: make(map[types.ID]*Member), + removed: make(map[types.ID]bool), } } -func (c Cluster) ID() uint64 { return c.id } +func (c Cluster) ID() types.ID { return c.id } func (c Cluster) Members() []*Member { var sms SortableMemberSlice @@ -153,7 +153,7 @@ func (s SortableMemberSlice) Len() int { return len(s) } func (s SortableMemberSlice) Less(i, j int) bool { return s[i].ID < s[j].ID } func (s SortableMemberSlice) Swap(i, j int) { s[i], s[j] = s[j], s[i] } -func (c *Cluster) Member(id uint64) *Member { +func (c *Cluster) Member(id types.ID) *Member { return c.members[id] } @@ -172,16 +172,16 @@ func (c *Cluster) MemberByName(name string) *Member { return memb } -func (c Cluster) MemberIDs() []uint64 { - var ids []uint64 +func (c Cluster) MemberIDs() []types.ID { + var ids []types.ID for _, m := range c.members { ids = append(ids, m.ID) } - sort.Sort(types.Uint64Slice(ids)) + sort.Sort(types.IDSlice(ids)) return ids } -func (c *Cluster) IsIDRemoved(id uint64) bool { +func (c *Cluster) IsIDRemoved(id types.ID) bool { return c.removed[id] } @@ -244,7 +244,7 @@ func (c *Cluster) ValidateAndAssignIDs(membs []*Member) error { } omembs[i].ID = membs[i].ID } - c.members = make(map[uint64]*Member) + c.members = make(map[types.ID]*Member) for _, m := range omembs { c.members[m.ID] = m } @@ -255,13 +255,13 @@ func (c *Cluster) genID() { mIDs := c.MemberIDs() b := make([]byte, 8*len(mIDs)) for i, id := range mIDs { - binary.BigEndian.PutUint64(b[8*i:], id) + binary.BigEndian.PutUint64(b[8*i:], uint64(id)) } hash := sha1.Sum(b) - c.id = binary.BigEndian.Uint64(hash[:8]) + c.id = types.ID(binary.BigEndian.Uint64(hash[:8])) } -func (c *Cluster) SetID(id uint64) { c.id = id } +func (c *Cluster) SetID(id types.ID) { c.id = id } func (c *Cluster) SetStore(st store.Store) { c.store = st } @@ -289,7 +289,7 @@ func (c *Cluster) AddMember(m *Member) { // RemoveMember removes a member from the store. // The given id MUST exist, or the function panics. -func (c *Cluster) RemoveMember(id uint64) { +func (c *Cluster) RemoveMember(id types.ID) { 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 b430ae235..7d016b196 100644 --- a/etcdserver/cluster_test.go +++ b/etcdserver/cluster_test.go @@ -21,6 +21,7 @@ import ( "reflect" "testing" + "github.com/coreos/etcd/pkg/types" "github.com/coreos/etcd/store" ) @@ -115,7 +116,7 @@ func TestClusterMember(t *testing.T) { newTestMember(2, nil, "node2", nil), } tests := []struct { - id uint64 + id types.ID match bool }{ {1, true}, @@ -165,7 +166,7 @@ func TestClusterMemberIDs(t *testing.T) { newTestMember(4, nil, "", nil), newTestMember(100, nil, "", nil), }) - w := []uint64{1, 4, 100} + w := []types.ID{1, 4, 100} g := c.MemberIDs() if !reflect.DeepEqual(w, g) { t.Errorf("IDs = %+v, want %+v", g, w) @@ -327,7 +328,7 @@ func TestClusterValidateAndAssignIDs(t *testing.T) { tests := []struct { clmembs []Member membs []*Member - wids []uint64 + wids []types.ID }{ { []Member{ @@ -338,7 +339,7 @@ func TestClusterValidateAndAssignIDs(t *testing.T) { newTestMemberp(3, []string{"http://127.0.0.1:2379"}, "", nil), newTestMemberp(4, []string{"http://127.0.0.2:2379"}, "", nil), }, - []uint64{3, 4}, + []types.ID{3, 4}, }, } for i, tt := range tests { @@ -439,7 +440,7 @@ func TestClusterAddMember(t *testing.T) { func TestClusterMembers(t *testing.T) { cls := &Cluster{ - members: map[uint64]*Member{ + members: map[types.ID]*Member{ 1: &Member{ID: 1}, 20: &Member{ID: 20}, 100: &Member{ID: 100}, @@ -461,7 +462,7 @@ func TestClusterMembers(t *testing.T) { func TestClusterString(t *testing.T) { cls := &Cluster{ - members: map[uint64]*Member{ + members: map[types.ID]*Member{ 1: newTestMemberp( 1, []string{"http://1.1.1.1:1111", "http://0.0.0.0:0000"}, @@ -533,7 +534,7 @@ func TestNodeToMember(t *testing.T) { } func newTestCluster(membs []Member) *Cluster { - c := &Cluster{members: make(map[uint64]*Member), removed: make(map[uint64]bool)} + c := &Cluster{members: make(map[types.ID]*Member), removed: make(map[types.ID]bool)} for i, m := range membs { c.members[m.ID] = &membs[i] } @@ -542,7 +543,7 @@ func newTestCluster(membs []Member) *Cluster { func newTestMember(id uint64, peerURLs []string, name string, clientURLs []string) Member { return Member{ - ID: id, + ID: types.ID(id), RaftAttributes: RaftAttributes{PeerURLs: peerURLs}, Attributes: Attributes{Name: name, ClientURLs: clientURLs}, } diff --git a/etcdserver/config.go b/etcdserver/config.go index 04c828b5a..7cdce10c3 100644 --- a/etcdserver/config.go +++ b/etcdserver/config.go @@ -45,7 +45,7 @@ func (c *ServerConfig) VerifyBootstrapConfig() error { if m == nil { return fmt.Errorf("couldn't find local name %s in the initial cluster configuration", c.Name) } - if m.ID == raft.None { + if uint64(m.ID) == raft.None { return fmt.Errorf("cannot use %x as member id", raft.None) } diff --git a/etcdserver/etcdhttp/client.go b/etcdserver/etcdhttp/client.go index 4ac224ce4..8c324cf81 100644 --- a/etcdserver/etcdhttp/client.go +++ b/etcdserver/etcdhttp/client.go @@ -35,7 +35,7 @@ import ( "github.com/coreos/etcd/etcdserver" "github.com/coreos/etcd/etcdserver/etcdhttp/httptypes" "github.com/coreos/etcd/etcdserver/etcdserverpb" - "github.com/coreos/etcd/pkg/strutil" + "github.com/coreos/etcd/pkg/types" "github.com/coreos/etcd/store" "github.com/coreos/etcd/version" ) @@ -96,8 +96,7 @@ func (h *keysHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET", "PUT", "POST", "DELETE") { return } - cid := strconv.FormatUint(h.clusterInfo.ID(), 16) - w.Header().Set("X-Etcd-Cluster-ID", cid) + w.Header().Set("X-Etcd-Cluster-ID", h.clusterInfo.ID().String()) ctx, cancel := context.WithTimeout(context.Background(), h.timeout) defer cancel() @@ -152,8 +151,7 @@ func (h *membersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET", "POST", "DELETE") { return } - cid := strconv.FormatUint(h.clusterInfo.ID(), 16) - w.Header().Set("X-Etcd-Cluster-ID", cid) + w.Header().Set("X-Etcd-Cluster-ID", h.clusterInfo.ID().String()) ctx, cancel := context.WithTimeout(context.Background(), defaultServerTimeout) defer cancel() @@ -189,7 +187,7 @@ func (h *membersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { now := h.clock.Now() m := etcdserver.NewMember("", req.PeerURLs, "", &now) if err := h.server.AddMember(ctx, *m); err != nil { - log.Printf("etcdhttp: error adding node %x: %v", m.ID, err) + log.Printf("etcdhttp: error adding node %s: %v", m.ID, err) writeError(w, err) return } @@ -206,17 +204,17 @@ func (h *membersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { http.Error(w, "Method Not Allowed", http.StatusMethodNotAllowed) return } - id, err := strconv.ParseUint(idStr, 16, 64) + id, err := types.IDFromString(idStr) if err != nil { writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, err.Error())) return } - err = h.server.RemoveMember(ctx, id) + err = h.server.RemoveMember(ctx, uint64(id)) switch { case err == etcdserver.ErrIDNotFound: writeError(w, httptypes.NewHTTPError(http.StatusNotFound, fmt.Sprintf("No such member: %s", idStr))) case err != nil: - log.Printf("etcdhttp: error removing node %x: %v", id, err) + log.Printf("etcdhttp: error removing node %s: %v", id, err) writeError(w, err) default: w.WriteHeader(http.StatusNoContent) @@ -544,7 +542,7 @@ func newMemberCollection(ms []*etcdserver.Member) *httptypes.MemberCollection { func newMember(m *etcdserver.Member) httptypes.Member { tm := httptypes.Member{ - ID: strutil.IDAsHex(m.ID), + ID: m.ID.String(), Name: m.Name, PeerURLs: make([]string, len(m.PeerURLs)), ClientURLs: make([]string, len(m.ClientURLs)), diff --git a/etcdserver/etcdhttp/client_test.go b/etcdserver/etcdhttp/client_test.go index a5abc7403..ab9af2e5e 100644 --- a/etcdserver/etcdhttp/client_test.go +++ b/etcdserver/etcdhttp/client_test.go @@ -27,7 +27,6 @@ import ( "net/url" "path" "reflect" - "strconv" "strings" "testing" "time" @@ -38,6 +37,7 @@ import ( "github.com/coreos/etcd/etcdserver" "github.com/coreos/etcd/etcdserver/etcdhttp/httptypes" "github.com/coreos/etcd/etcdserver/etcdserverpb" + "github.com/coreos/etcd/pkg/types" "github.com/coreos/etcd/raft/raftpb" "github.com/coreos/etcd/store" "github.com/coreos/etcd/version" @@ -591,7 +591,7 @@ func TestServeMembers(t *testing.T) { t.Errorf("#%d: content-type = %s, want %s", i, gct, tt.wct) } gcid := rw.Header().Get("X-Etcd-Cluster-ID") - wcid := strconv.FormatUint(cluster.ID(), 16) + wcid := cluster.ID().String() if gcid != wcid { t.Errorf("#%d: cid = %s, want %s", i, gcid, wcid) } @@ -629,7 +629,7 @@ func TestServeMembersCreate(t *testing.T) { t.Errorf("content-type = %s, want %s", gct, wct) } gcid := rw.Header().Get("X-Etcd-Cluster-ID") - wcid := strconv.FormatUint(h.clusterInfo.ID(), 16) + wcid := h.clusterInfo.ID().String() if gcid != wcid { t.Errorf("cid = %s, want %s", gcid, wcid) } @@ -672,7 +672,7 @@ func TestServeMembersDelete(t *testing.T) { t.Errorf("code=%d, want %d", rw.Code, wcode) } gcid := rw.Header().Get("X-Etcd-Cluster-ID") - wcid := strconv.FormatUint(h.clusterInfo.ID(), 16) + wcid := h.clusterInfo.ID().String() if gcid != wcid { t.Errorf("cid = %s, want %s", gcid, wcid) } @@ -819,7 +819,7 @@ func TestServeMembersFail(t *testing.T) { } if rw.Code != http.StatusMethodNotAllowed { gcid := rw.Header().Get("X-Etcd-Cluster-ID") - wcid := strconv.FormatUint(h.clusterInfo.ID(), 16) + wcid := h.clusterInfo.ID().String() if gcid != wcid { t.Errorf("#%d: cid = %s, want %s", i, gcid, wcid) } @@ -947,10 +947,10 @@ type dummyStats struct { data []byte } -func (ds *dummyStats) SelfStats() []byte { return ds.data } -func (ds *dummyStats) LeaderStats() []byte { return ds.data } -func (ds *dummyStats) StoreStats() []byte { return ds.data } -func (ds *dummyStats) UpdateRecvApp(_ uint64, _ int64) {} +func (ds *dummyStats) SelfStats() []byte { return ds.data } +func (ds *dummyStats) LeaderStats() []byte { return ds.data } +func (ds *dummyStats) StoreStats() []byte { return ds.data } +func (ds *dummyStats) UpdateRecvApp(_ types.ID, _ int64) {} func TestServeSelfStats(t *testing.T) { wb := []byte("some statistics") @@ -1160,7 +1160,7 @@ func TestBadServeKeys(t *testing.T) { } if rw.Code != http.StatusMethodNotAllowed { gcid := rw.Header().Get("X-Etcd-Cluster-ID") - wcid := strconv.FormatUint(h.clusterInfo.ID(), 16) + wcid := h.clusterInfo.ID().String() if gcid != wcid { t.Errorf("#%d: cid = %s, want %s", i, gcid, wcid) } @@ -1204,7 +1204,7 @@ func TestServeKeysEvent(t *testing.T) { t.Errorf("got code=%d, want %d", rw.Code, wcode) } gcid := rw.Header().Get("X-Etcd-Cluster-ID") - wcid := strconv.FormatUint(h.clusterInfo.ID(), 16) + wcid := h.clusterInfo.ID().String() if gcid != wcid { t.Errorf("cid = %s, want %s", gcid, wcid) } @@ -1254,7 +1254,7 @@ func TestServeKeysWatch(t *testing.T) { t.Errorf("got code=%d, want %d", rw.Code, wcode) } gcid := rw.Header().Get("X-Etcd-Cluster-ID") - wcid := strconv.FormatUint(h.clusterInfo.ID(), 16) + wcid := h.clusterInfo.ID().String() if gcid != wcid { t.Errorf("cid = %s, want %s", gcid, wcid) } diff --git a/etcdserver/etcdhttp/http_test.go b/etcdserver/etcdhttp/http_test.go index 4912685ae..439fea9eb 100644 --- a/etcdserver/etcdhttp/http_test.go +++ b/etcdserver/etcdhttp/http_test.go @@ -28,6 +28,7 @@ import ( etcdErr "github.com/coreos/etcd/error" "github.com/coreos/etcd/etcdserver" "github.com/coreos/etcd/etcdserver/etcdserverpb" + "github.com/coreos/etcd/pkg/types" "github.com/coreos/etcd/raft/raftpb" ) @@ -45,7 +46,7 @@ type fakeCluster struct { members map[uint64]*etcdserver.Member } -func (c *fakeCluster) ID() uint64 { return c.id } +func (c *fakeCluster) ID() types.ID { return types.ID(c.id) } func (c *fakeCluster) ClientURLs() []string { return c.clientURLs } func (c *fakeCluster) Members() []*etcdserver.Member { var sms etcdserver.SortableMemberSlice @@ -55,7 +56,7 @@ func (c *fakeCluster) Members() []*etcdserver.Member { sort.Sort(sms) return []*etcdserver.Member(sms) } -func (c *fakeCluster) Member(id uint64) *etcdserver.Member { return c.members[id] } +func (c *fakeCluster) Member(id types.ID) *etcdserver.Member { return c.members[uint64(id)] } // errServer implements the etcd.Server interface for testing. // It returns the given error from any Do/Process/AddMember/RemoveMember calls. diff --git a/etcdserver/etcdhttp/peer.go b/etcdserver/etcdhttp/peer.go index ee5ef5b6c..ec3aa430b 100644 --- a/etcdserver/etcdhttp/peer.go +++ b/etcdserver/etcdhttp/peer.go @@ -21,11 +21,10 @@ import ( "io/ioutil" "log" "net/http" - "strconv" "github.com/coreos/etcd/Godeps/_workspace/src/code.google.com/p/go.net/context" "github.com/coreos/etcd/etcdserver" - "github.com/coreos/etcd/pkg/strutil" + "github.com/coreos/etcd/pkg/types" "github.com/coreos/etcd/raft/raftpb" ) @@ -64,7 +63,7 @@ func (h *raftHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - wcid := strconv.FormatUint(h.clusterInfo.ID(), 16) + wcid := h.clusterInfo.ID().String() w.Header().Set("X-Etcd-Cluster-ID", wcid) gcid := r.Header.Get("X-Etcd-Cluster-ID") @@ -89,7 +88,7 @@ func (h *raftHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if err := h.server.Process(context.TODO(), m); err != nil { switch err { case etcdserver.ErrRemoved: - log.Printf("etcdhttp: reject message from removed member %s", strutil.IDAsHex(m.From)) + log.Printf("etcdhttp: reject message from removed member %s", types.ID(m.From).String()) http.Error(w, "cannot process message from removed member", http.StatusForbidden) default: writeError(w, err) @@ -97,7 +96,7 @@ func (h *raftHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } if m.Type == raftpb.MsgApp { - h.stats.UpdateRecvApp(m.From, r.ContentLength) + h.stats.UpdateRecvApp(types.ID(m.From), r.ContentLength) } w.WriteHeader(http.StatusNoContent) } @@ -110,8 +109,7 @@ func (h *peerMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET") { return } - cid := strconv.FormatUint(h.clusterInfo.ID(), 16) - w.Header().Set("X-Etcd-Cluster-ID", cid) + w.Header().Set("X-Etcd-Cluster-ID", h.clusterInfo.ID().String()) if r.URL.Path != peerMembersPrefix { http.Error(w, "bad path", http.StatusBadRequest) diff --git a/etcdserver/etcdhttp/peer_test.go b/etcdserver/etcdhttp/peer_test.go index 7724b0c78..495d9eb4a 100644 --- a/etcdserver/etcdhttp/peer_test.go +++ b/etcdserver/etcdhttp/peer_test.go @@ -24,7 +24,6 @@ import ( "net/http" "net/http/httptest" "path" - "strconv" "strings" "testing" @@ -247,7 +246,7 @@ func TestServeMembersGet(t *testing.T) { t.Errorf("#%d: body = %s, want %s", i, rw.Body.String(), tt.wbody) } gcid := rw.Header().Get("X-Etcd-Cluster-ID") - wcid := strconv.FormatUint(cluster.ID(), 16) + wcid := cluster.ID().String() if gcid != wcid { t.Errorf("#%d: cid = %s, want %s", i, gcid, wcid) } diff --git a/etcdserver/member.go b/etcdserver/member.go index dcaced977..ebe48e846 100644 --- a/etcdserver/member.go +++ b/etcdserver/member.go @@ -26,7 +26,6 @@ import ( "sort" "time" - "github.com/coreos/etcd/pkg/strutil" "github.com/coreos/etcd/pkg/types" ) @@ -43,7 +42,7 @@ type Attributes struct { } type Member struct { - ID uint64 `json:"id"` + ID types.ID `json:"id"` RaftAttributes Attributes } @@ -68,7 +67,7 @@ func NewMember(name string, peerURLs types.URLs, clusterName string, now *time.T } hash := sha1.Sum(b) - m.ID = binary.BigEndian.Uint64(hash[:8]) + m.ID = types.ID(binary.BigEndian.Uint64(hash[:8])) return m } @@ -81,20 +80,20 @@ func (m *Member) PickPeerURL() string { return m.PeerURLs[rand.Intn(len(m.PeerURLs))] } -func memberStoreKey(id uint64) string { - return path.Join(storeMembersPrefix, strutil.IDAsHex(id)) +func memberStoreKey(id types.ID) string { + return path.Join(storeMembersPrefix, id.String()) } -func mustParseMemberIDFromKey(key string) uint64 { - id, err := strutil.IDFromHex(path.Base(key)) +func mustParseMemberIDFromKey(key string) types.ID { + id, err := types.IDFromString(path.Base(key)) if err != nil { log.Panicf("unexpected parse member id error: %v", err) } return id } -func removedMemberStoreKey(id uint64) string { - return path.Join(storeRemovedMembersPrefix, strutil.IDAsHex(id)) +func removedMemberStoreKey(id types.ID) string { + return path.Join(storeRemovedMembersPrefix, id.String()) } type SortableMemberSliceByPeerURLs []*Member diff --git a/etcdserver/member_test.go b/etcdserver/member_test.go index 336f0b7ed..12fa21046 100644 --- a/etcdserver/member_test.go +++ b/etcdserver/member_test.go @@ -20,6 +20,8 @@ import ( "net/url" "testing" "time" + + "github.com/coreos/etcd/pkg/types" ) func timeParse(value string) *time.Time { @@ -33,7 +35,7 @@ func timeParse(value string) *time.Time { func TestMemberTime(t *testing.T) { tests := []struct { mem *Member - id uint64 + id types.ID }{ {NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298}, // Same ID, different name (names shouldn't matter) diff --git a/etcdserver/sender.go b/etcdserver/sender.go index 00a6c6c78..2484f2e21 100644 --- a/etcdserver/sender.go +++ b/etcdserver/sender.go @@ -21,11 +21,10 @@ import ( "fmt" "log" "net/http" - "strconv" "time" "github.com/coreos/etcd/etcdserver/stats" - "github.com/coreos/etcd/pkg/strutil" + "github.com/coreos/etcd/pkg/types" "github.com/coreos/etcd/raft/raftpb" ) @@ -50,16 +49,17 @@ func Sender(t *http.Transport, cl *Cluster, ss *stats.ServerStats, ls *stats.Lea // ClusterStore, retrying up to 3 times for each message. The given // ServerStats and LeaderStats are updated appropriately func send(c *http.Client, cl *Cluster, m raftpb.Message, ss *stats.ServerStats, ls *stats.LeaderStats) { + to := types.ID(m.To) cid := cl.ID() // TODO (xiangli): reasonable retry logic for i := 0; i < 3; i++ { - memb := cl.Member(m.To) + memb := cl.Member(to) if memb == nil { - if !cl.IsIDRemoved(m.To) { + if !cl.IsIDRemoved(to) { // TODO: unknown peer id.. what do we do? I // don't think his should ever happen, need to // look into this further. - log.Printf("etcdserver: error sending message to unknown receiver %s", strutil.IDAsHex(m.To)) + log.Printf("etcdserver: error sending message to unknown receiver %s", to.String()) } return } @@ -75,8 +75,7 @@ func send(c *http.Client, cl *Cluster, m raftpb.Message, ss *stats.ServerStats, if m.Type == raftpb.MsgApp { ss.SendAppendReq(len(data)) } - to := strutil.IDAsHex(m.To) - fs := ls.Follower(to) + fs := ls.Follower(to.String()) start := time.Now() sent := httpPost(c, u, cid, data) @@ -92,14 +91,14 @@ func send(c *http.Client, cl *Cluster, m raftpb.Message, ss *stats.ServerStats, // httpPost POSTs a data payload to a url using the given client. Returns true // if the POST succeeds, false on any failure. -func httpPost(c *http.Client, url string, cid uint64, data []byte) bool { +func httpPost(c *http.Client, url string, cid types.ID, data []byte) bool { req, err := http.NewRequest("POST", url, bytes.NewBuffer(data)) if err != nil { // TODO: log the error? return false } req.Header.Set("Content-Type", "application/protobuf") - req.Header.Set("X-Etcd-Cluster-ID", strconv.FormatUint(cid, 16)) + req.Header.Set("X-Etcd-Cluster-ID", cid.String()) resp, err := c.Do(req) if err != nil { // TODO: log the error? @@ -110,7 +109,7 @@ func httpPost(c *http.Client, url string, cid uint64, data []byte) bool { switch resp.StatusCode { case http.StatusPreconditionFailed: // TODO: shutdown the etcdserver gracefully? - log.Fatalf("etcd: conflicting cluster ID with the target cluster (%s != %s). Exiting.", resp.Header.Get("X-Etcd-Cluster-ID"), strutil.IDAsHex(cid)) + log.Fatalf("etcd: conflicting cluster ID with the target cluster (%s != %s). Exiting.", resp.Header.Get("X-Etcd-Cluster-ID"), cid.String()) return false case http.StatusForbidden: // TODO: stop the server diff --git a/etcdserver/server.go b/etcdserver/server.go index b8e6028c5..07862817b 100644 --- a/etcdserver/server.go +++ b/etcdserver/server.go @@ -27,7 +27,6 @@ import ( "os" "path" "regexp" - "strconv" "sync/atomic" "time" @@ -36,7 +35,7 @@ import ( pb "github.com/coreos/etcd/etcdserver/etcdserverpb" "github.com/coreos/etcd/etcdserver/stats" "github.com/coreos/etcd/pkg/pbutil" - "github.com/coreos/etcd/pkg/strutil" + "github.com/coreos/etcd/pkg/types" "github.com/coreos/etcd/raft" "github.com/coreos/etcd/raft/raftpb" "github.com/coreos/etcd/snap" @@ -132,7 +131,7 @@ type Stats interface { // StoreStats returns statistics of the store backing this EtcdServer StoreStats() []byte // UpdateRecvApp updates the underlying statistics in response to a receiving an Append request - UpdateRecvApp(from uint64, length int64) + UpdateRecvApp(from types.ID, length int64) } type RaftTimer interface { @@ -145,7 +144,7 @@ type EtcdServer struct { w wait.Wait done chan struct{} stopped chan struct{} - id uint64 + id types.ID attributes Attributes Cluster *Cluster @@ -184,7 +183,7 @@ func NewServer(cfg *ServerConfig) *EtcdServer { st := store.New() var w *wal.WAL var n raft.Node - var id uint64 + var id types.ID haveWAL := wal.Exist(cfg.WALDir()) switch { case !haveWAL && cfg.ClusterState == ClusterStateValueExisting: @@ -240,9 +239,9 @@ func NewServer(cfg *ServerConfig) *EtcdServer { sstats := &stats.ServerStats{ Name: cfg.Name, - ID: strutil.IDAsHex(id), + ID: id.String(), } - lstats := stats.NewLeaderStats(strutil.IDAsHex(id)) + lstats := stats.NewLeaderStats(id.String()) s := &EtcdServer{ store: st, @@ -290,7 +289,7 @@ func (s *EtcdServer) start() { } func (s *EtcdServer) Process(ctx context.Context, m raftpb.Message) error { - if s.Cluster.IsIDRemoved(m.From) { + if s.Cluster.IsIDRemoved(types.ID(m.From)) { return ErrRemoved } return s.node.Step(ctx, m) @@ -423,8 +422,8 @@ func (s *EtcdServer) StoreStats() []byte { return s.store.JsonStats() } -func (s *EtcdServer) UpdateRecvApp(from uint64, length int64) { - s.stats.RecvAppendReq(strutil.IDAsHex(from), int(length)) +func (s *EtcdServer) UpdateRecvApp(from types.ID, length int64) { + s.stats.RecvAppendReq(from.String(), int(length)) } func (s *EtcdServer) AddMember(ctx context.Context, memb Member) error { @@ -436,7 +435,7 @@ func (s *EtcdServer) AddMember(ctx context.Context, memb Member) error { cc := raftpb.ConfChange{ ID: GenID(), Type: raftpb.ConfChangeAddNode, - NodeID: memb.ID, + NodeID: uint64(memb.ID), Context: b, } return s.configure(ctx, cc) @@ -528,7 +527,7 @@ func (s *EtcdServer) publish(retryInterval time.Duration) { cancel() switch err { case nil: - log.Printf("etcdserver: published %+v to cluster %x", s.attributes, s.Cluster.ID()) + log.Printf("etcdserver: published %+v to cluster %s", s.attributes, s.Cluster.ID()) return case ErrStopped: log.Printf("etcdserver: aborting publish because server is stopped") @@ -595,7 +594,7 @@ func (s *EtcdServer) applyRequest(r pb.Request) Response { id := mustParseMemberIDFromKey(path.Dir(r.Path)) m := s.Cluster.Member(id) if m == nil { - log.Fatalf("fetch member %x should never fail", id) + log.Fatalf("fetch member %s should never fail", id) } if err := json.Unmarshal([]byte(r.Val), &m.Attributes); err != nil { log.Fatalf("unmarshal %s should never fail: %v", r.Val, err) @@ -634,18 +633,18 @@ func (s *EtcdServer) applyConfChange(cc raftpb.ConfChange, nodes []uint64) error if err := json.Unmarshal(cc.Context, m); err != nil { panic("unexpected unmarshal error") } - if cc.NodeID != m.ID { + if cc.NodeID != uint64(m.ID) { panic("unexpected nodeID mismatch") } s.Cluster.AddMember(m) case raftpb.ConfChangeRemoveNode: - s.Cluster.RemoveMember(cc.NodeID) + s.Cluster.RemoveMember(types.ID(cc.NodeID)) } return nil } func (s *EtcdServer) checkConfChange(cc raftpb.ConfChange, nodes []uint64) error { - if s.Cluster.IsIDRemoved(cc.NodeID) { + if s.Cluster.IsIDRemoved(types.ID(cc.NodeID)) { return ErrIDRemoved } switch cc.Type { @@ -692,7 +691,7 @@ func GetClusterFromPeers(urls []string) (*Cluster, error) { log.Printf("etcdserver: unmarshal body error: %v", err) continue } - id, err := strconv.ParseUint(resp.Header.Get("X-Etcd-Cluster-ID"), 16, 64) + id, err := types.IDFromString(resp.Header.Get("X-Etcd-Cluster-ID")) if err != nil { log.Printf("etcdserver: parse uint error: %v", err) continue @@ -702,12 +701,17 @@ func GetClusterFromPeers(urls []string) (*Cluster, error) { return nil, fmt.Errorf("etcdserver: could not retrieve cluster information from the given urls") } -func startNode(cfg *ServerConfig, ids []uint64) (id uint64, n raft.Node, w *wal.WAL) { +func startNode(cfg *ServerConfig, ids []types.ID) (id types.ID, n raft.Node, w *wal.WAL) { var err error // TODO: remove the discoveryURL when it becomes part of the source for // generating nodeID. member := cfg.Cluster.MemberByName(cfg.Name) - metadata := pbutil.MustMarshal(&pb.Metadata{NodeID: member.ID, ClusterID: cfg.Cluster.ID()}) + metadata := pbutil.MustMarshal( + &pb.Metadata{ + NodeID: uint64(member.ID), + ClusterID: uint64(cfg.Cluster.ID()), + }, + ) if w, err = wal.Create(cfg.WALDir(), metadata); err != nil { log.Fatal(err) } @@ -717,15 +721,15 @@ func startNode(cfg *ServerConfig, ids []uint64) (id uint64, n raft.Node, w *wal. if err != nil { log.Fatal(err) } - peers[i] = raft.Peer{ID: id, Context: ctx} + peers[i] = raft.Peer{ID: uint64(id), Context: ctx} } id = member.ID - log.Printf("etcdserver: start node %x in cluster %x", id, cfg.Cluster.ID()) - n = raft.StartNode(id, peers, 10, 1) + log.Printf("etcdserver: start node %x in cluster %x", id.String(), cfg.Cluster.ID().String()) + n = raft.StartNode(uint64(id), peers, 10, 1) return } -func restartNode(cfg *ServerConfig, index uint64, snapshot *raftpb.Snapshot) (id uint64, n raft.Node, w *wal.WAL) { +func restartNode(cfg *ServerConfig, index uint64, snapshot *raftpb.Snapshot) (id types.ID, n raft.Node, w *wal.WAL) { var err error // restart a node from previous wal if w, err = wal.OpenAtIndex(cfg.WALDir(), index); err != nil { @@ -738,10 +742,10 @@ func restartNode(cfg *ServerConfig, index uint64, snapshot *raftpb.Snapshot) (id var metadata pb.Metadata pbutil.MustUnmarshal(&metadata, wmetadata) - id = metadata.NodeID - cfg.Cluster.SetID(metadata.ClusterID) - log.Printf("etcdserver: restart member %x in cluster %x at commit index %d", id, cfg.Cluster.ID(), st.Commit) - n = raft.RestartNode(id, 10, 1, snapshot, st, ents) + id = types.ID(metadata.NodeID) + cfg.Cluster.SetID(types.ID(metadata.ClusterID)) + log.Printf("etcdserver: restart member %s in cluster %s at commit index %d", id, cfg.Cluster.ID(), st.Commit) + n = raft.RestartNode(uint64(id), 10, 1, snapshot, st, ents) return } diff --git a/etcdserver/server_test.go b/etcdserver/server_test.go index 6693d4f4e..1e7c06a4c 100644 --- a/etcdserver/server_test.go +++ b/etcdserver/server_test.go @@ -30,6 +30,7 @@ import ( "github.com/coreos/etcd/Godeps/_workspace/src/code.google.com/p/go.net/context" pb "github.com/coreos/etcd/etcdserver/etcdserverpb" "github.com/coreos/etcd/pkg/testutil" + "github.com/coreos/etcd/pkg/types" "github.com/coreos/etcd/raft" "github.com/coreos/etcd/raft/raftpb" "github.com/coreos/etcd/store" @@ -407,7 +408,7 @@ func TestApplyRequestOnAdminMemberAttributes(t *testing.T) { // TODO: test ErrIDRemoved func TestApplyConfChangeError(t *testing.T) { nodes := []uint64{1, 2, 3} - removed := map[uint64]bool{4: true} + removed := map[types.ID]bool{4: true} tests := []struct { cc raftpb.ConfChange werr error @@ -1381,7 +1382,7 @@ func (cs *removedClusterStore) IsRemoved(id uint64) bool { return cs.removed[id] func mustMakePeerSlice(t *testing.T, ids ...uint64) []raft.Peer { peers := make([]raft.Peer, len(ids)) for i, id := range ids { - m := Member{ID: id} + m := Member{ID: types.ID(id)} b, err := json.Marshal(m) if err != nil { t.Fatal(err) diff --git a/pkg/strutil/hex.go b/pkg/strutil/hex.go deleted file mode 100644 index acfc197e8..000000000 --- a/pkg/strutil/hex.go +++ /dev/null @@ -1,29 +0,0 @@ -/* - Copyright 2014 CoreOS, Inc. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package strutil - -import ( - "strconv" -) - -func IDAsHex(id uint64) string { - return strconv.FormatUint(id, 16) -} - -func IDFromHex(s string) (uint64, error) { - return strconv.ParseUint(s, 16, 64) -} diff --git a/pkg/types/id.go b/pkg/types/id.go new file mode 100644 index 000000000..3e8a46bbd --- /dev/null +++ b/pkg/types/id.go @@ -0,0 +1,43 @@ +/* + Copyright 2014 CoreOS, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package types + +import ( + "strconv" +) + +// ID represents a generic identifier which is canonically +// stored as a uint64 but is typically represented as a +// base-16 string for input/output +type ID uint64 + +func (i ID) String() string { + return strconv.FormatUint(uint64(i), 16) +} + +// IDFromString attempts to create an ID from a base-16 string. +func IDFromString(s string) (ID, error) { + i, err := strconv.ParseUint(s, 16, 64) + return ID(i), err +} + +// IDSlice implements the sort interface +type IDSlice []ID + +func (p IDSlice) Len() int { return len(p) } +func (p IDSlice) Less(i, j int) bool { return uint64(p[i]) < uint64(p[j]) } +func (p IDSlice) Swap(i, j int) { p[i], p[j] = p[j], p[i] } diff --git a/pkg/strutil/hex_test.go b/pkg/types/id_test.go similarity index 57% rename from pkg/strutil/hex_test.go rename to pkg/types/id_test.go index 1ee69df44..d3463aeeb 100644 --- a/pkg/strutil/hex_test.go +++ b/pkg/types/id_test.go @@ -13,64 +13,65 @@ See the License for the specific language governing permissions and limitations under the License. */ - -package strutil +package types import ( + "reflect" + "sort" "testing" ) -func TestIDAsHex(t *testing.T) { +func TestIDString(t *testing.T) { tests := []struct { - input uint64 + input ID want string }{ { - input: uint64(12), + input: 12, want: "c", }, { - input: uint64(4918257920282737594), + input: 4918257920282737594, want: "444129853c343bba", }, } for i, tt := range tests { - got := IDAsHex(tt.input) + got := tt.input.String() if tt.want != got { - t.Errorf("#%d: IDAsHex failure: want=%v, got=%v", i, tt.want, got) + t.Errorf("#%d: ID.String failure: want=%v, got=%v", i, tt.want, got) } } } -func TestIDFromHex(t *testing.T) { +func TestIDFromString(t *testing.T) { tests := []struct { input string - want uint64 + want ID }{ { input: "17", - want: uint64(23), + want: 23, }, { input: "612840dae127353", - want: uint64(437557308098245459), + want: 437557308098245459, }, } for i, tt := range tests { - got, err := IDFromHex(tt.input) + got, err := IDFromString(tt.input) if err != nil { - t.Errorf("#%d: IDFromHex failure: err=%v", i, err) + t.Errorf("#%d: IDFromString failure: err=%v", i, err) continue } if tt.want != got { - t.Errorf("#%d: IDFromHex failure: want=%v, got=%v", i, tt.want, got) + t.Errorf("#%d: IDFromString failure: want=%v, got=%v", i, tt.want, got) } } } -func TestIDFromHexFail(t *testing.T) { +func TestIDFromStringFail(t *testing.T) { tests := []string{ "", "XXX", @@ -78,9 +79,18 @@ func TestIDFromHexFail(t *testing.T) { } for i, tt := range tests { - _, err := IDFromHex(tt) + _, err := IDFromString(tt) if err == nil { - t.Fatalf("#%d: IDFromHex expected error, but err=nil", i) + t.Fatalf("#%d: IDFromString expected error, but err=nil", i) } } } + +func TestIDSlice(t *testing.T) { + g := []ID{10, 500, 5, 1, 100, 25} + w := []ID{1, 5, 10, 25, 100, 500} + sort.Sort(IDSlice(g)) + if !reflect.DeepEqual(g, w) { + t.Errorf("slice after sort = %#v, want %#v", g, w) + } +} diff --git a/test b/test index 04b339283..02ede089e 100755 --- a/test +++ b/test @@ -15,7 +15,7 @@ COVER=${COVER:-"-cover"} source ./build # Hack: gofmt ./ will recursively check the .git directory. So use *.go for gofmt. -TESTABLE_AND_FORMATTABLE="client discovery etcdctl/command etcdmain etcdserver etcdserver/etcdhttp etcdserver/etcdhttp/httptypes etcdserver/etcdserverpb integration pkg/flags pkg/strutil pkg/transport proxy raft snap store wait wal" +TESTABLE_AND_FORMATTABLE="client discovery etcdctl/command etcdmain etcdserver etcdserver/etcdhttp etcdserver/etcdhttp/httptypes etcdserver/etcdserverpb integration pkg/flags pkg/types pkg/transport proxy raft snap store wait wal" FORMATTABLE="$TESTABLE_AND_FORMATTABLE *.go etcdctl/" # user has not provided PKG override