From 114e57fff1c2a83c00a1dd0e9fdafa6bcf59ea27 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 21 Jun 2016 12:39:40 -0700 Subject: [PATCH] consul: use the Near field instead of PreferLocal --- command/agent/http.go | 8 +---- command/agent/http_test.go | 12 -------- command/agent/prepared_query_endpoint_test.go | 2 ++ consul/prepared_query_endpoint.go | 29 +++++++++++++++++-- consul/prepared_query_endpoint_test.go | 24 +++++++++++++-- consul/structs/prepared_query.go | 16 +++++----- 6 files changed, 58 insertions(+), 33 deletions(-) diff --git a/command/agent/http.go b/command/agent/http.go index 92247ef2c..0cfbafbe1 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -531,13 +531,7 @@ func (s *HTTPServer) parseToken(req *http.Request, token *string) { // DC in the request, if given, or else the agent's DC. func (s *HTTPServer) parseSource(req *http.Request, source *structs.QuerySource) { s.parseDC(req, &source.Datacenter) - if node := req.URL.Query().Get("near"); node != "" { - if node == "_agent" { - source.Node = s.agent.config.NodeName - } else { - source.Node = node - } - } + source.Node = req.URL.Query().Get("near") } // parse is a convenience method for endpoints that need diff --git a/command/agent/http_test.go b/command/agent/http_test.go index b6618977f..11369f8fd 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -382,18 +382,6 @@ func TestParseSource(t *testing.T) { if source.Datacenter != "foo" || source.Node != "bob" { t.Fatalf("bad: %v", source) } - - // The magic "_agent" node name will use the agent's local node name. - req, err = http.NewRequest("GET", - "/v1/catalog/nodes?near=_agent", nil) - if err != nil { - t.Fatalf("err: %v", err) - } - source = structs.QuerySource{} - srv.parseSource(req, &source) - if source.Datacenter != "dc1" || source.Node != srv.agent.config.NodeName { - t.Fatalf("bad: %v", source) - } } func TestParseWait(t *testing.T) { diff --git a/command/agent/prepared_query_endpoint_test.go b/command/agent/prepared_query_endpoint_test.go index 8997de05f..138124c69 100644 --- a/command/agent/prepared_query_endpoint_test.go +++ b/command/agent/prepared_query_endpoint_test.go @@ -279,6 +279,7 @@ func TestPreparedQuery_Execute(t *testing.T) { m.executeFn = func(args *structs.PreparedQueryExecuteRequest, reply *structs.PreparedQueryExecuteResponse) error { expected := &structs.PreparedQueryExecuteRequest{ + Origin: srv.agent.config.NodeName, Datacenter: "dc1", QueryIDOrName: "my-id", Limit: 5, @@ -350,6 +351,7 @@ func TestPreparedQuery_Explain(t *testing.T) { m.explainFn = func(args *structs.PreparedQueryExecuteRequest, reply *structs.PreparedQueryExplainResponse) error { expected := &structs.PreparedQueryExecuteRequest{ + Origin: srv.agent.config.NodeName, Datacenter: "dc1", QueryIDOrName: "my-id", Limit: 5, diff --git a/consul/prepared_query_endpoint.go b/consul/prepared_query_endpoint.go index 970b459d5..6b41c907c 100644 --- a/consul/prepared_query_endpoint.go +++ b/consul/prepared_query_endpoint.go @@ -368,12 +368,35 @@ func (p *PreparedQuery) Execute(args *structs.PreparedQueryExecuteRequest, // Shuffle the results in case coordinates are not available if they // requested an RTT sort. reply.Nodes.Shuffle() - if err := p.srv.sortNodesByDistanceFrom(args.Source, reply.Nodes); err != nil { + + // Get the source to sort by. This can be passed in by the requestor, or + // pre-defined using the Near parameter in the prepared query. If the + // near parameter was defined, that will be preferred. + sortFrom := args.Source + if query.Service.Near != "" { + sortFrom = structs.QuerySource{ + Datacenter: args.Datacenter, + Node: query.Service.Near, + } + } + + // Respect the magic "_agent" flag. + preferLocal := false + if sortFrom.Node == "_agent" { + preferLocal = true + sortFrom.Node = args.Origin + } + + // Perform the distance sort + if err := p.srv.sortNodesByDistanceFrom(sortFrom, reply.Nodes); err != nil { return err } - // Prefer the local service if it exists and is in the set. - if query.Service.PreferLocal { + // Nodes cannot be any "closer" than localhost, so this special case ensures + // the local node is returned first if it is present in the result. This + // allows the local agent to be preferred even when network coordinates are + // not enabled. + if preferLocal { for i, node := range reply.Nodes { if node.Node.Node == args.Origin { remote := append(reply.Nodes[:i], reply.Nodes[i+1:]...) diff --git a/consul/prepared_query_endpoint_test.go b/consul/prepared_query_endpoint_test.go index b4a62b130..b0a752863 100644 --- a/consul/prepared_query_endpoint_test.go +++ b/consul/prepared_query_endpoint_test.go @@ -1609,7 +1609,7 @@ func TestPreparedQuery_Execute(t *testing.T) { // Set the query to prefer a colocated service query.Op = structs.PreparedQueryUpdate - query.Query.Service.PreferLocal = true + query.Query.Service.Near = "_agent" if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Apply", &query, &query.Query.ID); err != nil { t.Fatalf("err: %v", err) } @@ -1630,7 +1630,6 @@ func TestPreparedQuery_Execute(t *testing.T) { if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply); err != nil { t.Fatalf("err: %v", err) } - if n := len(reply.Nodes); n != 10 { t.Fatalf("expect 10 nodes, got: %d", n) } @@ -1640,8 +1639,27 @@ func TestPreparedQuery_Execute(t *testing.T) { } } + // Falls back to remote nodes if service is not available locally + { + req := structs.PreparedQueryExecuteRequest{ + Origin: "not-in-result", + Datacenter: "dc1", + QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: execToken}, + } + + var reply structs.PreparedQueryExecuteResponse + + if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply); err != nil { + t.Fatalf("err: %v", err) + } + if n := len(reply.Nodes); n != 10 { + t.Fatalf("expect 10 nodes, got: %d", n) + } + } + // Remove local preference. - query.Query.Service.PreferLocal = false + query.Query.Service.Near = "" if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Apply", &query, &query.Query.ID); err != nil { t.Fatalf("err:% v", err) } diff --git a/consul/structs/prepared_query.go b/consul/structs/prepared_query.go index 85439b6e0..49ad25a98 100644 --- a/consul/structs/prepared_query.go +++ b/consul/structs/prepared_query.go @@ -34,10 +34,10 @@ type ServiceQuery struct { // discarded) OnlyPassing bool - // If PreferLocal is true, the local agent will be checked for a local - // copy of the service before continuing to remote machines. This is - // useful to prefer colocated services but fall back when unavailable. - PreferLocal bool + // Near allows the query to always prefer the node nearest the given + // node. If the node does not exist, results are returned in their + // normal randomly-shuffled order. + Near string // Tags are a set of required and/or disallowed tags. If a tag is in // this list it must be present. If the tag is preceded with "!" then @@ -168,10 +168,6 @@ func (q *PreparedQuerySpecificRequest) RequestDatacenter() string { // PreparedQueryExecuteRequest is used to execute a prepared query. type PreparedQueryExecuteRequest struct { - // Origin is used to carry around a reference to the node which - // is executing the query on behalf of the client. - Origin string - // Datacenter is the target this request is intended for. Datacenter string @@ -182,6 +178,10 @@ type PreparedQueryExecuteRequest struct { // Limit will trim the resulting list down to the given limit. Limit int + // Origin is used to carry around a reference to the node which + // is executing the query on behalf of the client. + Origin string + // Source is used to sort the results relative to a given node using // network coordinates. Source QuerySource