Skip to content

Commit 3fabbce

Browse files
authored
Ignore permission denied error in parquet converter (#6867)
* ignore permission denied error in parquet converter Signed-off-by: yeya24 <[email protected]> * update parquet common to cover upload failure Signed-off-by: yeya24 <[email protected]> --------- Signed-off-by: yeya24 <[email protected]>
1 parent 4457537 commit 3fabbce

File tree

7 files changed

+131
-18
lines changed

7 files changed

+131
-18
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ require (
8383
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822
8484
github.com/oklog/ulid/v2 v2.1.1
8585
github.com/parquet-go/parquet-go v0.25.1
86-
github.com/prometheus-community/parquet-common v0.0.0-20250708150752-0811a700a852
86+
github.com/prometheus-community/parquet-common v0.0.0-20250708210438-f89902fcd994
8787
github.com/prometheus/procfs v0.16.1
8888
github.com/sercand/kuberesolver/v5 v5.1.1
8989
github.com/tjhop/slog-gokit v0.1.4

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -814,8 +814,8 @@ github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndr
814814
github.com/posener/complete v1.2.3/go.mod h1:WZIdtGGp+qx0sLrYKtIRAruyNpv6hFCicSgv7Sy7s/s=
815815
github.com/prashantv/gostub v1.1.0 h1:BTyx3RfQjRHnUWaGF9oQos79AlQ5k8WNktv7VGvVH4g=
816816
github.com/prashantv/gostub v1.1.0/go.mod h1:A5zLQHz7ieHGG7is6LLXLz7I8+3LZzsrV0P1IAHhP5U=
817-
github.com/prometheus-community/parquet-common v0.0.0-20250708150752-0811a700a852 h1:GNUP6g2eSqZbzGTdFK9D1RLQLjZxCkuuA/MkgfB/enQ=
818-
github.com/prometheus-community/parquet-common v0.0.0-20250708150752-0811a700a852/go.mod h1:zJNGzMKctJoOESjRVaNTlPis3C9VcY3cRzNxj6ll3Is=
817+
github.com/prometheus-community/parquet-common v0.0.0-20250708210438-f89902fcd994 h1:xHR2Xex5XWYl5rQKObX8sVqykPXzlL0Rytd9mKo0sss=
818+
github.com/prometheus-community/parquet-common v0.0.0-20250708210438-f89902fcd994/go.mod h1:zJNGzMKctJoOESjRVaNTlPis3C9VcY3cRzNxj6ll3Is=
819819
github.com/prometheus-community/prom-label-proxy v0.11.1 h1:jX+m+BQCNM0z3/P6V6jVxbiDKgugvk91SaICD6bVhT4=
820820
github.com/prometheus-community/prom-label-proxy v0.11.1/go.mod h1:uTeQW+wZ/VPV1LL3IPfvUE++wR2nPLex+Y4RE38Cpis=
821821
github.com/prometheus/alertmanager v0.28.1 h1:BK5pCoAtaKg01BYRUJhEDV1tqJMEtYBGzPw8QdvnnvA=

pkg/parquetconverter/converter.go

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import (
2727
"github.com/thanos-io/thanos/pkg/block/metadata"
2828
"github.com/thanos-io/thanos/pkg/extprom"
2929
"github.com/thanos-io/thanos/pkg/logutil"
30+
"google.golang.org/grpc/codes"
31+
"google.golang.org/grpc/status"
3032

3133
"github.com/cortexproject/cortex/pkg/ring"
3234
"github.com/cortexproject/cortex/pkg/storage/bucket"
@@ -36,6 +38,7 @@ import (
3638
"github.com/cortexproject/cortex/pkg/storage/tsdb/users"
3739
"github.com/cortexproject/cortex/pkg/tenant"
3840
"github.com/cortexproject/cortex/pkg/util"
41+
cortex_errors "github.com/cortexproject/cortex/pkg/util/errors"
3942
util_log "github.com/cortexproject/cortex/pkg/util/log"
4043
"github.com/cortexproject/cortex/pkg/util/services"
4144
"github.com/cortexproject/cortex/pkg/util/validation"
@@ -247,6 +250,10 @@ func (c *Converter) running(ctx context.Context) error {
247250
if ctx.Err() != nil {
248251
return ctx.Err()
249252
}
253+
if c.isCausedByPermissionDenied(err) {
254+
level.Warn(userLogger).Log("msg", "skipping convert user due to PermissionDenied", "err", err)
255+
continue
256+
}
250257
level.Error(userLogger).Log("msg", "failed to convert user", "err", err)
251258
}
252259
}
@@ -474,14 +481,33 @@ func (c *Converter) convertUser(ctx context.Context, logger log.Logger, ring rin
474481
}
475482

476483
func (c *Converter) checkConvertError(userID string, err error) (terminate bool) {
477-
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
484+
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || c.isCausedByPermissionDenied(err) {
478485
terminate = true
479486
} else {
480487
c.metrics.convertBlockFailures.WithLabelValues(userID).Inc()
481488
}
482489
return
483490
}
484491

492+
func (c *Converter) isCausedByPermissionDenied(err error) bool {
493+
cause := errors.Cause(err)
494+
res := cortex_errors.ErrorIs(cause, func(err error) bool {
495+
return c.isPermissionDeniedErr(err)
496+
})
497+
return res
498+
}
499+
500+
func (c *Converter) isPermissionDeniedErr(err error) bool {
501+
if c.bkt.IsAccessDeniedErr(err) {
502+
return true
503+
}
504+
s, ok := status.FromError(err)
505+
if !ok {
506+
return false
507+
}
508+
return s.Code() == codes.PermissionDenied
509+
}
510+
485511
func (c *Converter) ownUser(r ring.ReadRing, userId string) (bool, error) {
486512
if userId == tenant.GlobalMarkersDir {
487513
// __markers__ is reserved for global markers and no tenant should be allowed to have that name.

pkg/parquetconverter/converter_test.go

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ package parquetconverter
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"io"
78
"math/rand"
89
"path"
10+
"strings"
911
"testing"
1012
"time"
1113

@@ -21,6 +23,8 @@ import (
2123
"github.com/thanos-io/objstore/providers/filesystem"
2224
"github.com/thanos-io/thanos/pkg/block"
2325
"github.com/thanos-io/thanos/pkg/block/metadata"
26+
"google.golang.org/grpc/codes"
27+
"google.golang.org/grpc/status"
2428

2529
"github.com/cortexproject/cortex/integration/e2e"
2630
"github.com/cortexproject/cortex/pkg/ring"
@@ -31,6 +35,7 @@ import (
3135
"github.com/cortexproject/cortex/pkg/storage/tsdb/bucketindex"
3236
"github.com/cortexproject/cortex/pkg/storage/tsdb/users"
3337
"github.com/cortexproject/cortex/pkg/util/concurrency"
38+
cortex_errors "github.com/cortexproject/cortex/pkg/util/errors"
3439
"github.com/cortexproject/cortex/pkg/util/flagext"
3540
"github.com/cortexproject/cortex/pkg/util/services"
3641
"github.com/cortexproject/cortex/pkg/util/test"
@@ -269,7 +274,8 @@ func TestConverter_BlockConversionFailure(t *testing.T) {
269274

270275
// Create a mock bucket that wraps the filesystem bucket but fails uploads
271276
mockBucket := &mockBucket{
272-
Bucket: fsBucket,
277+
Bucket: fsBucket,
278+
uploadFailure: fmt.Errorf("mock upload failure"),
273279
}
274280

275281
converter := newConverter(cfg, objstore.WithNoopInstr(mockBucket), storageCfg, []int64{3600000, 7200000}, nil, reg, overrides, nil)
@@ -284,13 +290,94 @@ func TestConverter_BlockConversionFailure(t *testing.T) {
284290
assert.Equal(t, 1.0, testutil.ToFloat64(converter.metrics.convertBlockFailures.WithLabelValues(userID)))
285291
}
286292

293+
func TestConverter_ShouldNotFailOnAccessDenyError(t *testing.T) {
294+
// Create a new registry for testing
295+
reg := prometheus.NewRegistry()
296+
297+
// Create a new converter with test configuration
298+
cfg := Config{
299+
MetaSyncConcurrency: 1,
300+
DataDir: t.TempDir(),
301+
}
302+
logger := log.NewNopLogger()
303+
storageCfg := cortex_tsdb.BlocksStorageConfig{}
304+
flagext.DefaultValues(&storageCfg)
305+
limits := &validation.Limits{}
306+
flagext.DefaultValues(limits)
307+
overrides := validation.NewOverrides(*limits, nil)
308+
limits.ParquetConverterEnabled = true
309+
310+
// Create a filesystem bucket for initial block upload
311+
fsBucket, err := filesystem.NewBucket(t.TempDir())
312+
require.NoError(t, err)
313+
314+
// Create test labels
315+
lbls := labels.Labels{labels.Label{
316+
Name: "__name__",
317+
Value: "test",
318+
}}
319+
320+
// Create a real TSDB block
321+
dir := t.TempDir()
322+
rnd := rand.New(rand.NewSource(time.Now().Unix()))
323+
blockID, err := e2e.CreateBlock(context.Background(), rnd, dir, []labels.Labels{lbls}, 2, 0, 2*time.Hour.Milliseconds(), time.Minute.Milliseconds(), 10)
324+
require.NoError(t, err)
325+
bdir := path.Join(dir, blockID.String())
326+
327+
userID := "test-user"
328+
329+
// Upload the block to filesystem bucket
330+
err = block.Upload(context.Background(), logger, bucket.NewPrefixedBucketClient(fsBucket, userID), bdir, metadata.NoneFunc)
331+
require.NoError(t, err)
332+
333+
var mb *mockBucket
334+
t.Run("get failure", func(t *testing.T) {
335+
// Create a mock bucket that wraps the filesystem bucket but fails with permission denied error.
336+
mb = &mockBucket{
337+
Bucket: fsBucket,
338+
getFailure: cortex_errors.WithCause(errors.New("dummy error"), status.Error(codes.PermissionDenied, "dummy")),
339+
}
340+
})
341+
342+
t.Run("upload failure", func(t *testing.T) {
343+
// Create a mock bucket that wraps the filesystem bucket but fails with permission denied error.
344+
mb = &mockBucket{
345+
Bucket: fsBucket,
346+
uploadFailure: cortex_errors.WithCause(errors.New("dummy error"), status.Error(codes.PermissionDenied, "dummy")),
347+
}
348+
})
349+
350+
converter := newConverter(cfg, objstore.WithNoopInstr(mb), storageCfg, []int64{3600000, 7200000}, nil, reg, overrides, nil)
351+
converter.ringLifecycler = &ring.Lifecycler{
352+
Addr: "1.2.3.4",
353+
}
354+
355+
err = converter.convertUser(context.Background(), logger, &RingMock{ReadRing: &ring.Ring{}}, userID)
356+
require.Error(t, err)
357+
358+
// Verify the failure metric was not incremented
359+
assert.Equal(t, 0.0, testutil.ToFloat64(converter.metrics.convertBlockFailures.WithLabelValues(userID)))
360+
}
361+
287362
// mockBucket implements objstore.Bucket for testing
288363
type mockBucket struct {
289364
objstore.Bucket
365+
uploadFailure error
366+
getFailure error
290367
}
291368

292369
func (m *mockBucket) Upload(ctx context.Context, name string, r io.Reader) error {
293-
return fmt.Errorf("mock upload failure")
370+
if m.uploadFailure != nil {
371+
return m.uploadFailure
372+
}
373+
return m.Bucket.Upload(ctx, name, r)
374+
}
375+
376+
func (m *mockBucket) Get(ctx context.Context, name string) (io.ReadCloser, error) {
377+
if m.getFailure != nil && strings.Contains(name, "index") {
378+
return nil, m.getFailure
379+
}
380+
return m.Bucket.Get(ctx, name)
294381
}
295382

296383
type RingMock struct {

vendor/github.com/prometheus-community/parquet-common/convert/convert.go

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vendor/github.com/prometheus-community/parquet-common/convert/writer.go

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vendor/modules.txt

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)