diff --git a/CHANGELOG.md b/CHANGELOG.md index 9df155d04cc..cb8f4af4698 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## master / unreleased +* [CHANGE] The frontend http server will now send 502 in case of deadline exceeded and 499 if the user requested cancellation. #2156 * [CHANGE] Config file changed to remove top level `config_store` field in favor of a nested `configdb` field. #2125 * [CHANGE] Removed unnecessary `frontend.cache-split-interval` in favor of `querier.split-queries-by-interval` both to reduce configuration complexity and guarantee alignment of these two configs. Starting from now, `-querier.cache-results` may only be enabled in conjunction with `-querier.split-queries-by-interval` (previously the cache interval default was `24h` so if you want to preserve the same behaviour you should set `-querier.split-queries-by-interval=24h`). #2040 * [CHANGE] Removed remaining support for using denormalised tokens in the ring. If you're still running ingesters with denormalised tokens (Cortex 0.4 or earlier, with `-ingester.normalise-tokens=false`), such ingesters will now be completely invisible to distributors and need to be either switched to Cortex 0.6.0 or later, or be configured to use normalised tokens. #2034 diff --git a/pkg/querier/frontend/frontend.go b/pkg/querier/frontend/frontend.go index 9e53924a72a..2f532daf063 100644 --- a/pkg/querier/frontend/frontend.go +++ b/pkg/querier/frontend/frontend.go @@ -38,8 +38,12 @@ var ( Help: "Number of queries in the queue.", }) - errTooManyRequest = httpgrpc.Errorf(http.StatusTooManyRequests, "too many outstanding requests") - errCanceled = httpgrpc.Errorf(http.StatusInternalServerError, "context cancelled") + // StatusClientClosedRequest is the status code for when a client request cancellation of an http request + StatusClientClosedRequest = 499 + + errTooManyRequest = httpgrpc.Errorf(http.StatusTooManyRequests, "too many outstanding requests") + errCanceled = httpgrpc.Errorf(StatusClientClosedRequest, context.Canceled.Error()) + errDeadlineExceeded = httpgrpc.Errorf(http.StatusGatewayTimeout, context.DeadlineExceeded.Error()) ) // Config for a Frontend. @@ -151,14 +155,14 @@ func (f *Frontend) handle(w http.ResponseWriter, r *http.Request) { startTime := time.Now() resp, err := f.roundTripper.RoundTrip(r) - queryResponseTime := time.Now().Sub(startTime) + queryResponseTime := time.Since(startTime) if f.cfg.LogQueriesLongerThan > 0 && queryResponseTime > f.cfg.LogQueriesLongerThan { level.Info(f.log).Log("msg", "slow query", "org_id", userID, "url", fmt.Sprintf("http://%s", r.Host+r.RequestURI), "time_taken", queryResponseTime.String()) } if err != nil { - server.WriteError(w, err) + writeError(w, err) return } @@ -170,6 +174,17 @@ func (f *Frontend) handle(w http.ResponseWriter, r *http.Request) { io.Copy(w, resp.Body) } +func writeError(w http.ResponseWriter, err error) { + switch err { + case context.Canceled: + err = errCanceled + case context.DeadlineExceeded: + err = errDeadlineExceeded + default: + } + server.WriteError(w, err) +} + // RoundTrip implement http.Transport. func (f *Frontend) RoundTrip(r *http.Request) (*http.Response, error) { req, err := server.HTTPRequest(r) @@ -230,7 +245,7 @@ func (f *Frontend) RoundTripGRPC(ctx context.Context, req *ProcessRequest) (*Pro select { case <-ctx.Done(): - return nil, errCanceled + return nil, ctx.Err() case resp := <-request.response: return resp, nil diff --git a/pkg/querier/frontend/frontend_test.go b/pkg/querier/frontend/frontend_test.go index b1d1ff65448..6e10756c195 100644 --- a/pkg/querier/frontend/frontend_test.go +++ b/pkg/querier/frontend/frontend_test.go @@ -2,10 +2,12 @@ package frontend import ( "context" + "errors" "fmt" "io/ioutil" "net" "net/http" + "net/http/httptest" "sync/atomic" "testing" "time" @@ -18,6 +20,7 @@ import ( "github.com/stretchr/testify/require" jaeger "github.com/uber/jaeger-client-go" "github.com/uber/jaeger-client-go/config" + "github.com/weaveworks/common/httpgrpc" httpgrpc_server "github.com/weaveworks/common/httpgrpc/server" "github.com/weaveworks/common/middleware" "github.com/weaveworks/common/user" @@ -133,6 +136,24 @@ func TestFrontendCancel(t *testing.T) { testFrontend(t, handler, test) } +func TestFrontendCancelStatusCode(t *testing.T) { + for _, test := range []struct { + status int + err error + }{ + {http.StatusInternalServerError, errors.New("unknown")}, + {http.StatusGatewayTimeout, context.DeadlineExceeded}, + {StatusClientClosedRequest, context.Canceled}, + {http.StatusBadRequest, httpgrpc.Errorf(http.StatusBadRequest, "")}, + } { + t.Run(test.err.Error(), func(t *testing.T) { + w := httptest.NewRecorder() + writeError(w, test.err) + require.Equal(t, test.status, w.Result().StatusCode) + }) + } +} + func defaultOverrides(t *testing.T) *validation.Overrides { var limits validation.Limits flagext.DefaultValues(&limits)