diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e36e85784..9fdde24963 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ * [BUGFIX] Distributor: don't panic when `metric_relabel_configs` in overrides contains null element. #3868 * [BUGFIX] Distributor: don't panic when OTLP histograms don't have any buckets. #3853 * [BUGFIX] Ingester, Compactor: fix panic that can occur when compaction fails. #3955 +* [BUGFIX] Store-gateway: return `Canceled` rather than `Aborted` error when the calling querier cancels the request. #4007 ### Mixin diff --git a/pkg/storegateway/bucket.go b/pkg/storegateway/bucket.go index f3f3dba1c5..55a7b73062 100644 --- a/pkg/storegateway/bucket.go +++ b/pkg/storegateway/bucket.go @@ -817,6 +817,8 @@ func (s *BucketStore) Series(req *storepb.SeriesRequest, srv storepb.Store_Serie code := codes.Aborted if st, ok := status.FromError(errors.Cause(err)); ok { code = st.Code() + } else if errors.Is(err, context.Canceled) { + code = codes.Canceled } err = status.Error(code, err.Error()) }() diff --git a/pkg/storegateway/bucket_test.go b/pkg/storegateway/bucket_test.go index bbf3bf9def..299f36adc8 100644 --- a/pkg/storegateway/bucket_test.go +++ b/pkg/storegateway/bucket_test.go @@ -48,6 +48,8 @@ import ( "github.com/thanos-io/objstore/providers/filesystem" "go.uber.org/atomic" "golang.org/x/exp/slices" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/grafana/mimir/pkg/mimirpb" "github.com/grafana/mimir/pkg/storage/bucket" @@ -1697,6 +1699,108 @@ func TestSeries_ErrorUnmarshallingRequestHints(t *testing.T) { assert.Equal(t, true, regexp.MustCompile(".*unmarshal series request hints.*").MatchString(err.Error())) } +func TestSeries_CanceledRequest(t *testing.T) { + tmpDir := t.TempDir() + bktDir := filepath.Join(tmpDir, "bkt") + bkt, err := filesystem.NewBucket(bktDir) + assert.NoError(t, err) + defer func() { assert.NoError(t, bkt.Close()) }() + + logger := log.NewNopLogger() + instrBkt := objstore.WithNoopInstr(bkt) + fetcher, err := block.NewMetaFetcher(logger, 10, instrBkt, tmpDir, nil, nil) + assert.NoError(t, err) + + store, err := NewBucketStore( + "test", + instrBkt, + fetcher, + tmpDir, + NewChunksLimiterFactory(10000/MaxSamplesPerChunk), + NewSeriesLimiterFactory(0), + newGapBasedPartitioner(mimir_tsdb.DefaultPartitionerMaxGapSize, nil), + 10, + mimir_tsdb.DefaultPostingOffsetInMemorySampling, + indexheader.Config{}, + false, + 0, + hashcache.NewSeriesHashCache(1024*1024), + NewBucketStoreMetrics(nil), + WithLogger(logger), + WithQueryGate(gate.NewBlocking(0)), + ) + assert.NoError(t, err) + defer func() { assert.NoError(t, store.RemoveBlocksAndClose()) }() + + req := &storepb.SeriesRequest{ + MinTime: 0, + MaxTime: 3, + Matchers: []storepb.LabelMatcher{ + {Type: storepb.LabelMatcher_EQ, Name: "foo", Value: "bar"}, + }, + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + srv := newBucketStoreSeriesServer(ctx) + err = store.Series(req, srv) + assert.Error(t, err) + s, ok := status.FromError(err) + assert.True(t, ok) + assert.Equal(t, codes.Canceled, s.Code()) +} + +func TestSeries_InvalidRequest(t *testing.T) { + tmpDir := t.TempDir() + bktDir := filepath.Join(tmpDir, "bkt") + bkt, err := filesystem.NewBucket(bktDir) + assert.NoError(t, err) + defer func() { assert.NoError(t, bkt.Close()) }() + + logger := log.NewNopLogger() + instrBkt := objstore.WithNoopInstr(bkt) + fetcher, err := block.NewMetaFetcher(logger, 10, instrBkt, tmpDir, nil, nil) + assert.NoError(t, err) + + store, err := NewBucketStore( + "test", + instrBkt, + fetcher, + tmpDir, + NewChunksLimiterFactory(10000/MaxSamplesPerChunk), + NewSeriesLimiterFactory(0), + newGapBasedPartitioner(mimir_tsdb.DefaultPartitionerMaxGapSize, nil), + 10, + mimir_tsdb.DefaultPostingOffsetInMemorySampling, + indexheader.Config{}, + false, + 0, + hashcache.NewSeriesHashCache(1024*1024), + NewBucketStoreMetrics(nil), + WithLogger(logger), + ) + assert.NoError(t, err) + defer func() { assert.NoError(t, store.RemoveBlocksAndClose()) }() + + // Use an invalid matcher regex to trigger an error. + req := &storepb.SeriesRequest{ + MinTime: 0, + MaxTime: 3, + Matchers: []storepb.LabelMatcher{ + {Type: storepb.LabelMatcher_RE, Name: "foo", Value: "("}, + }, + } + + srv := newBucketStoreSeriesServer(context.Background()) + err = store.Series(req, srv) + assert.Error(t, err) + s, ok := status.FromError(err) + assert.True(t, ok) + assert.Equal(t, codes.InvalidArgument, s.Code()) + assert.ErrorContains(t, s.Err(), "error parsing regexp: missing closing )") +} + func TestSeries_BlockWithMultipleChunks(t *testing.T) { tmpDir := t.TempDir()