Return Canceled when a LabelNames or LabelValues request to a s…

…tore-gateway is cancelled by the caller, and return `Internal` otherwise for all requests. (#4061)

* Return Canceled when a LabelNames or LabelValues request to a store-gateway is cancelled by the caller, and return Internal otherwise.

See #4007 for explanation.

* Add changelog entry.

CHANGELOG.md

@@ -17,6 +17,7 @@
 * [ENHANCEMENT] Store-gateway: Reduce memory allocation rate when loading TSDB chunks from Memcached. #4074
 * [ENHANCEMENT] Query-frontend: track `cortex_frontend_query_response_codec_duration_seconds` and `cortex_frontend_query_response_codec_payload_bytes` metrics to measure the time taken and bytes read / written while encoding and decoding query result payloads. #4110
 * [BUGFIX] Ingester: remove series from ephemeral storage even if there are no persistent series. #4052
+* [BUGFIX] Store-gateway: return `Canceled` rather than `Aborted` or `Internal` error when the calling querier cancels a label names or values request, and return `Internal` if processing the request fails for another reason. #4061
 * [BUGFIX] Ingester: reuse memory when ingesting ephemeral series. #4072
 * [BUGFIX] Fix JSON and YAML marshalling of `ephemeral_series_matchers` field in `/runtime_config`. #4091
 * [BUGFIX] Querier: track canceled requests with status code `499` in the metrics instead of `503` or `422`. #4099

pkg/storegateway/bucket.go

@@ -770,7 +770,7 @@ func (s *BucketStore) Series(req *storepb.SeriesRequest, srv storepb.Store_Serie
 		if err == nil {
 			return
 		}
-		code := codes.Aborted
+		code := codes.Internal
 		if st, ok := status.FromError(errors.Cause(err)); ok {
 			code = st.Code()
 		} else if errors.Is(err, context.Canceled) {
@@ -1270,6 +1270,10 @@ func (s *BucketStore) LabelNames(ctx context.Context, req *storepb.LabelNamesReq
 	s.blocksMx.RUnlock()
 
 	if err := g.Wait(); err != nil {
+		if errors.Is(err, context.Canceled) {
+			return nil, status.Error(codes.Canceled, err.Error())
+		}
+
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 
@@ -1431,7 +1435,11 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR
 	s.blocksMx.RUnlock()
 
 	if err := g.Wait(); err != nil {
-		return nil, status.Error(codes.Aborted, err.Error())
+		if errors.Is(err, context.Canceled) {
+			return nil, status.Error(codes.Canceled, err.Error())
+		}
+
+		return nil, status.Error(codes.Internal, err.Error())
 	}
 
 	anyHints, err := types.MarshalAny(resHints)

pkg/storegateway/bucket_test.go

@@ -2376,6 +2376,59 @@ func TestLabelNamesAndValuesHints(t *testing.T) {
 	}
 }
 
+func TestLabelNames_Cancelled(t *testing.T) {
+	_, store, _, _, _, _, close := setupStoreForHintsTest(t)
+	defer close()
+
+	req := &storepb.LabelNamesRequest{
+		Start: 0,
+		End:   1,
+		Matchers: []storepb.LabelMatcher{
+			{
+				Name:  "__name__",
+				Type:  storepb.LabelMatcher_RE,
+				Value: ".*",
+			},
+		},
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	_, err := store.LabelNames(ctx, req)
+	assert.Error(t, err)
+	s, ok := status.FromError(err)
+	assert.True(t, ok)
+	assert.Equal(t, codes.Canceled, s.Code())
+}
+
+func TestLabelValues_Cancelled(t *testing.T) {
+	_, store, _, _, _, _, close := setupStoreForHintsTest(t)
+	defer close()
+
+	req := &storepb.LabelValuesRequest{
+		Label: "ext1",
+		Start: 0,
+		End:   1,
+		Matchers: []storepb.LabelMatcher{
+			{
+				Name:  "__name__",
+				Type:  storepb.LabelMatcher_RE,
+				Value: ".*",
+			},
+		},
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	_, err := store.LabelValues(ctx, req)
+	assert.Error(t, err)
+	s, ok := status.FromError(err)
+	assert.True(t, ok)
+	assert.Equal(t, codes.Canceled, s.Code())
+}
+
 func labelNamesFromSeriesSet(series []*storepb.Series) []string {
 	labelsMap := map[string]struct{}{}