From dd5c56088c87a4f3c11c4787e7a4f13797da8c8a Mon Sep 17 00:00:00 2001 From: "Xiaochao Dong (@damnever)" Date: Thu, 30 Mar 2023 17:07:30 +0800 Subject: [PATCH] Catch context error in the s3 bucket client Signed-off-by: Xiaochao Dong (@damnever) --- CHANGELOG.md | 1 + pkg/storage/bucket/s3/bucket_client.go | 2 +- pkg/storage/bucket/s3/bucket_client_test.go | 21 ++++++++++++++++++++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bc0ed9dbb1..2941e5b57f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## master / unreleased * [ENHANCEMENT] Querier: Batch Iterator optimization to prevent transversing it multiple times query ranges steps does not overlap. #5237 +* [BUGFIX] Catch context error in the s3 bucket client. #5240 ## 1.15.0 in progress diff --git a/pkg/storage/bucket/s3/bucket_client.go b/pkg/storage/bucket/s3/bucket_client.go index d0625b6deef..ed76d835e5c 100644 --- a/pkg/storage/bucket/s3/bucket_client.go +++ b/pkg/storage/bucket/s3/bucket_client.go @@ -124,7 +124,7 @@ func (b *BucketWithRetries) retry(ctx context.Context, f func() error, operation level.Error(b.logger).Log("msg", "bucket operation fail after retries", "err", lastErr, "operation", operationInfo) return lastErr } - return nil + return retries.Err() } func (b *BucketWithRetries) Name() string { diff --git a/pkg/storage/bucket/s3/bucket_client_test.go b/pkg/storage/bucket/s3/bucket_client_test.go index c62f9107093..bc991b4f8df 100644 --- a/pkg/storage/bucket/s3/bucket_client_test.go +++ b/pkg/storage/bucket/s3/bucket_client_test.go @@ -73,6 +73,25 @@ func TestBucketWithRetries_UploadFailed(t *testing.T) { require.ErrorContains(t, err, "failed upload: ") } +func TestBucketWithRetries_ContextCanceled(t *testing.T) { + t.Parallel() + + m := mockBucket{} + b := BucketWithRetries{ + logger: log.NewNopLogger(), + bucket: &m, + operationRetries: 5, + retryMinBackoff: 10 * time.Millisecond, + retryMaxBackoff: time.Second, + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + obj, err := b.GetRange(ctx, "dummy", 0, 10) + require.ErrorIs(t, err, context.Canceled) + require.Nil(t, obj) +} + type fakeReader struct { } @@ -121,7 +140,7 @@ func (m *mockBucket) Get(ctx context.Context, name string) (io.ReadCloser, error // GetRange mocks objstore.Bucket.GetRange() func (m *mockBucket) GetRange(ctx context.Context, name string, off, length int64) (io.ReadCloser, error) { - return nil, nil + return io.NopCloser(bytes.NewBuffer(bytes.Repeat([]byte{1}, int(length)))), nil } // Exists mocks objstore.Bucket.Exists()