From 878b55ec403a14cbae8a6ca16985755546d9bf35 Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Tue, 1 May 2018 13:04:49 +0300 Subject: [PATCH] Removes /id and /hosts endpoints. Augments /status endpoint with the node local ID. --- client.go | 25 ------------------------- docs/administration.md | 17 +++++++++++++---- docs/api-reference.md | 13 ------------- handler.go | 27 ++++++--------------------- handler_test.go | 2 +- server/cluster_test.go | 36 +++++++++++++++++++++--------------- 6 files changed, 41 insertions(+), 79 deletions(-) diff --git a/client.go b/client.go index c2e88c77b..eee1a3a9c 100644 --- a/client.go +++ b/client.go @@ -1142,30 +1142,6 @@ func (c *InternalHTTPClient) clientURI(ctx context.Context) *URI { return clientURI } -func (c *InternalHTTPClient) NodeID(uri *URI) (string, error) { - u := uriPathToURL(uri, "/id") - req, err := http.NewRequest("GET", u.String(), nil) - resp, err := c.HTTPClient.Do(req) - if err != nil { - return "", fmt.Errorf("executing http request: %v", err) - } - defer resp.Body.Close() - - // Read body. - body, err := ioutil.ReadAll(resp.Body) - if err != nil { - return "", fmt.Errorf("reading response body: %v", err) - } - - // Return error if status is not OK. - switch resp.StatusCode { - case http.StatusOK: // ok - default: - return "", fmt.Errorf("unexpected response status code: %d: %s", resp.StatusCode, body) - } - return string(body), nil -} - // Bit represents the location of a single bit. type Bit struct { RowID uint64 @@ -1362,5 +1338,4 @@ type InternalClient interface { ColumnAttrDiff(ctx context.Context, index string, blks []AttrBlock) (map[uint64]map[string]interface{}, error) RowAttrDiff(ctx context.Context, index, frame string, blks []AttrBlock) (map[uint64]map[string]interface{}, error) SendMessage(ctx context.Context, pb proto.Message) error - NodeID(uri *URI) (string, error) } diff --git a/docs/administration.md b/docs/administration.md index 190b5ca1f..fe7809553 100644 --- a/docs/administration.md +++ b/docs/administration.md @@ -108,12 +108,20 @@ If the node is being added to a cluster which contains no data (for example, dur In order to remove a node from a cluster, your cluster must be configured to have a [cluster replicas](../configuration/#cluster-replicas) value of at least 2; if you're removing a node that no longer exists (for example a node that has died), there must be at least one additional replica of the data owned by the dead node in order for the cluster to correctly rebalance itself. -To remove node `localhost:10102` from a cluster having coordinator `localhost:10101`, first determine the ID of the node to be removed. If the node to be removed is still available, you can find the ID by issuing an `/id` request to the node: +To remove node `localhost:10102` from a cluster having coordinator `localhost:10101`, first determine the ID of the node to be removed. If the node to be removed is still available, you can find the ID by issuing an `/status` request to the node. Node's ID is in the `localID` field: ``` request -curl localhost:10102/id +curl localhost:10101/status ``` ``` response -40a891fa-243b-4d71-ae24-4f5c78a0f4b1 +{ + "state":"NORMAL", + "nodes":[ + {"id":"24824777-62ec-4151-9fbd-67e4676e317d","uri":{"scheme":"http","host":"localhost","port":10101}} + {"id":"40a891fa-243b-4d71-ae24-4f5c78a0f4b1","uri":{"scheme":"http","host":"localhost","port":10102}} + {"id":"9fab09cc-3c26-4202-9622-d167c84684d9","uri":{"scheme":"http","host":"localhost","port":10103}} + ], + "localID": "40a891fa-243b-4d71-ae24-4f5c78a0f4b1" +} ``` If the node to be removed is no longer available, you can get the IDs of the nodes in the cluster by issuing a `/status` request to any available node: @@ -127,7 +135,8 @@ curl localhost:10101/status {"id":"24824777-62ec-4151-9fbd-67e4676e317d","uri":{"scheme":"http","host":"localhost","port":10101}} {"id":"40a891fa-243b-4d71-ae24-4f5c78a0f4b1","uri":{"scheme":"http","host":"localhost","port":10102}} {"id":"9fab09cc-3c26-4202-9622-d167c84684d9","uri":{"scheme":"http","host":"localhost","port":10103}} - ] + ], + "localID": "40a891fa-243b-4d71-ae24-4f5c78a0f4b1" } ``` diff --git a/docs/api-reference.md b/docs/api-reference.md index 2e744954e..c73083d3c 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -166,19 +166,6 @@ curl localhost:10101/index/repository/frame/stats/field/pullrequests \ {} ``` -### List hosts - -`GET /hosts` - -Returns the hosts in the cluster. - -``` request -curl -XGET localhost:10101/hosts -``` -``` response -[{"host":":10101"}] -``` - ### Get version `GET /version` diff --git a/handler.go b/handler.go index 7bf11af1b..92161cef1 100644 --- a/handler.go +++ b/handler.go @@ -121,8 +121,6 @@ func NewRouter(handler *Handler) *mux.Router { router.PathPrefix("/debug/pprof/").Handler(http.DefaultServeMux).Methods("GET") router.Handle("/debug/vars", expvar.Handler()).Methods("GET") router.HandleFunc("/fragment/data", handler.handleGetFragmentData).Methods("GET").Name("GetFragmentData") - router.HandleFunc("/hosts", handler.handleGetHosts).Methods("GET") - router.HandleFunc("/id", handler.handleGetID).Methods("GET") router.HandleFunc("/schema", handler.handleGetSchema).Methods("GET") router.HandleFunc("/slices/max", handler.handleGetSlicesMax).Methods("GET") // TODO: deprecate, but it's being used by the client (for backups) router.HandleFunc("/status", handler.handleGetStatus).Methods("GET") @@ -244,8 +242,9 @@ func (h *Handler) handleGetSchema(w http.ResponseWriter, r *http.Request) { // handleGetStatus handles GET /status requests. func (h *Handler) handleGetStatus(w http.ResponseWriter, r *http.Request) { status := getStatusResponse{ - State: h.API.State(), - Nodes: h.API.Hosts(r.Context()), + State: h.API.State(), + Nodes: h.API.Hosts(r.Context()), + LocalID: h.API.LocalID(), } if err := json.NewEncoder(w).Encode(status); err != nil { h.Logger.Printf("write status response error: %s", err) @@ -257,8 +256,9 @@ type getSchemaResponse struct { } type getStatusResponse struct { - State string `json:"state"` - Nodes []*Node `json:"nodes"` + State string `json:"state"` + Nodes []*Node `json:"nodes"` + LocalID string `json:"localID"` } // handlePostQuery handles /query requests. @@ -1180,14 +1180,6 @@ func (h *Handler) handlePostFrameRestore(w http.ResponseWriter, r *http.Request) } } -// handleGetHosts handles /hosts requests. -func (h *Handler) handleGetHosts(w http.ResponseWriter, r *http.Request) { - hosts := h.API.Hosts(r.Context()) - if err := json.NewEncoder(w).Encode(hosts); err != nil { - h.Logger.Printf("write version response error: %s", err) - } -} - // handleGetVersion handles /version requests. func (h *Handler) handleGetVersion(w http.ResponseWriter, r *http.Request) { err := json.NewEncoder(w).Encode(struct { @@ -1615,13 +1607,6 @@ func (h *Handler) handlePostClusterMessage(w http.ResponseWriter, r *http.Reques } } -func (h *Handler) handleGetID(w http.ResponseWriter, r *http.Request) { - _, err := w.Write([]byte(h.API.LocalID())) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - } -} - type defaultClusterMessageResponse struct{} type queryValidationSpec struct { diff --git a/handler_test.go b/handler_test.go index 4187b3303..d4dea5f6c 100644 --- a/handler_test.go +++ b/handler_test.go @@ -149,7 +149,7 @@ func TestHandler_Status(t *testing.T) { h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/status", nil)) if w.Code != http.StatusOK { t.Fatalf("unexpected status code: %d", w.Code) - } else if body := w.Body.String(); body != `{"state":"NORMAL","nodes":[{"id":"node0","uri":{"scheme":"http","host":"host0"},"isCoordinator":false}]}`+"\n" { + } else if body := w.Body.String(); body != `{"state":"NORMAL","nodes":[{"id":"node0","uri":{"scheme":"http","host":"host0"},"isCoordinator":false}],"localID":"node0"}`+"\n" { t.Fatalf("unexpected body: %s", body) } } diff --git a/server/cluster_test.go b/server/cluster_test.go index 1a64ce305..a71a356e9 100644 --- a/server/cluster_test.go +++ b/server/cluster_test.go @@ -16,6 +16,7 @@ package server_test import ( "context" + "encoding/json" "fmt" "net/http" "reflect" @@ -443,6 +444,19 @@ func TestClusterResize_RemoveNode(t *testing.T) { m0 := cluster[0] m1 := cluster[1] + mustNodeID := func(baseURL string) string { + body := test.MustDo("GET", fmt.Sprintf("%s/status", baseURL), "").Body + var resp map[string]interface{} + err := json.Unmarshal([]byte(body), &resp) + if err != nil { + panic(err) + } + if localID, ok := resp["localID"].(string); ok { + return localID + } + panic("localID should be a string") + } + t.Run("ErrorRemoveInvalidNode", func(t *testing.T) { resp := test.MustDo("POST", m0.URL()+fmt.Sprintf("/cluster/resize/remove-node"), `{"id": "invalid-node-id"}`) expBody := "removing node: finding node to remove: node with provided ID does not exist" @@ -454,10 +468,8 @@ func TestClusterResize_RemoveNode(t *testing.T) { }) t.Run("ErrorRemoveCoordinator", func(t *testing.T) { - resp := test.MustDo("GET", m0.URL()+fmt.Sprintf("/id"), "") - nodeID := resp.Body - - resp = test.MustDo("POST", m0.URL()+fmt.Sprintf("/cluster/resize/remove-node"), fmt.Sprintf(`{"id": "%s"}`, nodeID)) + nodeID := mustNodeID(m0.URL()) + resp := test.MustDo("POST", m0.URL()+fmt.Sprintf("/cluster/resize/remove-node"), fmt.Sprintf(`{"id": "%s"}`, nodeID)) expBody := "removing node: calling node leave: coordinator cannot be removed; first, make a different node the new coordinator." if resp.StatusCode != http.StatusInternalServerError { @@ -468,13 +480,9 @@ func TestClusterResize_RemoveNode(t *testing.T) { }) t.Run("ErrorRemoveOnNonCoordinator", func(t *testing.T) { - resp := test.MustDo("GET", m0.URL()+fmt.Sprintf("/id"), "") - coordinatorNodeID := resp.Body - - resp = test.MustDo("GET", m1.URL()+fmt.Sprintf("/id"), "") - nodeID := resp.Body - - resp = test.MustDo("POST", m1.URL()+fmt.Sprintf("/cluster/resize/remove-node"), fmt.Sprintf(`{"id": "%s"}`, nodeID)) + coordinatorNodeID := mustNodeID(m0.URL()) + nodeID := mustNodeID(m1.URL()) + resp := test.MustDo("POST", m1.URL()+fmt.Sprintf("/cluster/resize/remove-node"), fmt.Sprintf(`{"id": "%s"}`, nodeID)) expBody := fmt.Sprintf("removing node: calling node leave: node removal requests are only valid on the coordinator node: %s", coordinatorNodeID) if resp.StatusCode != http.StatusInternalServerError { @@ -505,10 +513,8 @@ func TestClusterResize_RemoveNode(t *testing.T) { t.Fatal(err) } - resp := test.MustDo("GET", m1.URL()+fmt.Sprintf("/id"), "") - nodeID := resp.Body - - resp = test.MustDo("POST", m0.URL()+fmt.Sprintf("/cluster/resize/remove-node"), fmt.Sprintf(`{"id": "%s"}`, nodeID)) + nodeID := mustNodeID(m1.URL()) + resp := test.MustDo("POST", m0.URL()+fmt.Sprintf("/cluster/resize/remove-node"), fmt.Sprintf(`{"id": "%s"}`, nodeID)) expBody := "not enough data to perform resize" if resp.StatusCode != http.StatusInternalServerError { t.Fatalf("expected StatusCode %d but got %d", http.StatusInternalServerError, resp.StatusCode)