Merge pull request #894 from rqlite/nodes-timeout

nodes/ endpoint supports timeout

CHANGELOG.md

@@ -4,6 +4,7 @@
 - [PR #888](https://github.com/rqlite/rqlite/pull/888): Expose stats about BoltDB on the `status/` endpoint.
 - [PR #889](https://github.com/rqlite/rqlite/pull/889): Add OS-level information to `status/` output.
 - [PR #892](https://github.com/rqlite/rqlite/pull/892): More Snapshot metrics.
+- [PR #894](https://github.com/rqlite/rqlite/pull/894): Support controlling timeout on nodes/ endpoint.
 
 ## 6.4.3 (September 8th 2021)
 ### Implementation changes and bug fixes

DOC/DIAGNOSTICS.md

@@ -37,6 +37,7 @@ runtime:
 ```bash
 curl localhost:4001/nodes?pretty
 curl localhost:4001/nodes?nonvoters&pretty  # Also check non-voting nodes.
+curl localhost:4001/nodes?timeout=5s  # Give up if all nodes don't respond within 5 seconds. Default is 30 seconds.
 ```
 
 You can also request the same nodes information via the CLI:

cluster/client.go

@@ -106,6 +106,10 @@ func (c *Client) GetNodeAPIAddr(nodeAddr string, timeout time.Duration) (string,
 	}
 
 	// Read length of response.
+	if err := conn.SetDeadline(time.Now().Add(timeout)); err != nil {
+		handleConnError(conn)
+		return "", err
+	}
 	_, err = io.ReadFull(conn, b)
 	if err != nil {
 		return "", err
@@ -175,12 +179,11 @@ func (c *Client) Execute(er *command.ExecuteRequest, nodeAddr string, timeout ti
 		return nil, err
 	}
 
+	// Read length of response.
 	if err := conn.SetDeadline(time.Now().Add(timeout)); err != nil {
 		handleConnError(conn)
 		return nil, err
 	}
-
-	// Read length of response.
 	_, err = io.ReadFull(conn, b)
 	if err != nil {
 		return nil, err
@@ -189,6 +192,10 @@ func (c *Client) Execute(er *command.ExecuteRequest, nodeAddr string, timeout ti
 
 	// Read in the actual response.
 	p = make([]byte, sz)
+	if err := conn.SetDeadline(time.Now().Add(timeout)); err != nil {
+		handleConnError(conn)
+		return nil, err
+	}
 	_, err = io.ReadFull(conn, p)
 	if err != nil {
 		return nil, err

http/service.go

@@ -143,7 +143,7 @@ const (
 	numAuthFail         = "authFail"
 
 	// Default timeout for cluster communications.
-	defaulTimeout = 30 * time.Second
+	defaultTimeout = 30 * time.Second
 
 	// PermAll means all actions permitted.
 	PermAll = "all"
@@ -677,7 +677,7 @@ func (s *Service) handleNodes(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	t, err := timeout(r, time.Duration(1))
+	timeout, err := timeoutParam(r, defaultTimeout)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
@@ -712,7 +712,7 @@ func (s *Service) handleNodes(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	nodesResp, err := s.checkNodes(filteredNodes, t)
+	nodesResp, err := s.checkNodes(filteredNodes, timeout)
 	if err != nil {
 		http.Error(w, fmt.Sprintf("check nodes: %s", err.Error()),
 			http.StatusInternalServerError)
@@ -773,7 +773,7 @@ func (s *Service) handleExecute(w http.ResponseWriter, r *http.Request) {
 
 	resp := NewResponse()
 
-	timeout, isTx, timings, redirect, err := reqParams(r, defaulTimeout)
+	timeout, isTx, timings, redirect, err := reqParams(r, defaultTimeout)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
@@ -855,7 +855,7 @@ func (s *Service) handleQuery(w http.ResponseWriter, r *http.Request) {
 
 	resp := NewResponse()
 
-	timeout, isTx, timings, redirect, err := reqParams(r, defaulTimeout)
+	timeout, isTx, timings, redirect, err := reqParams(r, defaultTimeout)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
@@ -1009,7 +1009,7 @@ func (s *Service) LeaderAPIAddr() string {
 		return ""
 	}
 
-	apiAddr, err := s.cluster.GetNodeAPIAddr(nodeAddr, defaulTimeout)
+	apiAddr, err := s.cluster.GetNodeAPIAddr(nodeAddr, defaultTimeout)
 
 	if err != nil {
 		return ""
@@ -1044,7 +1044,7 @@ func (s *Service) checkNodes(nodes []*store.Server, timeout time.Duration) (map[
 			defer mu.Unlock()
 
 			start := time.Now()
-			apiAddr, err := s.cluster.GetNodeAPIAddr(raftAddr, defaulTimeout)
+			apiAddr, err := s.cluster.GetNodeAPIAddr(raftAddr, timeout)
 			if err != nil {
 				resp[id].error = err.Error()
 				return
@@ -1197,19 +1197,19 @@ func isRedirect(req *http.Request) (bool, error) {
 	return queryParam(req, "redirect")
 }
 
-// timeoutParam returns the value, if any, set for timeout. If not set,
+// timeoutParam returns the value, if any, set for timeout. If not set, it
-// it returns the value passed in as a default.
+// returns the value passed in as a default.
 func timeoutParam(req *http.Request, def time.Duration) (time.Duration, error) {
 	q := req.URL.Query()
 	timeout := strings.TrimSpace(q.Get("timeout"))
 	if timeout == "" {
 		return def, nil
 	}
-	d, err := time.ParseDuration(timeout)
+	t, err := time.ParseDuration(timeout)
 	if err != nil {
 		return 0, err
 	}
-	return d, nil
+	return t, nil
 }
 
 // isTx returns whether the HTTP request is requesting a transaction.
@@ -1254,21 +1254,6 @@ func isTimings(req *http.Request) (bool, error) {
 	return queryParam(req, "timings")
 }
 
-// timeout returns the timeout included in the query, or the given default
-func timeout(req *http.Request, d time.Duration) (time.Duration, error) {
-	q := req.URL.Query()
-	tStr := q.Get("timeout")
-	if tStr == "" {
-		return d, nil
-	}
-
-	t, err := time.ParseDuration(tStr)
-	if err != nil {
-		return d, nil
-	}
-	return t, nil
-}
-
 // level returns the requested consistency level for a query
 func level(req *http.Request) (command.QueryRequest_Level, error) {
 	q := req.URL.Query()

http/service_test.go

@@ -854,6 +854,7 @@ func Test_timeoutQueryParam(t *testing.T) {
 	tests := []struct {
 		u   string
 		dur string
+		err bool
 	}{
 		{
 			u:   "http://localhost:4001/nodes?timeout=5s",
@@ -873,15 +874,19 @@ func Test_timeoutQueryParam(t *testing.T) {
 		},
 		{
 			u:   "http://localhost:4001/nodes?timeout=zdfjkh",
-			dur: defStr,
+			err: true,
 		},
 	}
 
 	for _, tt := range tests {
 		req.URL = mustURLParse(tt.u)
-		timeout, err := timeout(&req, def)
+		timeout, err := timeoutParam(&req, def)
 		if err != nil {
-			t.Fatalf("failed to get timeout: %s", err)
+			if tt.err {
+				// Error is expected, all is OK.
+				continue
+			}
+			t.Fatalf("failed to get timeout as expected: %s", err)
 		}
 		if timeout != mustParseDuration(tt.dur) {
 			t.Fatalf("got wrong timeout, expected %s, got %s", mustParseDuration(tt.dur), timeout)