Skip to content

Commit

Permalink
Return Canceled rather than Aborted when a Series request to a …
Browse files Browse the repository at this point in the history
…store-gateway is cancelled by the calling querier. (#4007)

* Return Canceled rather than Aborted when a Series request to a store-gateway is cancelled.

* Add changelog entry.
  • Loading branch information
charleskorn committed Jan 19, 2023
1 parent 5fe3b80 commit 20ea888
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions pkg/storegateway/bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}()
Expand Down
104 changes: 104 additions & 0 deletions pkg/storegateway/bucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()

Expand Down

0 comments on commit 20ea888

Please sign in to comment.