http: Check HTTPUseCache in a single place

HTTPUseCache is only used is a gate for allowing QueryOptions.UseCache to be enabled. By
moving it to the place where the query options are set, this behaviour is more obvious.

Also remove parseInternal which was an alias for parse.
This commit is contained in:
Daniel Nephin 2020-10-04 13:54:56 -04:00
parent c66a63275f
commit b58401480b
4 changed files with 17 additions and 20 deletions

View File

@ -5,8 +5,9 @@ import (
"net/http" "net/http"
"strings" "strings"
"github.com/armon/go-metrics" metrics "github.com/armon/go-metrics"
"github.com/armon/go-metrics/prometheus" "github.com/armon/go-metrics/prometheus"
cachetype "github.com/hashicorp/consul/agent/cache-types" cachetype "github.com/hashicorp/consul/agent/cache-types"
"github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/structs"
) )
@ -200,7 +201,7 @@ func (s *HTTPHandlers) CatalogDatacenters(resp http.ResponseWriter, req *http.Re
parseCacheControl(resp, req, &args.QueryOptions) parseCacheControl(resp, req, &args.QueryOptions)
var out []string var out []string
if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache { if args.QueryOptions.UseCache {
raw, m, err := s.agent.cache.Get(req.Context(), cachetype.CatalogDatacentersName, &args) raw, m, err := s.agent.cache.Get(req.Context(), cachetype.CatalogDatacentersName, &args)
if err != nil { if err != nil {
metrics.IncrCounterWithLabels([]string{"client", "rpc", "error", "catalog_datacenters"}, 1, metrics.IncrCounterWithLabels([]string{"client", "rpc", "error", "catalog_datacenters"}, 1,
@ -282,7 +283,7 @@ func (s *HTTPHandlers) CatalogServices(resp http.ResponseWriter, req *http.Reque
var out structs.IndexedServices var out structs.IndexedServices
defer setMeta(resp, &out.QueryMeta) defer setMeta(resp, &out.QueryMeta)
if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache { if args.QueryOptions.UseCache {
raw, m, err := s.agent.cache.Get(req.Context(), cachetype.CatalogListServicesName, &args) raw, m, err := s.agent.cache.Get(req.Context(), cachetype.CatalogListServicesName, &args)
if err != nil { if err != nil {
metrics.IncrCounterWithLabels([]string{"client", "rpc", "error", "catalog_services"}, 1, metrics.IncrCounterWithLabels([]string{"client", "rpc", "error", "catalog_services"}, 1,
@ -371,7 +372,7 @@ func (s *HTTPHandlers) catalogServiceNodes(resp http.ResponseWriter, req *http.R
var out structs.IndexedServiceNodes var out structs.IndexedServiceNodes
defer setMeta(resp, &out.QueryMeta) defer setMeta(resp, &out.QueryMeta)
if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache { if args.QueryOptions.UseCache {
raw, m, err := s.agent.cache.Get(req.Context(), cachetype.CatalogServicesName, &args) raw, m, err := s.agent.cache.Get(req.Context(), cachetype.CatalogServicesName, &args)
if err != nil { if err != nil {
metrics.IncrCounterWithLabels([]string{"client", "rpc", "error", "catalog_service_nodes"}, 1, metrics.IncrCounterWithLabels([]string{"client", "rpc", "error", "catalog_service_nodes"}, 1,

View File

@ -6,10 +6,11 @@ import (
"strings" "strings"
"time" "time"
"github.com/mitchellh/mapstructure"
cachetype "github.com/hashicorp/consul/agent/cache-types" cachetype "github.com/hashicorp/consul/agent/cache-types"
"github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/lib/decode" "github.com/hashicorp/consul/lib/decode"
"github.com/mitchellh/mapstructure"
) )
func (s *HTTPHandlers) DiscoveryChainRead(resp http.ResponseWriter, req *http.Request) (interface{}, error) { func (s *HTTPHandlers) DiscoveryChainRead(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
@ -59,7 +60,7 @@ func (s *HTTPHandlers) DiscoveryChainRead(resp http.ResponseWriter, req *http.Re
var out structs.DiscoveryChainResponse var out structs.DiscoveryChainResponse
defer setMeta(resp, &out.QueryMeta) defer setMeta(resp, &out.QueryMeta)
if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache { if args.QueryOptions.UseCache {
raw, m, err := s.agent.cache.Get(req.Context(), cachetype.CompiledDiscoveryChainName, &args) raw, m, err := s.agent.cache.Get(req.Context(), cachetype.CompiledDiscoveryChainName, &args)
if err != nil { if err != nil {
return nil, err return nil, err

View File

@ -18,6 +18,10 @@ import (
"github.com/NYTimes/gziphandler" "github.com/NYTimes/gziphandler"
"github.com/armon/go-metrics" "github.com/armon/go-metrics"
"github.com/armon/go-metrics/prometheus" "github.com/armon/go-metrics/prometheus"
"github.com/hashicorp/go-cleanhttp"
"github.com/mitchellh/mapstructure"
"github.com/pkg/errors"
"github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/cache" "github.com/hashicorp/consul/agent/cache"
"github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/agent/config"
@ -27,9 +31,6 @@ import (
"github.com/hashicorp/consul/api" "github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/lib"
"github.com/hashicorp/consul/logging" "github.com/hashicorp/consul/logging"
"github.com/hashicorp/go-cleanhttp"
"github.com/mitchellh/mapstructure"
"github.com/pkg/errors"
) )
var HTTPSummaries = []prometheus.SummaryDefinition{ var HTTPSummaries = []prometheus.SummaryDefinition{
@ -858,7 +859,7 @@ func (s *HTTPHandlers) parseConsistency(resp http.ResponseWriter, req *http.Requ
if _, ok := query["leader"]; ok { if _, ok := query["leader"]; ok {
defaults = false defaults = false
} }
if _, ok := query["cached"]; ok { if _, ok := query["cached"]; ok && s.agent.config.HTTPUseCache {
b.SetUseCache(true) b.SetUseCache(true)
defaults = false defaults = false
} }
@ -1037,9 +1038,9 @@ func parseMetaPair(raw string) (string, string) {
return pair[0], "" return pair[0], ""
} }
// parseInternal is a convenience method for endpoints that need // parse is a convenience method for endpoints that need to use both parseWait
// to use both parseWait and parseDC. // and parseDC.
func (s *HTTPHandlers) parseInternal(resp http.ResponseWriter, req *http.Request, dc *string, b structs.QueryOptionsCompat) bool { func (s *HTTPHandlers) parse(resp http.ResponseWriter, req *http.Request, dc *string, b structs.QueryOptionsCompat) bool {
s.parseDC(req, dc) s.parseDC(req, dc)
var token string var token string
s.parseTokenWithDefault(req, &token) s.parseTokenWithDefault(req, &token)
@ -1056,12 +1057,6 @@ func (s *HTTPHandlers) parseInternal(resp http.ResponseWriter, req *http.Request
return parseWait(resp, req, b) return parseWait(resp, req, b)
} }
// parse is a convenience method for endpoints that need
// to use both parseWait and parseDC.
func (s *HTTPHandlers) parse(resp http.ResponseWriter, req *http.Request, dc *string, b structs.QueryOptionsCompat) bool {
return s.parseInternal(resp, req, dc, b)
}
func (s *HTTPHandlers) checkWriteAccess(req *http.Request) error { func (s *HTTPHandlers) checkWriteAccess(req *http.Request) error {
if req.Method == http.MethodGet || req.Method == http.MethodHead || req.Method == http.MethodOptions { if req.Method == http.MethodGet || req.Method == http.MethodHead || req.Method == http.MethodOptions {
return nil return nil

View File

@ -120,7 +120,7 @@ func (s *HTTPHandlers) preparedQueryExecute(id string, resp http.ResponseWriter,
var reply structs.PreparedQueryExecuteResponse var reply structs.PreparedQueryExecuteResponse
defer setMeta(resp, &reply.QueryMeta) defer setMeta(resp, &reply.QueryMeta)
if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache { if args.QueryOptions.UseCache {
raw, m, err := s.agent.cache.Get(req.Context(), cachetype.PreparedQueryName, &args) raw, m, err := s.agent.cache.Get(req.Context(), cachetype.PreparedQueryName, &args)
if err != nil { if err != nil {
// Don't return error if StaleIfError is set and we are within it and had // Don't return error if StaleIfError is set and we are within it and had