From aa3dd5c435bde5d2d511ca4cab7a37c568563ccc Mon Sep 17 00:00:00 2001 From: SungJin1212 Date: Fri, 23 Oct 2026 19:17:50 +0900 Subject: [PATCH 1/3] Implement hybrid mode Store Gateway Signed-off-by: SungJin1212 --- CHANGELOG.md | 1 + docs/blocks-storage/querier.md | 12 +- docs/blocks-storage/store-gateway.md | 12 +- docs/configuration/config-file-reference.md | 12 +- docs/configuration/v1-guarantees.md | 3 +- docs/guides/parquet-mode.md | 20 + integration/parquet_querier_test.go | 493 +++++++++- pkg/storage/tsdb/config.go | 9 +- pkg/storage/tsdb/config_test.go | 22 + .../bucket_index_metadata_fetcher.go | 3 +- .../bucket_index_metadata_fetcher_test.go | 8 + .../bucket_store_inmemory_server.go | 49 +- .../bucket_store_streaming_server.go | 126 +++ .../bucket_store_streaming_server_test.go | 96 ++ pkg/storegateway/bucket_stores.go | 84 +- pkg/storegateway/bucket_stores_test.go | 5 + pkg/storegateway/hybrid_bucket_stores.go | 541 +++++++++++ pkg/storegateway/hybrid_bucket_stores_test.go | 898 ++++++++++++++++++ pkg/storegateway/metadata_fetcher_filters.go | 58 ++ .../metadata_fetcher_filters_test.go | 46 + pkg/storegateway/parquet_bucket_stores.go | 39 +- .../parquet_bucket_stores_test.go | 30 +- schemas/cortex-config-schema.json | 8 +- 23 files changed, 2501 insertions(+), 74 deletions(-) create mode 100644 pkg/storegateway/bucket_store_streaming_server.go create mode 100644 pkg/storegateway/bucket_store_streaming_server_test.go create mode 100644 pkg/storegateway/hybrid_bucket_stores.go create mode 100644 pkg/storegateway/hybrid_bucket_stores_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 3497bb1a6ea..1ba665c1cae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * [FEATURE] Ruler: Add experimental support for federated rule groups. A rule group listing tenants in its `source_tenants` field is evaluated against those tenants while the resulting series and alerts are written to the tenant owning the rule group. Enabled with `-ruler.enable-federated-rules` (requires `-tenant-federation.enabled`), and restricted to selected tenants with `-ruler.allowed-federated-tenants` and `-ruler.disallowed-federated-tenants`. #7828 * [FEATURE] Distributor: Add experimental OTLP metrics ingestion over gRPC (`opentelemetry.proto.collector.metrics.v1.MetricsService/Export`) on the gRPC server port. Enable it with `-distributor.otlp.grpc-enabled`. The tenant is read from the `X-Scope-OrgID` gRPC metadata, and the request size is limited by `-server.grpc-max-recv-msg-size-bytes`. #7873 +* [FEATURE] StoreGateway: Add a hybrid mode to the parquet bucket store (`-blocks-storage.bucket-store.bucket-store-type=parquet`), which serves blocks not yet converted to Parquet from TSDB blocks so they stay queryable. Hybrid mode is the default and requires the bucket index. Set `-blocks-storage.bucket-store.parquet-fallback-disabled=true` to keep the previous Parquet-only behavior. #7689 * [ENHANCEMENT] Ruler: Add rule information to query parameters sent to the Query Frontend when `-ruler.frontend-address` is configured, so the Query Frontend query stats logs (`-frontend.enabled-ruler-query-stats`) can identify the evaluated rule. In this mode, the Ruler query stats log (`-ruler.query-stats-enabled`) omits the fetched statistics fields, which are always zero. #6539 * [ENHANCEMENT] gRPC: When `-auth.enabled=true`, a gRPC request without the `X-Scope-OrgID` metadata is now rejected with the `UNAUTHENTICATED` status code instead of `UNKNOWN`. #7873 * [ENHANCEMENT] Upgrade promql-engine and Thanos to latest. #7889 diff --git a/docs/blocks-storage/querier.md b/docs/blocks-storage/querier.md index 166b8f43635..a54d85c8ffb 100644 --- a/docs/blocks-storage/querier.md +++ b/docs/blocks-storage/querier.md @@ -2102,10 +2102,20 @@ blocks_storage: # CLI flag: -blocks-storage.bucket-store.block-discovery-strategy [block_discovery_strategy: | default = "concurrent"] - # Type of bucket store to use (tsdb or parquet). + # Type of bucket store to use (tsdb or parquet). With parquet, blocks + # converted to Parquet are served from Parquet files and blocks not yet + # converted are served from TSDB blocks, unless + # -blocks-storage.bucket-store.parquet-fallback-disabled is set. # CLI flag: -blocks-storage.bucket-store.bucket-store-type [bucket_store_type: | default = "tsdb"] + # [Experimental] Only used when the bucket store type is parquet. If true, + # the store-gateway serves only blocks converted to Parquet and does not + # fall back to TSDB blocks for blocks not converted yet, so it keeps no + # local state. If false, the bucket index must be enabled. + # CLI flag: -blocks-storage.bucket-store.parquet-fallback-disabled + [parquet_fallback_disabled: | default = false] + # Max size - in bytes - of a chunks pool, used to reduce memory allocations. # The pool is shared across all tenants. 0 to disable the limit. # CLI flag: -blocks-storage.bucket-store.max-chunk-pool-bytes diff --git a/docs/blocks-storage/store-gateway.md b/docs/blocks-storage/store-gateway.md index 8c43bcb4653..4ccb72bf968 100644 --- a/docs/blocks-storage/store-gateway.md +++ b/docs/blocks-storage/store-gateway.md @@ -2155,10 +2155,20 @@ blocks_storage: # CLI flag: -blocks-storage.bucket-store.block-discovery-strategy [block_discovery_strategy: | default = "concurrent"] - # Type of bucket store to use (tsdb or parquet). + # Type of bucket store to use (tsdb or parquet). With parquet, blocks + # converted to Parquet are served from Parquet files and blocks not yet + # converted are served from TSDB blocks, unless + # -blocks-storage.bucket-store.parquet-fallback-disabled is set. # CLI flag: -blocks-storage.bucket-store.bucket-store-type [bucket_store_type: | default = "tsdb"] + # [Experimental] Only used when the bucket store type is parquet. If true, + # the store-gateway serves only blocks converted to Parquet and does not + # fall back to TSDB blocks for blocks not converted yet, so it keeps no + # local state. If false, the bucket index must be enabled. + # CLI flag: -blocks-storage.bucket-store.parquet-fallback-disabled + [parquet_fallback_disabled: | default = false] + # Max size - in bytes - of a chunks pool, used to reduce memory allocations. # The pool is shared across all tenants. 0 to disable the limit. # CLI flag: -blocks-storage.bucket-store.max-chunk-pool-bytes diff --git a/docs/configuration/config-file-reference.md b/docs/configuration/config-file-reference.md index 628ee3f9027..c0296ccbb2a 100644 --- a/docs/configuration/config-file-reference.md +++ b/docs/configuration/config-file-reference.md @@ -2799,10 +2799,20 @@ bucket_store: # CLI flag: -blocks-storage.bucket-store.block-discovery-strategy [block_discovery_strategy: | default = "concurrent"] - # Type of bucket store to use (tsdb or parquet). + # Type of bucket store to use (tsdb or parquet). With parquet, blocks + # converted to Parquet are served from Parquet files and blocks not yet + # converted are served from TSDB blocks, unless + # -blocks-storage.bucket-store.parquet-fallback-disabled is set. # CLI flag: -blocks-storage.bucket-store.bucket-store-type [bucket_store_type: | default = "tsdb"] + # [Experimental] Only used when the bucket store type is parquet. If true, the + # store-gateway serves only blocks converted to Parquet and does not fall back + # to TSDB blocks for blocks not converted yet, so it keeps no local state. If + # false, the bucket index must be enabled. + # CLI flag: -blocks-storage.bucket-store.parquet-fallback-disabled + [parquet_fallback_disabled: | default = false] + # Max size - in bytes - of a chunks pool, used to reduce memory allocations. # The pool is shared across all tenants. 0 to disable the limit. # CLI flag: -blocks-storage.bucket-store.max-chunk-pool-bytes diff --git a/docs/configuration/v1-guarantees.md b/docs/configuration/v1-guarantees.md index 9bba2b68bad..2d0cfd51246 100644 --- a/docs/configuration/v1-guarantees.md +++ b/docs/configuration/v1-guarantees.md @@ -165,7 +165,8 @@ Currently experimental features are: ring configuration - Querier: `-querier.parquet-queryable-default-block-store`, `-querier.parquet-queryable-fallback-disabled`, the `-querier.parquet-queryable.max-fetched-*` limits and `-querier.parquet-shard-cache-*` - - Store Gateway: `-blocks-storage.bucket-store.parquet-query-concurrency`, + - Store Gateway: `-blocks-storage.bucket-store.parquet-fallback-disabled`, + `-blocks-storage.bucket-store.parquet-query-concurrency`, `-blocks-storage.bucket-store.parquet-shard-cache-*` and the `-blocks-storage.bucket-store.parquet-labels-cache.*` / `-blocks-storage.bucket-store.parquet-row-ranges-cache.*` cache configuration diff --git a/docs/guides/parquet-mode.md b/docs/guides/parquet-mode.md index 653d43a6b05..d2b4513e7d0 100644 --- a/docs/guides/parquet-mode.md +++ b/docs/guides/parquet-mode.md @@ -142,6 +142,26 @@ querier: parquet_queryable_fallback_disabled: false ``` +### Querying Parquet Through the Store Gateway + +As an alternative to the parquet queryable, the store-gateway can serve Parquet files while the querier keeps querying the store-gateway as usual: + +```yaml +blocks_storage: + bucket_store: + bucket_store_type: parquet + + # Disable fallback to TSDB blocks for blocks not converted to Parquet yet + parquet_fallback_disabled: false + + bucket_index: + enabled: true +``` + +By default the store-gateway runs in hybrid mode: blocks converted to Parquet are served from Parquet files, and blocks not converted yet are served from TSDB blocks, so they stay queryable while the conversion is pending. The store-gateway syncs only the not-yet-converted blocks, so it keeps index-headers on local disk for those blocks only. Hybrid mode requires the bucket index to be enabled. + +When `parquet_fallback_disabled` is `true`, the store-gateway serves only Parquet files and keeps no local state. Queries fail for blocks that have not been converted yet, so enable it only when every queried block is available as Parquet. + ### Query Limits for Parquet Configure query limits specific to parquet operations: diff --git a/integration/parquet_querier_test.go b/integration/parquet_querier_test.go index 8c727cfbe62..efdaac9deb7 100644 --- a/integration/parquet_querier_test.go +++ b/integration/parquet_querier_test.go @@ -3,17 +3,27 @@ package integration import ( + "bytes" "context" + "encoding/json" "fmt" + "path" "path/filepath" "slices" + "sort" "strconv" + "strings" "testing" "time" "github.com/cortexproject/promqlsmith" + "github.com/oklog/ulid/v2" + "github.com/prometheus-community/parquet-common/convert" "github.com/prometheus/common/model" + "github.com/prometheus/common/promslog" "github.com/prometheus/prometheus/model/labels" + prom_tsdb "github.com/prometheus/prometheus/tsdb" + "github.com/prometheus/prometheus/tsdb/chunkenc" "github.com/stretchr/testify/require" "github.com/thanos-io/objstore" "github.com/thanos-io/thanos/pkg/block" @@ -405,15 +415,18 @@ func TestParquetProjectionPushdownFuzz(t *testing.T) { func TestParquetMultiShardQuery(t *testing.T) { for name, tc := range map[string]struct { viaStoreGateway bool + // fallbackDisabled runs the store-gateway in Parquet-only mode (no TSDB sub-store). + fallbackDisabled bool // replicationFactor is the store-gateway sharding ring replication factor. replicationFactor string // extraStoreGateways is the number of additional store-gateway replicas to // start alongside the one embedded in the single binary (target "all"). extraStoreGateways int }{ - "querier parquet queryable": {viaStoreGateway: false, replicationFactor: "1", extraStoreGateways: 0}, - "store-gateway parquet bucket store": {viaStoreGateway: true, replicationFactor: "1", extraStoreGateways: 0}, - "store-gateway parquet bucket store with replication": {viaStoreGateway: true, replicationFactor: "2", extraStoreGateways: 1}, + "querier parquet queryable": {viaStoreGateway: false, replicationFactor: "1", extraStoreGateways: 0}, + "store-gateway parquet bucket store": {viaStoreGateway: true, replicationFactor: "1", extraStoreGateways: 0}, + "store-gateway parquet bucket store with replication": {viaStoreGateway: true, replicationFactor: "2", extraStoreGateways: 1}, + "store-gateway parquet bucket store with fallback disabled": {viaStoreGateway: true, fallbackDisabled: true, replicationFactor: "1", extraStoreGateways: 0}, } { t.Run(name, func(t *testing.T) { s, err := e2e.NewScenario(networkName) @@ -469,7 +482,8 @@ func TestParquetMultiShardQuery(t *testing.T) { if tc.viaStoreGateway { // Route reads through the store-gateway's parquet bucket store. flags = mergeFlags(flags, map[string]string{ - "-blocks-storage.bucket-store.bucket-store-type": "parquet", + "-blocks-storage.bucket-store.bucket-store-type": "parquet", + "-blocks-storage.bucket-store.parquet-fallback-disabled": strconv.FormatBool(tc.fallbackDisabled), // Enable sharding so the querier discovers the store-gateway via the // ring and routes block queries to it. "-store-gateway.sharding-enabled": "true", @@ -610,6 +624,12 @@ func TestParquetMultiShardQuery(t *testing.T) { return len(labelSets) == totalSeries }) + if tc.viaStoreGateway && !tc.fallbackDisabled { + // wait until the parquet block is converted + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.GreaterOrEqual(1), []string{"cortex_blocks_meta_synced"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "state", "parquet-converted")))) + } + rangeRes, err := c.QueryRange(`test_series_a`, start, end, scrapeInterval) require.NoError(t, err) rangeMatrix, ok := rangeRes.(model.Matrix) @@ -627,6 +647,18 @@ func TestParquetMultiShardQuery(t *testing.T) { require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Greater(0), []string{"cortex_bucket_index_loads_total"}, e2e.WithLabelMatchers( labels.MustNewMatcher(labels.MatchEqual, "component", "store-gateway")))) } + + hybridOps, err := cortex.SumMetrics([]string{"cortex_hybrid_bucket_stores_operations_total"}, e2e.SkipMissingMetrics) + require.NoError(t, err) + if tc.fallbackDisabled { + // Parquet-only mode never creates the TSDB sub-store: no TSDB block sync and no hybrid routing. + tsdbSynced, err := cortex.SumMetrics([]string{"cortex_blocks_meta_synced"}, e2e.SkipMissingMetrics) + require.NoError(t, err) + require.Zero(t, tsdbSynced[0], "the TSDB sub-store must not sync blocks when the parquet fallback is disabled") + require.Zero(t, hybridOps[0], "queries must not go through the hybrid store when the parquet fallback is disabled") + } else if tc.extraStoreGateways == 0 { + require.Positive(t, hybridOps[0], "queries must go through the hybrid store by default") + } } else { require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Greater(0), []string{"cortex_parquet_queryable_blocks_queried_total"}, e2e.WithLabelMatchers( labels.MustNewMatcher(labels.MatchEqual, "type", "parquet")))) @@ -637,3 +669,456 @@ func TestParquetMultiShardQuery(t *testing.T) { }) } } + +// hybridStoreGatewayFlags returns the flags shared by the store-gateway hybrid mode tests: a single +// binary without the parquet-converter service (blocks are converted manually), reading blocks +// through the store-gateway Parquet bucket store. +func hybridStoreGatewayFlags(consulEndpoint string) map[string]string { + return mergeFlags(AlertmanagerLocalFlags(), BlocksStorageFlags(), map[string]string{ + "-target": "all", + "-blocks-storage.tsdb.block-ranges-period": "1m,24h", + "-blocks-storage.tsdb.ship-interval": "1s", + "-blocks-storage.bucket-store.sync-interval": "1s", + "-blocks-storage.bucket-store.metadata-cache.bucket-index-content-ttl": "1s", + "-blocks-storage.bucket-store.bucket-index.idle-timeout": "1s", + "-blocks-storage.bucket-store.bucket-index.enabled": "true", + // Route reads through the store-gateway Parquet (hybrid) bucket store. + "-blocks-storage.bucket-store.bucket-store-type": "parquet", + "-compactor.cleanup-interval": "1s", + "-ring.store": "consul", + "-consul.hostname": consulEndpoint, + "-distributor.replication-factor": "1", + "-store-gateway.sharding-enabled": "true", + "-store-gateway.sharding-ring.store": "consul", + "-store-gateway.sharding-ring.consul.hostname": consulEndpoint, + "-store-gateway.sharding-ring.replication-factor": "1", + "-querier.enable-parquet-queryable": "false", + // Keep the queried range older than this so all data is served from blocks, not ingesters. + "-limits.query-ingesters-within": "2h", + "-alertmanager.web.external-url": "http://localhost/alertmanager", + // Enables EnableParquet() in the compactor's bucket index updater so manually-created + // Parquet markers are honoured in the bucket index. + "-parquet-converter.enabled": "true", + }) +} + +// convertBlockToParquet converts the local TSDB block id to Parquet in object storage and writes +// the converter markers the store-gateway and the bucket index updater look for. +func convertBlockToParquet(t *testing.T, ctx context.Context, userBkt objstore.InstrumentedBucket, dir string, id ulid.ULID) { + t.Helper() + + tsdbBlock, err := prom_tsdb.OpenBlock(nil, filepath.Join(dir, id.String()), chunkenc.NewPool(), prom_tsdb.DefaultPostingsDecoderFactory) + require.NoError(t, err) + numShards, convertErr := convert.ConvertTSDBBlock(ctx, userBkt, + tsdbBlock.MinTime(), tsdbBlock.MaxTime(), + []convert.Convertible{tsdbBlock}, promslog.NewNopLogger(), + convert.WithName(id.String())) + require.NoError(t, tsdbBlock.Close()) + require.NoError(t, convertErr) + + markerBytes, err := json.Marshal(cortex_parquet.ConverterMark{ + Version: cortex_parquet.CurrentVersion, + Shards: numShards, + }) + require.NoError(t, err) + require.NoError(t, userBkt.Upload(ctx, path.Join(id.String(), cortex_parquet.ConverterMarkerFileName), bytes.NewReader(markerBytes))) + // Upload at the global marker path so the bucket index updater discovers it. + require.NoError(t, userBkt.Upload(ctx, bucketindex.ConverterMarkFilePath(id), strings.NewReader("{}"))) +} + +// waitBlockParquetState waits until the bucket index records block id with the expected Parquet state. +func waitBlockParquetState(t *testing.T, ctx context.Context, bkt objstore.Bucket, userID string, id ulid.ULID, parquet bool) { + t.Helper() + + cortex_testutil.Poll(t, 60*time.Second, true, func() any { + idx, err := bucketindex.ReadIndex(ctx, bkt, userID, nil, log.Logger) + if err != nil { + return false + } + for _, b := range idx.Blocks { + if b.ID == id { + return (b.Parquet != nil) == parquet + } + } + return false + }) +} + +func TestParquetStoreGateway_HybridMode(t *testing.T) { + s, err := e2e.NewScenario(networkName) + require.NoError(t, err) + defer s.Close() + + consul := e2edb.NewConsulWithName("consul") + require.NoError(t, s.StartAndWaitReady(consul)) + + flags := hybridStoreGatewayFlags(consul.NetworkHTTPEndpoint()) + + require.NoError(t, writeFileToSharedDir(s, "alertmanager_configs", []byte{})) + + const ( + userID = "user-1" + metricParquet = "series_parquet" + metricTSDB = "series_tsdb" + metricMerge = "series_merge" + numSamples = 60 + ) + + ctx := context.Background() + rnd := newFuzzRand(t) + dir := filepath.Join(s.SharedDir(), "data") + scrapeInterval := time.Minute + now := time.Now() + // Both time ranges must be older than -limits.query-ingesters-within (2h). + midPoint := now.Add(-time.Hour * 10) + start := now.Add(-time.Hour * 24) + end := now.Add(-time.Hour * 3) + + // Block A: series_parquet [start, midPoint) — will be converted to Parquet block. + // Also carries two series_merge series (labeled "pk", a Parquet-only label) to verify + // that Series/LabelNames/LabelValues correctly merge results across both blocks. + idA, err := e2e.CreateBlock(ctx, rnd, dir, + []labels.Labels{ + labels.FromStrings(labels.MetricName, metricParquet, "job", "test"), + labels.FromStrings(labels.MetricName, metricMerge, "job", "test", "series", "a", "pk", "1"), + labels.FromStrings(labels.MetricName, metricMerge, "job", "test", "series", "c", "pk", "1"), + }, + numSamples, start.UnixMilli(), midPoint.UnixMilli(), scrapeInterval.Milliseconds(), 10) + require.NoError(t, err) + + // Block B: series_tsdb [midPoint, end) — stays as TSDB block. + // Also carries three series_merge series (labeled "tk", a TSDB-only label). The "series" + // value "c" is shared with block A (under a different label set) to exercise + // de-duplication in the merged LabelValues response. + idB, err := e2e.CreateBlock(ctx, rnd, dir, + []labels.Labels{ + labels.FromStrings(labels.MetricName, metricTSDB, "job", "test"), + labels.FromStrings(labels.MetricName, metricMerge, "job", "test", "series", "b", "tk", "1"), + labels.FromStrings(labels.MetricName, metricMerge, "job", "test", "series", "c", "tk", "1"), + labels.FromStrings(labels.MetricName, metricMerge, "job", "test", "series", "d", "tk", "1"), + }, + numSamples, midPoint.UnixMilli(), end.UnixMilli(), scrapeInterval.Milliseconds(), 10) + require.NoError(t, err) + + minio := e2edb.NewMinio(9000, flags["-blocks-storage.s3.bucket-name"]) + require.NoError(t, s.StartAndWaitReady(minio)) + + storage, err := e2ecortex.NewS3ClientForMinio(minio, flags["-blocks-storage.s3.bucket-name"]) + require.NoError(t, err) + userBkt := bucket.NewUserBucketClient(userID, storage.GetBucket(), nil) + + // Upload both TSDB blocks to object storage. + require.NoError(t, block.Upload(ctx, log.Logger, userBkt, filepath.Join(dir, idA.String()), metadata.NoneFunc)) + require.NoError(t, block.Upload(ctx, log.Logger, userBkt, filepath.Join(dir, idB.String()), metadata.NoneFunc)) + + // Manually convert block A to Parquet in object storage. + convertBlockToParquet(t, ctx, userBkt, dir, idA) + + cortex := e2ecortex.NewSingleBinary("cortex", flags, "") + require.NoError(t, s.StartAndWaitReady(cortex)) + + c, err := e2ecortex.NewClient("", cortex.HTTPEndpoint(), "", "", userID) + require.NoError(t, err) + + // Wait until the compactor has built the bucket index with the correct state: + // block A must be tagged as Parquet, block B must be TSDB. + waitBlockParquetState(t, ctx, storage.GetBucket(), userID, idA, true) + waitBlockParquetState(t, ctx, storage.GetBucket(), userID, idB, false) + + // Wait until both metrics are queryable via the store-gateway. + cortex_testutil.Poll(t, 120*time.Second, true, func() any { + labelSets, err := c.Series([]string{`{job="test"}`}, start, end) + if err != nil { + return false + } + foundParquet, foundTSDB := false, false + for _, ls := range labelSets { + switch string(ls[model.MetricNameLabel]) { + case metricParquet: + foundParquet = true + case metricTSDB: + foundTSDB = true + } + } + return foundParquet && foundTSDB + }) + + // series_parquet must be served by the Parquet store. + resParquet, err := c.QueryRange(metricParquet, start, midPoint, scrapeInterval) + require.NoError(t, err) + matrixParquet, ok := resParquet.(model.Matrix) + require.True(t, ok) + require.Len(t, matrixParquet, 1, "series_parquet must return one series (served from Parquet store)") + + // series_tsdb must be served by the TSDB store. + resTSDB, err := c.QueryRange(metricTSDB, midPoint, end, scrapeInterval) + require.NoError(t, err) + matrixTSDB, ok := resTSDB.(model.Matrix) + require.True(t, ok) + require.Len(t, matrixTSDB, 1, "series_tsdb must return one series (served from TSDB store)") + + // Series() must return the union of series_merge series from both the Parquet block + // (pk-labeled: a, c) and the TSDB block (tk-labeled: b, c, d). + mergeMatcher := fmt.Sprintf(`{__name__=%q}`, metricMerge) + mergedSeries, err := c.Series([]string{mergeMatcher}, start, end) + require.NoError(t, err) + require.Len(t, mergedSeries, 5, "series_merge must return the union of series from both stores") + var gotSeriesValues []string + for _, ls := range mergedSeries { + gotSeriesValues = append(gotSeriesValues, string(ls["series"])) + } + sort.Strings(gotSeriesValues) + require.Equal(t, []string{"a", "b", "c", "c", "d"}, gotSeriesValues, + "series_merge series values must include both blocks' series (c appears twice, once per block)") + + // LabelNames() must merge label names from both stores: "pk" only exists in the Parquet + // block, "tk" only in the TSDB block. + names, err := c.LabelNames(start, end, mergeMatcher) + require.NoError(t, err) + require.Equal(t, model.LabelNames{labels.MetricName, "job", "pk", "series", "tk"}, names, + "LabelNames must merge Parquet-only and TSDB-only label names") + + // LabelValues() must merge and de-duplicate values from both stores: "c" is present in + // both blocks but must appear only once in the merged, sorted result. + values, err := c.LabelValues("series", start, end, []string{mergeMatcher}) + require.NoError(t, err) + require.Equal(t, model.LabelValues{"a", "b", "c", "d"}, values, + "LabelValues must merge and de-duplicate values across both stores") + + // TSDB sub-store must have loaded only block B (loaded=1) and excluded block A (parquet-converted=1). + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Equals(1), []string{"cortex_blocks_meta_synced"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "state", "loaded")))) + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Equals(1), []string{"cortex_blocks_meta_synced"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "state", "parquet-converted")))) + + // Every query above spans both blocks, so each method must have merged both sub-stores. + for _, method := range []string{"Series", "LabelNames", "LabelValues"} { + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Greater(0), []string{"cortex_hybrid_bucket_stores_operations_total"}, + e2e.WithLabelMatchers( + labels.MustNewMatcher(labels.MatchEqual, "store", "mixed"), + labels.MustNewMatcher(labels.MatchEqual, "method", method)))) + } +} + +func TestParquetStoreGateway_HybridModeFuzz(t *testing.T) { + s, err := e2e.NewScenario(networkName) + require.NoError(t, err) + defer s.Close() + + consul := e2edb.NewConsulWithName("consul") + require.NoError(t, s.StartAndWaitReady(consul)) + + flags := mergeFlags(hybridStoreGatewayFlags(consul.NetworkHTTPEndpoint()), map[string]string{ + "-blocks-storage.bucket-store.index-cache.backend": tsdb.IndexCacheBackendInMemory, + "-frontend.query-vertical-shard-size": "3", + // Batch series responses so the mixed path's batched merge is exercised as well. + "-querier.store-gateway-series-batch-size": "3", + }) + + require.NoError(t, writeFileToSharedDir(s, "alertmanager_configs", []byte{})) + + const ( + userID = "user-1" + numShared = 6 // series present in both blocks + numPOnly = 4 // series present only in the Parquet block + numTOnly = 4 // series present only in the TSDB block + numSamples = 60 + ) + + ctx := context.Background() + rnd := newFuzzRand(t) + dir := filepath.Join(s.SharedDir(), "data") + scrapeInterval := time.Minute + statusCodes := []string{"200", "400", "404", "500", "502"} + + now := time.Now() + // The whole range must be older than -limits.query-ingesters-within (2h). Block A covers + // [start, mid) and is converted to Parquet; block B covers [mid, end) and stays TSDB. + start := now.Add(-time.Hour * 24) + mid := now.Add(-time.Hour * 13) + end := now.Add(-time.Hour * 3) + + // shared: same series in both blocks. In the store-gateway these are served by the Parquet + // store (block A) and the TSDB store (block B) and their chunks are merged for the same + // series. + sharedLbls := make([]labels.Labels, 0, numShared) + for i := range numShared { + sharedLbls = append(sharedLbls, labels.FromStrings( + labels.MetricName, "test_shared", "job", "test", + "series", strconv.Itoa(i%3), "status_code", statusCodes[i%5])) + } + // parquet-only: only in block A. Carries a Parquet-only label name "pk". + parquetOnlyLbls := make([]labels.Labels, 0, numPOnly) + for i := range numPOnly { + parquetOnlyLbls = append(parquetOnlyLbls, labels.FromStrings( + labels.MetricName, "test_parquet_only", "job", "test", + "series", strconv.Itoa(i%3), "pk", strconv.Itoa(i))) + } + // tsdb-only: only in block B. Carries a TSDB-only label name "tk". + tsdbOnlyLbls := make([]labels.Labels, 0, numTOnly) + for i := range numTOnly { + tsdbOnlyLbls = append(tsdbOnlyLbls, labels.FromStrings( + labels.MetricName, "test_tsdb_only", "job", "test", + "series", strconv.Itoa(i%3), "tk", strconv.Itoa(i))) + } + + // Block A (Parquet) gets shared + parquet-only, block B (TSDB) gets shared + tsdb-only. + lblsA := append(append([]labels.Labels{}, sharedLbls...), parquetOnlyLbls...) + lblsB := append(append([]labels.Labels{}, sharedLbls...), tsdbOnlyLbls...) + lblsAll := append(append(append([]labels.Labels{}, sharedLbls...), parquetOnlyLbls...), tsdbOnlyLbls...) + + // Block A [start, mid): converted to Parquet. Block B [mid, end): stays TSDB. + idA, err := e2e.CreateBlock(ctx, rnd, dir, lblsA, numSamples, start.UnixMilli(), mid.UnixMilli(), scrapeInterval.Milliseconds(), 10) + require.NoError(t, err) + idB, err := e2e.CreateBlock(ctx, rnd, dir, lblsB, numSamples, mid.UnixMilli(), end.UnixMilli(), scrapeInterval.Milliseconds(), 10) + require.NoError(t, err) + + minio := e2edb.NewMinio(9000, flags["-blocks-storage.s3.bucket-name"]) + require.NoError(t, s.StartAndWaitReady(minio)) + + storage, err := e2ecortex.NewS3ClientForMinio(minio, flags["-blocks-storage.s3.bucket-name"]) + require.NoError(t, err) + userBkt := bucket.NewUserBucketClient(userID, storage.GetBucket(), nil) + + // Upload both TSDB blocks to object storage. + require.NoError(t, block.Upload(ctx, log.Logger, userBkt, filepath.Join(dir, idA.String()), metadata.NoneFunc)) + require.NoError(t, block.Upload(ctx, log.Logger, userBkt, filepath.Join(dir, idB.String()), metadata.NoneFunc)) + + // Manually convert block A to Parquet in object storage (block B is left as TSDB). + convertBlockToParquet(t, ctx, userBkt, dir, idA) + + cortex := e2ecortex.NewSingleBinary("cortex", flags, "") + require.NoError(t, s.StartAndWaitReady(cortex)) + + // Wait until the bucket index reflects the hybrid state: block A tagged Parquet, block B TSDB. + waitBlockParquetState(t, ctx, storage.GetBucket(), userID, idA, true) + waitBlockParquetState(t, ctx, storage.GetBucket(), userID, idB, false) + + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Equals(1), []string{"cortex_blocks_meta_synced"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "state", "loaded")))) + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Equals(1), []string{"cortex_blocks_meta_synced"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "state", "parquet-converted")))) + + c1, err := e2ecortex.NewClient("", cortex.HTTPEndpoint(), "", "", userID) + require.NoError(t, err) + + // Prometheus reads both blocks directly from the shared data dir, giving it the same data. + require.NoError(t, writeFileToSharedDir(s, "prometheus.yml", []byte(""))) + prom := e2edb.NewPrometheus("", nil) + require.NoError(t, s.StartAndWaitReady(prom)) + + c2, err := e2ecortex.NewPromQueryClient(prom.HTTPEndpoint()) + require.NoError(t, err) + waitUntilReady(t, ctx, c1, c2, `{job="test"}`, start, end) + + opts := []promqlsmith.Option{ + promqlsmith.WithEnabledFunctions(enabledFunctions), + promqlsmith.WithEnabledAggrs(enabledAggrs), + } + ps := promqlsmith.New(rnd, lblsAll, opts...) + + runQueryFuzzTestCases(t, ps, c1, c2, end, start, end, scrapeInterval, 500, true) + + // Confirm the hybrid path actually routed blocks to both sub-stores. + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Greater(0), []string{"cortex_hybrid_bucket_stores_blocks_routed_total"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "store", "parquet")))) + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Greater(0), []string{"cortex_hybrid_bucket_stores_blocks_routed_total"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "store", "tsdb")))) + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Greater(0), []string{"cortex_hybrid_bucket_stores_operations_total"}, + e2e.WithLabelMatchers( + labels.MustNewMatcher(labels.MatchEqual, "store", "mixed"), + labels.MustNewMatcher(labels.MatchEqual, "method", "Series")))) +} + +// TestParquetStoreGateway_HybridModeConversionHandover verifies the TSDB -> Parquet handover at +// runtime: a block is first served by the TSDB sub-store, then converted to Parquet while the +// store-gateway is running. +func TestParquetStoreGateway_HybridModeConversionHandover(t *testing.T) { + s, err := e2e.NewScenario(networkName) + require.NoError(t, err) + defer s.Close() + + consul := e2edb.NewConsulWithName("consul") + require.NoError(t, s.StartAndWaitReady(consul)) + + flags := hybridStoreGatewayFlags(consul.NetworkHTTPEndpoint()) + require.NoError(t, writeFileToSharedDir(s, "alertmanager_configs", []byte{})) + + const ( + userID = "user-1" + metricName = "series_handover" + numSeries = 3 + numSamples = 60 + ) + + ctx := context.Background() + rnd := newFuzzRand(t) + dir := filepath.Join(s.SharedDir(), "data") + scrapeInterval := time.Minute + now := time.Now() + // The whole range must be older than -limits.query-ingesters-within (2h). + start := now.Add(-time.Hour * 24) + end := now.Add(-time.Hour * 3) + + lbls := make([]labels.Labels, 0, numSeries) + for i := range numSeries { + lbls = append(lbls, labels.FromStrings(labels.MetricName, metricName, "job", "test", "series", strconv.Itoa(i))) + } + id, err := e2e.CreateBlock(ctx, rnd, dir, lbls, numSamples, start.UnixMilli(), end.UnixMilli(), scrapeInterval.Milliseconds(), 10) + require.NoError(t, err) + + minio := e2edb.NewMinio(9000, flags["-blocks-storage.s3.bucket-name"]) + require.NoError(t, s.StartAndWaitReady(minio)) + + storage, err := e2ecortex.NewS3ClientForMinio(minio, flags["-blocks-storage.s3.bucket-name"]) + require.NoError(t, err) + userBkt := bucket.NewUserBucketClient(userID, storage.GetBucket(), nil) + require.NoError(t, block.Upload(ctx, log.Logger, userBkt, filepath.Join(dir, id.String()), metadata.NoneFunc)) + + cortex := e2ecortex.NewSingleBinary("cortex", flags, "") + require.NoError(t, s.StartAndWaitReady(cortex)) + + c, err := e2ecortex.NewClient("", cortex.HTTPEndpoint(), "", "", userID) + require.NoError(t, err) + + // Phase 1: the block is not converted yet, so the TSDB sub-store loads and serves it. + waitBlockParquetState(t, ctx, storage.GetBucket(), userID, id, false) + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Equals(1), []string{"cortex_bucket_store_blocks_loaded"})) + + var before model.Matrix + cortex_testutil.Poll(t, 60*time.Second, numSeries, func() any { + res, err := c.QueryRange(metricName, start, end, scrapeInterval) + if err != nil { + return 0 + } + before = res.(model.Matrix) + return len(before) + }) + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Greater(0), []string{"cortex_hybrid_bucket_stores_blocks_routed_total"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "store", "tsdb")))) + routedToParquet, err := cortex.SumMetrics([]string{"cortex_hybrid_bucket_stores_blocks_routed_total"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "store", "parquet")), e2e.SkipMissingMetrics) + require.NoError(t, err) + require.Zero(t, routedToParquet[0], "the block must not be routed to the Parquet store before it is converted") + + // Phase 2: convert the block while the store-gateway is running. + convertBlockToParquet(t, ctx, userBkt, dir, id) + waitBlockParquetState(t, ctx, storage.GetBucket(), userID, id, true) + + // The TSDB sub-store must drop the converted block and release its index-header. + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Equals(1), []string{"cortex_blocks_meta_synced"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "state", "parquet-converted")))) + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Equals(0), []string{"cortex_blocks_meta_synced"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "state", "loaded")))) + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Equals(0), []string{"cortex_bucket_store_blocks_loaded"})) + + // The same query must now be served by the Parquet store with identical results. + res, err := c.QueryRange(metricName, start, end, scrapeInterval) + require.NoError(t, err) + after, ok := res.(model.Matrix) + require.True(t, ok) + require.Equal(t, before, after, "query results must not change across the TSDB -> Parquet handover") + + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Greater(0), []string{"cortex_hybrid_bucket_stores_blocks_routed_total"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "store", "parquet")))) +} diff --git a/pkg/storage/tsdb/config.go b/pkg/storage/tsdb/config.go index db51ebc5765..ebf819af3e0 100644 --- a/pkg/storage/tsdb/config.go +++ b/pkg/storage/tsdb/config.go @@ -63,6 +63,7 @@ var ( ErrInvalidTokenBucketBytesLimiterMode = errors.New("invalid token bucket bytes limiter mode") ErrInvalidLazyExpandedPostingGroupMaxKeySeriesRatio = errors.New("lazy expanded posting group max key series ratio needs to be equal or greater than 0") ErrInvalidBucketStoreType = errors.New("invalid bucket store type") + ErrParquetFallbackRequiresBucketIndex = errors.New("the parquet bucket store requires the bucket index to be enabled unless the parquet fallback is disabled") ErrInvalidMaxConcurrentDataBytes = errors.New("max concurrent data bytes must be non-negative") ) @@ -296,6 +297,7 @@ type BucketStoreConfig struct { BucketIndex BucketIndexConfig `yaml:"bucket_index"` BlockDiscoveryStrategy string `yaml:"block_discovery_strategy"` BucketStoreType string `yaml:"bucket_store_type"` + ParquetFallbackDisabled bool `yaml:"parquet_fallback_disabled"` // Chunk pool. MaxChunkPoolBytes uint64 `yaml:"max_chunk_pool_bytes"` @@ -391,7 +393,8 @@ func (cfg *BucketStoreConfig) RegisterFlags(f *flag.FlagSet) { f.Float64Var(&cfg.LazyExpandedPostingGroupMaxKeySeriesRatio, "blocks-storage.bucket-store.lazy-expanded-posting-group-max-key-series-ratio", 100, "Mark posting group as lazy if it fetches more keys than R * max series the query should fetch. With R set to 100, a posting group which fetches 100K keys will be marked as lazy if the current query only fetches 1000 series. This config is only valid if lazy expanded posting is enabled. 0 disables the limit.") f.IntVar(&cfg.SeriesBatchSize, "blocks-storage.bucket-store.series-batch-size", store.SeriesBatchSize, "Controls how many series to fetch per batch in Store Gateway. Default value is 10000.") f.StringVar(&cfg.BlockDiscoveryStrategy, "blocks-storage.bucket-store.block-discovery-strategy", string(ConcurrentDiscovery), "One of "+strings.Join(supportedBlockDiscoveryStrategies, ", ")+". When set to concurrent, stores will concurrently issue one call per directory to discover active blocks in the bucket. The recursive strategy iterates through all objects in the bucket, recursively traversing into each directory. This avoids N+1 calls at the expense of having slower bucket iterations. bucket_index strategy can be used in Compactor only and utilizes the existing bucket index to fetch block IDs to sync. This avoids iterating the bucket but can be impacted by delays of cleaner creating bucket index.") - f.StringVar(&cfg.BucketStoreType, "blocks-storage.bucket-store.bucket-store-type", "tsdb", "Type of bucket store to use (tsdb or parquet).") + f.StringVar(&cfg.BucketStoreType, "blocks-storage.bucket-store.bucket-store-type", "tsdb", "Type of bucket store to use (tsdb or parquet). With parquet, blocks converted to Parquet are served from Parquet files and blocks not yet converted are served from TSDB blocks, unless -blocks-storage.bucket-store.parquet-fallback-disabled is set.") + f.BoolVar(&cfg.ParquetFallbackDisabled, "blocks-storage.bucket-store.parquet-fallback-disabled", false, "[Experimental] Only used when the bucket store type is parquet. If true, the store-gateway serves only blocks converted to Parquet and does not fall back to TSDB blocks for blocks not converted yet, so it keeps no local state. If false, the bucket index must be enabled.") f.StringVar(&cfg.TokenBucketBytesLimiter.Mode, "blocks-storage.bucket-store.token-bucket-bytes-limiter.mode", string(TokenBucketBytesLimiterDisabled), fmt.Sprintf("Token bucket bytes limiter mode. Supported values are: %s", strings.Join(supportedTokenBucketBytesLimiterModes, ", "))) f.Int64Var(&cfg.TokenBucketBytesLimiter.InstanceTokenBucketSize, "blocks-storage.bucket-store.token-bucket-bytes-limiter.instance-token-bucket-size", int64(820*units.Mebibyte), "Instance token bucket size") f.Int64Var(&cfg.TokenBucketBytesLimiter.UserTokenBucketSize, "blocks-storage.bucket-store.token-bucket-bytes-limiter.user-token-bucket-size", int64(615*units.Mebibyte), "User token bucket size") @@ -438,6 +441,10 @@ func (cfg *BucketStoreConfig) Validate() error { if !slices.Contains(supportedBucketStoreTypes, cfg.BucketStoreType) { return ErrInvalidBucketStoreType } + // The TSDB fallback relies on the bucket index to tell which blocks are already served by Parquet. + if cfg.BucketStoreType == string(ParquetBucketStore) && !cfg.ParquetFallbackDisabled && !cfg.BucketIndex.Enabled { + return ErrParquetFallbackRequiresBucketIndex + } if cfg.LazyExpandedPostingGroupMaxKeySeriesRatio < 0 { return ErrInvalidLazyExpandedPostingGroupMaxKeySeriesRatio } diff --git a/pkg/storage/tsdb/config_test.go b/pkg/storage/tsdb/config_test.go index 620db51bba1..d7c5c8fd848 100644 --- a/pkg/storage/tsdb/config_test.go +++ b/pkg/storage/tsdb/config_test.go @@ -163,6 +163,28 @@ func TestConfig_Validate(t *testing.T) { }, expectedErr: nil, }, + "should fail on parquet bucket store with fallback enabled and bucket index disabled": { + setup: func(cfg *BlocksStorageConfig) { + cfg.BucketStore.BucketStoreType = string(ParquetBucketStore) + cfg.BucketStore.BucketIndex.Enabled = false + }, + expectedErr: ErrParquetFallbackRequiresBucketIndex, + }, + "should pass on parquet bucket store with fallback enabled and bucket index enabled": { + setup: func(cfg *BlocksStorageConfig) { + cfg.BucketStore.BucketStoreType = string(ParquetBucketStore) + cfg.BucketStore.BucketIndex.Enabled = true + }, + expectedErr: nil, + }, + "should pass on parquet bucket store with fallback disabled and bucket index disabled": { + setup: func(cfg *BlocksStorageConfig) { + cfg.BucketStore.BucketStoreType = string(ParquetBucketStore) + cfg.BucketStore.ParquetFallbackDisabled = true + cfg.BucketStore.BucketIndex.Enabled = false + }, + expectedErr: nil, + }, "should fail on negative max concurrent data bytes": { setup: func(cfg *BlocksStorageConfig) { cfg.BucketStore.MaxConcurrentDataBytes = -1 diff --git a/pkg/storegateway/bucket_index_metadata_fetcher.go b/pkg/storegateway/bucket_index_metadata_fetcher.go index a66e8072482..75a1a4d520b 100644 --- a/pkg/storegateway/bucket_index_metadata_fetcher.go +++ b/pkg/storegateway/bucket_index_metadata_fetcher.go @@ -21,6 +21,7 @@ const ( corruptedBucketIndex = "corrupted-bucket-index" keyAccessDenied = "key-access-denied" noBucketIndex = "no-bucket-index" + parquetConvertedMeta = "parquet-converted" ) // BucketIndexMetadataFetcher is a Thanos MetadataFetcher implementation leveraging on the Cortex bucket index. @@ -50,7 +51,7 @@ func NewBucketIndexMetadataFetcher( cfgProvider: cfgProvider, logger: logger, filters: filters, - metrics: block.NewFetcherMetrics(reg, [][]string{{corruptedBucketIndex}, {noBucketIndex}}, nil), + metrics: block.NewFetcherMetrics(reg, [][]string{{corruptedBucketIndex}, {noBucketIndex}, {parquetConvertedMeta}}, nil), } } diff --git a/pkg/storegateway/bucket_index_metadata_fetcher_test.go b/pkg/storegateway/bucket_index_metadata_fetcher_test.go index 859570f8ce4..db1a0b704e5 100644 --- a/pkg/storegateway/bucket_index_metadata_fetcher_test.go +++ b/pkg/storegateway/bucket_index_metadata_fetcher_test.go @@ -86,6 +86,7 @@ func TestBucketIndexMetadataFetcher_Fetch(t *testing.T) { blocks_meta_synced{state="marked-for-no-compact"} 0 blocks_meta_synced{state="no-bucket-index"} 0 blocks_meta_synced{state="no-meta-json"} 0 + blocks_meta_synced{state="parquet-converted"} 0 blocks_meta_synced{state="parquet-migrated"} 0 blocks_meta_synced{state="time-excluded"} 0 blocks_meta_synced{state="too-fresh"} 0 @@ -135,6 +136,7 @@ func TestBucketIndexMetadataFetcher_Fetch_KeyPermissionDenied(t *testing.T) { blocks_meta_synced{state="marked-for-no-compact"} 0 blocks_meta_synced{state="no-bucket-index"} 0 blocks_meta_synced{state="no-meta-json"} 0 + blocks_meta_synced{state="parquet-converted"} 0 blocks_meta_synced{state="parquet-migrated"} 0 blocks_meta_synced{state="time-excluded"} 0 blocks_meta_synced{state="too-fresh"} 0 @@ -187,6 +189,7 @@ func TestBucketIndexMetadataFetcher_Fetch_NoBucketIndex(t *testing.T) { blocks_meta_synced{state="marked-for-no-compact"} 0 blocks_meta_synced{state="no-bucket-index"} 1 blocks_meta_synced{state="no-meta-json"} 0 + blocks_meta_synced{state="parquet-converted"} 0 blocks_meta_synced{state="parquet-migrated"} 0 blocks_meta_synced{state="time-excluded"} 0 blocks_meta_synced{state="too-fresh"} 0 @@ -243,6 +246,7 @@ func TestBucketIndexMetadataFetcher_Fetch_CorruptedBucketIndex(t *testing.T) { blocks_meta_synced{state="marked-for-no-compact"} 0 blocks_meta_synced{state="no-bucket-index"} 0 blocks_meta_synced{state="no-meta-json"} 0 + blocks_meta_synced{state="parquet-converted"} 0 blocks_meta_synced{state="parquet-migrated"} 0 blocks_meta_synced{state="time-excluded"} 0 blocks_meta_synced{state="too-fresh"} 0 @@ -291,6 +295,7 @@ func TestBucketIndexMetadataFetcher_Fetch_ShouldResetGaugeMetrics(t *testing.T) blocks_meta_synced{state="marked-for-no-compact"} 0 blocks_meta_synced{state="no-bucket-index"} 0 blocks_meta_synced{state="no-meta-json"} 0 + blocks_meta_synced{state="parquet-converted"} 0 blocks_meta_synced{state="parquet-migrated"} 0 blocks_meta_synced{state="time-excluded"} 0 blocks_meta_synced{state="too-fresh"} 0 @@ -316,6 +321,7 @@ func TestBucketIndexMetadataFetcher_Fetch_ShouldResetGaugeMetrics(t *testing.T) blocks_meta_synced{state="marked-for-no-compact"} 0 blocks_meta_synced{state="no-bucket-index"} 1 blocks_meta_synced{state="no-meta-json"} 0 + blocks_meta_synced{state="parquet-converted"} 0 blocks_meta_synced{state="parquet-migrated"} 0 blocks_meta_synced{state="time-excluded"} 0 blocks_meta_synced{state="too-fresh"} 0 @@ -349,6 +355,7 @@ func TestBucketIndexMetadataFetcher_Fetch_ShouldResetGaugeMetrics(t *testing.T) blocks_meta_synced{state="marked-for-no-compact"} 0 blocks_meta_synced{state="no-bucket-index"} 0 blocks_meta_synced{state="no-meta-json"} 0 + blocks_meta_synced{state="parquet-converted"} 0 blocks_meta_synced{state="parquet-migrated"} 0 blocks_meta_synced{state="time-excluded"} 0 blocks_meta_synced{state="too-fresh"} 0 @@ -376,6 +383,7 @@ func TestBucketIndexMetadataFetcher_Fetch_ShouldResetGaugeMetrics(t *testing.T) blocks_meta_synced{state="marked-for-no-compact"} 0 blocks_meta_synced{state="no-bucket-index"} 0 blocks_meta_synced{state="no-meta-json"} 0 + blocks_meta_synced{state="parquet-converted"} 0 blocks_meta_synced{state="parquet-migrated"} 0 blocks_meta_synced{state="time-excluded"} 0 blocks_meta_synced{state="too-fresh"} 0 diff --git a/pkg/storegateway/bucket_store_inmemory_server.go b/pkg/storegateway/bucket_store_inmemory_server.go index d3902d5b234..dcc5fca516d 100644 --- a/pkg/storegateway/bucket_store_inmemory_server.go +++ b/pkg/storegateway/bucket_store_inmemory_server.go @@ -41,26 +41,53 @@ func (s *bucketStoreSeriesServer) Send(r *storepb.SeriesResponse) error { } if recvSeries := r.GetSeries(); recvSeries != nil { - // Thanos uses a pool for the chunks and may use other pools in the future. - // Given we need to retain the reference after the pooled slices are recycled, - // we need to do a copy here. We prefer to stay on the safest side at this stage - // so we do a marshal+unmarshal to copy the whole series. - recvSeriesData, err := recvSeries.Marshal() - if err != nil { - return errors.Wrap(err, "marshal received series") + if err := s.appendSeries(recvSeries); err != nil { + return err } + } - copiedSeries := &storepb.Series{} - if err = copiedSeries.Unmarshal(recvSeriesData); err != nil { - return errors.Wrap(err, "unmarshal received series") + // When the request's ResponseBatchSize is >= 2, series are delivered as a Batch response + // instead of individual Series responses. + if recvBatch := r.GetBatch(); recvBatch != nil { + for _, recvSeries := range recvBatch.Series { + if err := s.appendSeries(recvSeries); err != nil { + return err + } } + } + + return nil +} - s.SeriesSet = append(s.SeriesSet, copiedSeries) +func (s *bucketStoreSeriesServer) appendSeries(recvSeries *storepb.Series) error { + copiedSeries, err := copySeries(recvSeries) + if err != nil { + return err } + s.SeriesSet = append(s.SeriesSet, copiedSeries) return nil } +// copySeries returns a deep copy of series. +// +// Thanos uses a pool for the chunks and may use other pools in the future. +// Given we need to retain the reference after the pooled slices are recycled, +// we need to do a copy here. We prefer to stay on the safest side at this stage +// so we do a marshal+unmarshal to copy the whole series. +func copySeries(series *storepb.Series) (*storepb.Series, error) { + data, err := series.Marshal() + if err != nil { + return nil, errors.Wrap(err, "marshal received series") + } + + copied := &storepb.Series{} + if err = copied.Unmarshal(data); err != nil { + return nil, errors.Wrap(err, "unmarshal received series") + } + return copied, nil +} + func (s *bucketStoreSeriesServer) Context() context.Context { return s.ctx } diff --git a/pkg/storegateway/bucket_store_streaming_server.go b/pkg/storegateway/bucket_store_streaming_server.go new file mode 100644 index 00000000000..77cad48891f --- /dev/null +++ b/pkg/storegateway/bucket_store_streaming_server.go @@ -0,0 +1,126 @@ +package storegateway + +import ( + "context" + "sync" + + "github.com/gogo/protobuf/types" + "github.com/pkg/errors" + "github.com/prometheus/prometheus/model/labels" + "github.com/prometheus/prometheus/util/annotations" + "github.com/thanos-io/thanos/pkg/store/hintspb" + "github.com/thanos-io/thanos/pkg/store/storepb" +) + +// channelSeriesServer adapts a push-based storepb.Store_SeriesServer into a pull-based +// storepb.SeriesSet backed by a bounded channel. A producer goroutine runs a sub-store's +// Series() with this server, and Send pushes each received series onto the channel. The +// consumer pulls them via Next/At/Err so storepb.MergeSeriesSets can stream-merge multiple +// stores without buffering full results. +type channelSeriesServer struct { + storepb.Store_SeriesServer + + ctx context.Context + ch chan *storepb.Series + + // copyOnSend copies each received series before retaining it. + copyOnSend bool + + warnings annotations.Annotations + hints hintspb.SeriesResponseHints + + mu sync.Mutex + err error + + // cur holds the series returned by the most recent successful Next call. + cur *storepb.Series +} + +func newChannelSeriesServer(ctx context.Context, bufferSize int, copyOnSend bool) *channelSeriesServer { + if bufferSize < 1 { + bufferSize = 1 + } + return &channelSeriesServer{ + ctx: ctx, + ch: make(chan *storepb.Series, bufferSize), + copyOnSend: copyOnSend, + } +} + +// Context implements storepb.Store_SeriesServer. +func (s *channelSeriesServer) Context() context.Context { return s.ctx } + +// Send implements storepb.Store_SeriesServer. It handles single Series, batched Series, warning +// and hints responses. +func (s *channelSeriesServer) Send(r *storepb.SeriesResponse) error { + if w := r.GetWarning(); w != "" { + s.warnings.Add(errors.New(w)) + } + + if rawHints := r.GetHints(); rawHints != nil { + if err := types.UnmarshalAny(rawHints, &s.hints); err != nil { + return errors.Wrap(err, "failed to unmarshal series hints") + } + } + + if series := r.GetSeries(); series != nil { + return s.push(series) + } + + if batch := r.GetBatch(); batch != nil { + for _, series := range batch.Series { + if err := s.push(series); err != nil { + return err + } + } + } + + return nil +} + +func (s *channelSeriesServer) push(series *storepb.Series) error { + if s.copyOnSend { + var err error + if series, err = copySeries(series); err != nil { + return err + } + } + + select { + case s.ch <- series: + return nil + case <-s.ctx.Done(): + return s.ctx.Err() + } +} + +// Close records the producer's terminal error (if any) and closes the channel. It must be called +// exactly once by the producer goroutine after Series() returns. +func (s *channelSeriesServer) Close(err error) { + s.mu.Lock() + s.err = err + s.mu.Unlock() + close(s.ch) +} + +// Next implements storepb.SeriesSet. +func (s *channelSeriesServer) Next() bool { + series, ok := <-s.ch + if !ok { + return false + } + s.cur = series + return true +} + +// At implements storepb.SeriesSet. +func (s *channelSeriesServer) At() (labels.Labels, []storepb.AggrChunk) { + return s.cur.PromLabels(), s.cur.Chunks +} + +// Err implements storepb.SeriesSet. It is safe to call concurrently while the producer is running. +func (s *channelSeriesServer) Err() error { + s.mu.Lock() + defer s.mu.Unlock() + return s.err +} diff --git a/pkg/storegateway/bucket_store_streaming_server_test.go b/pkg/storegateway/bucket_store_streaming_server_test.go new file mode 100644 index 00000000000..6898416b8b0 --- /dev/null +++ b/pkg/storegateway/bucket_store_streaming_server_test.go @@ -0,0 +1,96 @@ +package storegateway + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/gogo/protobuf/types" + "github.com/prometheus/prometheus/model/labels" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/thanos-io/thanos/pkg/store/hintspb" + "github.com/thanos-io/thanos/pkg/store/labelpb" + "github.com/thanos-io/thanos/pkg/store/storepb" +) + +func newTestSeries(value string) *storepb.Series { + return &storepb.Series{Labels: labelpb.ZLabelsFromPromLabels(labels.FromStrings("series", value))} +} + +func TestChannelSeriesServer_CollectsSeriesWarningsAndHints(t *testing.T) { + s := newChannelSeriesServer(context.Background(), 10, true) + + anyHints, err := types.MarshalAny(&hintspb.SeriesResponseHints{ + QueriedBlocks: []hintspb.Block{{Id: "block-1"}}, + QueryStats: &hintspb.QueryStats{BlocksQueried: 1}, + }) + require.NoError(t, err) + + require.NoError(t, s.Send(storepb.NewSeriesResponse(newTestSeries("a")))) + require.NoError(t, s.Send(storepb.NewBatchResponse([]*storepb.Series{newTestSeries("b"), newTestSeries("c")}))) + require.NoError(t, s.Send(storepb.NewWarnSeriesResponse(errors.New("warning-1")))) + require.NoError(t, s.Send(storepb.NewHintsSeriesResponse(anyHints))) + s.Close(nil) + + var got []string + for s.Next() { + lset, _ := s.At() + got = append(got, lset.Get("series")) + } + require.NoError(t, s.Err()) + assert.Equal(t, []string{"a", "b", "c"}, got) + + assert.Len(t, s.warnings, 1) + assert.Contains(t, s.warnings, "warning-1") + assert.Equal(t, []hintspb.Block{{Id: "block-1"}}, s.hints.QueriedBlocks) + require.NotNil(t, s.hints.QueryStats) + assert.Equal(t, int64(1), s.hints.QueryStats.BlocksQueried) +} + +func TestChannelSeriesServer_CloseWithError(t *testing.T) { + s := newChannelSeriesServer(context.Background(), 10, true) + require.NoError(t, s.Send(storepb.NewSeriesResponse(newTestSeries("a")))) + + producerErr := errors.New("producer failed") + s.Close(producerErr) + + // Series pushed before the failure are still drained, then the error is exposed. + require.True(t, s.Next()) + require.False(t, s.Next()) + assert.ErrorIs(t, s.Err(), producerErr) +} + +func TestChannelSeriesServer_SendUnblocksOnContextCancel(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + s := newChannelSeriesServer(ctx, 1, true) + + // The first series fills the buffer; with no consumer the next one would block forever. + require.NoError(t, s.Send(storepb.NewSeriesResponse(newTestSeries("a")))) + + cancel() + err := s.Send(storepb.NewSeriesResponse(newTestSeries("b"))) + assert.ErrorIs(t, err, context.Canceled) +} + +func TestChannelSeriesServer_CopyOnSend(t *testing.T) { + for _, copyOnSend := range []bool{true, false} { + t.Run(fmt.Sprintf("copyOnSend=%t", copyOnSend), func(t *testing.T) { + s := newChannelSeriesServer(context.Background(), 1, copyOnSend) + sent := newTestSeries("a") + require.NoError(t, s.Send(storepb.NewSeriesResponse(sent))) + s.Close(nil) + + require.True(t, s.Next()) + lset, _ := s.At() + assert.Equal(t, "a", lset.Get("series")) + if copyOnSend { + // A copied series must not alias the sender's memory, which may be recycled. + assert.NotSame(t, sent, s.cur) + } else { + assert.Same(t, sent, s.cur) + } + }) + } +} diff --git a/pkg/storegateway/bucket_stores.go b/pkg/storegateway/bucket_stores.go index a5ef68a969e..ddced5c7f17 100644 --- a/pkg/storegateway/bucket_stores.go +++ b/pkg/storegateway/bucket_stores.go @@ -63,6 +63,10 @@ type ThanosBucketStores struct { metaFetcherMetrics *MetadataFetcherMetrics shardingStrategy ShardingStrategy + // ignoreParquetBlocks when true, excludes Parquet-converted blocks from + // syncing so they are not loaded by this TSDB store. + ignoreParquetBlocks bool + // Index cache shared across all tenants. indexCache storecache.IndexCache @@ -86,6 +90,11 @@ type ThanosBucketStores struct { storesErrorsMu sync.RWMutex storesErrors map[string]error + // Per-tenant Parquet filter (set only when ignoreParquetBlocks). Exposes the + // Parquet-converted block IDs, used for hybrid routing. + parquetFiltersMu sync.RWMutex + parquetFilters map[string]*IgnoreParquetBlocksFilter + instanceTokenBucket *util.TokenBucket userTokenBucketsMu sync.RWMutex @@ -116,20 +125,40 @@ var ErrTooManyInflightRequests = status.Error(codes.ResourceExhausted, "too many func NewBucketStores(cfg tsdb.BlocksStorageConfig, shardingStrategy ShardingStrategy, bucketClient objstore.InstrumentedBucket, limits *validation.Overrides, logLevel logging.Level, logger log.Logger, reg prometheus.Registerer) (BucketStores, error) { switch cfg.BucketStore.BucketStoreType { case string(tsdb.ParquetBucketStore): - return newParquetBucketStores(cfg, bucketClient, limits, logger, reg) + if cfg.BucketStore.ParquetFallbackDisabled { + return newParquetBucketStores(cfg, bucketClient, nil, nil, limits, logger, reg) + } + return newHybridBucketStores(cfg, shardingStrategy, bucketClient, limits, logLevel, logger, reg) case string(tsdb.TSDBBucketStore): - return newThanosBucketStores(cfg, shardingStrategy, bucketClient, limits, logLevel, logger, reg) + return newThanosBucketStores(cfg, shardingStrategy, bucketClient, nil, nil, false, limits, logLevel, logger, reg) default: return nil, fmt.Errorf("unsupported bucket store type: %s", cfg.BucketStore.BucketStoreType) } } +func newMatchersCache(cfg tsdb.BlocksStorageConfig, logger log.Logger, reg prometheus.Registerer) (storecache.MatchersCache, error) { + if cfg.BucketStore.MatchersCacheMaxItems <= 0 { + return storecache.NoopMatchersCache, nil + } + r := prometheus.NewRegistry() + reg.MustRegister(tsdb.NewMatchCacheMetrics("cortex_storegateway", r, logger)) + return storecache.NewMatchersCache(storecache.WithSize(cfg.BucketStore.MatchersCacheMaxItems), storecache.WithPromRegistry(r)) +} + // newThanosBucketStores creates a new TSDB-based bucket stores -func newThanosBucketStores(cfg tsdb.BlocksStorageConfig, shardingStrategy ShardingStrategy, bucketClient objstore.InstrumentedBucket, limits *validation.Overrides, logLevel logging.Level, logger log.Logger, reg prometheus.Registerer) (*ThanosBucketStores, error) { - matchers := tsdb.NewMatchers() - cachingBucket, err := tsdb.CreateCachingBucket(cfg.BucketStore.ChunksCache, cfg.BucketStore.MetadataCache, tsdb.ParquetLabelsCacheConfig{}, matchers, bucketClient, logger, reg) - if err != nil { - return nil, errors.Wrapf(err, "create caching bucket") +func newThanosBucketStores(cfg tsdb.BlocksStorageConfig, shardingStrategy ShardingStrategy, bucketClient objstore.InstrumentedBucket, cachingBucket objstore.InstrumentedBucket, matcherCache storecache.MatchersCache, ignoreParquetBlocks bool, limits *validation.Overrides, logLevel logging.Level, logger log.Logger, reg prometheus.Registerer) (*ThanosBucketStores, error) { + var err error + if cachingBucket == nil { + matchers := tsdb.NewMatchers() + cachingBucket, err = tsdb.CreateCachingBucket(cfg.BucketStore.ChunksCache, cfg.BucketStore.MetadataCache, tsdb.ParquetLabelsCacheConfig{}, matchers, bucketClient, logger, reg) + if err != nil { + return nil, errors.Wrapf(err, "create caching bucket") + } + } + if matcherCache == nil { + if matcherCache, err = newMatchersCache(cfg, logger, reg); err != nil { + return nil, err + } } // The number of concurrent queries against the tenants BucketStores are limited. @@ -146,8 +175,10 @@ func newThanosBucketStores(cfg tsdb.BlocksStorageConfig, shardingStrategy Shardi limits: limits, bucket: cachingBucket, shardingStrategy: shardingStrategy, + ignoreParquetBlocks: ignoreParquetBlocks, stores: map[string]*store.BucketStore{}, storesErrors: map[string]error{}, + parquetFilters: map[string]*IgnoreParquetBlocksFilter{}, logLevel: logLevel, bucketStoreMetrics: NewBucketStoreMetrics(), metaFetcherMetrics: NewMetadataFetcherMetrics(), @@ -183,16 +214,7 @@ func newThanosBucketStores(cfg tsdb.BlocksStorageConfig, shardingStrategy Shardi return nil, errors.Wrap(err, "failed to create users scanner") } - u.matcherCache = storecache.NoopMatchersCache - - if cfg.BucketStore.MatchersCacheMaxItems > 0 { - r := prometheus.NewRegistry() - reg.MustRegister(tsdb.NewMatchCacheMetrics("cortex_storegateway", r, logger)) - u.matcherCache, err = storecache.NewMatchersCache(storecache.WithSize(cfg.BucketStore.MatchersCacheMaxItems), storecache.WithPromRegistry(r)) - if err != nil { - return nil, err - } - } + u.matcherCache = matcherCache // Init the index cache. if u.indexCache, err = tsdb.NewIndexCache(cfg.BucketStore.IndexCache, logger, reg); err != nil { @@ -511,6 +533,18 @@ func (u *ThanosBucketStores) getStoreError(userID string) error { return u.storesErrors[userID] } +// parquetBlockIDs returns the user's Parquet-converted block IDs recorded on the last +// sync, or nil when the user has not been synced with the bucket index yet. +func (u *ThanosBucketStores) parquetBlockIDs(userID string) map[string]struct{} { + u.parquetFiltersMu.RLock() + f := u.parquetFilters[userID] + u.parquetFiltersMu.RUnlock() + if f == nil { + return nil + } + return f.ParquetBlockIDs() +} + var ( errBucketStoreNotEmpty = errors.New("bucket store not empty") errBucketStoreNotFound = errors.New("bucket store not found") @@ -549,6 +583,10 @@ func (u *ThanosBucketStores) closeEmptyBucketStore(userID string) error { u.userTokenBucketsMu.Unlock() } + u.parquetFiltersMu.Lock() + delete(u.parquetFilters, userID) + u.parquetFiltersMu.Unlock() + u.metaFetcherMetrics.RemoveUserRegistry(userID) u.bucketStoreMetrics.RemoveUserRegistry(userID) return bs.Close() @@ -620,6 +658,12 @@ func (u *ThanosBucketStores) getOrCreateStore(userID string) (*store.BucketStore filters = append(filters, NewIgnoreNonQueryableBlocksFilter(userLogger, u.cfg.BucketStore.IgnoreBlocksWithin)) } + var parquetFilter *IgnoreParquetBlocksFilter + if u.ignoreParquetBlocks { + parquetFilter = NewIgnoreParquetBlocksFilter(userLogger) + filters = append(filters, parquetFilter) + } + // Instantiate a different blocks metadata fetcher based on whether bucket index is enabled or not. var fetcher block.MetadataFetcher if u.cfg.BucketStore.BucketIndex.Enabled { @@ -734,6 +778,12 @@ func (u *ThanosBucketStores) getOrCreateStore(userID string) (*store.BucketStore } u.stores[userID] = bs + // Register the Parquet filter only after the store is successfully created. + if parquetFilter != nil { + u.parquetFiltersMu.Lock() + u.parquetFilters[userID] = parquetFilter + u.parquetFiltersMu.Unlock() + } u.metaFetcherMetrics.AddUserRegistry(userID, fetcherReg) u.bucketStoreMetrics.AddUserRegistry(userID, bucketStoreReg) diff --git a/pkg/storegateway/bucket_stores_test.go b/pkg/storegateway/bucket_stores_test.go index 8f5e92e1beb..39971d78c7b 100644 --- a/pkg/storegateway/bucket_stores_test.go +++ b/pkg/storegateway/bucket_stores_test.go @@ -730,6 +730,10 @@ func generateStorageBlock(t *testing.T, storageDir, userID string, metricName st } func querySeries(stores BucketStores, userID, metricName string, minT, maxT int64, blockIDs ...string) ([]*storepb.Series, annotations.Annotations, error) { + return querySeriesWithBatchSize(stores, userID, metricName, minT, maxT, 0, blockIDs...) +} + +func querySeriesWithBatchSize(stores BucketStores, userID, metricName string, minT, maxT, batchSize int64, blockIDs ...string) ([]*storepb.Series, annotations.Annotations, error) { var ( anyHints *types.Any err error @@ -760,6 +764,7 @@ func querySeries(stores BucketStores, userID, metricName string, minT, maxT int6 }}, PartialResponseStrategy: storepb.PartialResponseStrategy_ABORT, Hints: anyHints, + ResponseBatchSize: batchSize, } ctx := setUserIDToGRPCContext(context.Background(), userID) diff --git a/pkg/storegateway/hybrid_bucket_stores.go b/pkg/storegateway/hybrid_bucket_stores.go new file mode 100644 index 00000000000..ca09d1cbc51 --- /dev/null +++ b/pkg/storegateway/hybrid_bucket_stores.go @@ -0,0 +1,541 @@ +package storegateway + +import ( + "context" + "fmt" + "strings" + + "github.com/go-kit/log" + "github.com/gogo/protobuf/proto" + "github.com/gogo/protobuf/types" + "github.com/pkg/errors" + parquet_util "github.com/prometheus-community/parquet-common/util" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/thanos-io/objstore" + "github.com/thanos-io/thanos/pkg/block" + "github.com/thanos-io/thanos/pkg/store/hintspb" + "github.com/thanos-io/thanos/pkg/store/labelpb" + "github.com/thanos-io/thanos/pkg/store/storepb" + "github.com/weaveworks/common/logging" + "github.com/weaveworks/common/user" + "golang.org/x/sync/errgroup" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/cortexproject/cortex/pkg/storage/tsdb" + cortex_util "github.com/cortexproject/cortex/pkg/util" + "github.com/cortexproject/cortex/pkg/util/multierror" + "github.com/cortexproject/cortex/pkg/util/spanlogger" + "github.com/cortexproject/cortex/pkg/util/validation" +) + +type HybridBucketStores struct { + cfg tsdb.BlocksStorageConfig + + inflightRequests *cortex_util.InflightRequestTracker + + parquet *ParquetBucketStores + tsdb *ThanosBucketStores + + metrics *hybridBucketStoresMetrics +} + +type hybridBucketStoresMetrics struct { + blocksRoutedTotal *prometheus.CounterVec + operationsTotal *prometheus.CounterVec +} + +func newHybridBucketStoresMetrics(reg prometheus.Registerer) *hybridBucketStoresMetrics { + return &hybridBucketStoresMetrics{ + blocksRoutedTotal: promauto.With(reg).NewCounterVec(prometheus.CounterOpts{ + Name: "cortex_hybrid_bucket_stores_blocks_routed_total", + Help: "Total number of requested blocks routed to each sub-store.", + }, []string{"store"}), + operationsTotal: promauto.With(reg).NewCounterVec(prometheus.CounterOpts{ + Name: "cortex_hybrid_bucket_stores_operations_total", + Help: "Total number of operations served, labeled by method and which sub-store(s) served them.", + }, []string{"store", "method"}), + } +} + +func newHybridBucketStores(cfg tsdb.BlocksStorageConfig, shardingStrategy ShardingStrategy, bucketClient objstore.InstrumentedBucket, limits *validation.Overrides, logLevel logging.Level, logger log.Logger, reg prometheus.Registerer) (*HybridBucketStores, error) { + cachingBucket, err := createCachingBucketClientForParquet(cfg, bucketClient, "parquet-storegateway", logger, reg) + if err != nil { + return nil, err + } + matcherCache, err := newMatchersCache(cfg, logger, reg) + if err != nil { + return nil, err + } + + // The TSDB store only syncs blocks not yet converted to Parquet. + tsdbStore, err := newThanosBucketStores(cfg, shardingStrategy, bucketClient, cachingBucket, matcherCache, true, limits, logLevel, logger, reg) + if err != nil { + return nil, errors.Wrap(err, "create TSDB store for hybrid bucket stores") + } + + parquetStore, err := newParquetBucketStores(cfg, bucketClient, cachingBucket, matcherCache, limits, logger, reg) + if err != nil { + return nil, errors.Wrap(err, "create parquet store for hybrid bucket stores") + } + + return &HybridBucketStores{ + cfg: cfg, + inflightRequests: cortex_util.NewInflightRequestTracker(), + parquet: parquetStore, + tsdb: tsdbStore, + metrics: newHybridBucketStoresMetrics(reg), + }, nil +} + +// Series implements BucketStores. +func (h *HybridBucketStores) Series(req *storepb.SeriesRequest, srv storepb.Store_SeriesServer) error { + spanLog, spanCtx := spanlogger.New(srv.Context(), "HybridBucketStores.Series") + defer spanLog.Finish() + + userID := getUserIDFromGRPCContext(spanCtx) + if userID == "" { + return fmt.Errorf("no userID") + } + spanCtx = user.InjectOrgID(spanCtx, userID) + + if err := h.checkStoreError(userID); err != nil { + return err + } + + store, err := h.parquet.getOrCreateStore(userID) + if err != nil { + return status.Error(codes.Internal, err.Error()) + } + + if maxInflightRequests := h.cfg.BucketStore.MaxInflightRequests; maxInflightRequests > 0 { + if h.inflightRequests.Count() >= maxInflightRequests { + return ErrTooManyInflightRequests + } + h.inflightRequests.Inc() + defer h.inflightRequests.Dec() + } + + wrappedSrv := spanSeriesServer{ + Store_SeriesServer: srv, + ctx: spanCtx, + } + return h.seriesWithTSDBStore(spanCtx, userID, req, store, wrappedSrv) +} + +// LabelNames implements BucketStores. +func (h *HybridBucketStores) LabelNames(ctx context.Context, req *storepb.LabelNamesRequest) (*storepb.LabelNamesResponse, error) { + spanLog, spanCtx := spanlogger.New(ctx, "HybridBucketStores.LabelNames") + defer spanLog.Finish() + + userID := getUserIDFromGRPCContext(spanCtx) + if userID == "" { + return nil, fmt.Errorf("no userID") + } + spanCtx = user.InjectOrgID(spanCtx, userID) + + if err := h.checkStoreError(userID); err != nil { + return nil, err + } + + store, err := h.parquet.getOrCreateStore(userID) + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + + return h.labelNamesWithTSDBStore(spanCtx, userID, req, store) +} + +// LabelValues implements BucketStores. +func (h *HybridBucketStores) LabelValues(ctx context.Context, req *storepb.LabelValuesRequest) (*storepb.LabelValuesResponse, error) { + spanLog, spanCtx := spanlogger.New(ctx, "HybridBucketStores.LabelValues") + defer spanLog.Finish() + + userID := getUserIDFromGRPCContext(spanCtx) + if userID == "" { + return nil, fmt.Errorf("no userID") + } + spanCtx = user.InjectOrgID(spanCtx, userID) + + if err := h.checkStoreError(userID); err != nil { + return nil, err + } + + store, err := h.parquet.getOrCreateStore(userID) + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + + return h.labelValuesWithTSDBStore(spanCtx, userID, req, store) +} + +func (h *HybridBucketStores) checkStoreError(userID string) error { + if err := h.parquet.getStoreError(userID); err != nil { + return err + } + return h.tsdb.getStoreError(userID) +} + +// SyncBlocks implements BucketStores. +func (h *HybridBucketStores) SyncBlocks(ctx context.Context) error { + return h.tsdb.SyncBlocks(ctx) +} + +// InitialSync implements BucketStores. +func (h *HybridBucketStores) InitialSync(ctx context.Context) error { + if err := h.parquet.InitialSync(ctx); err != nil { + return err + } + return h.tsdb.InitialSync(ctx) +} + +// Stop implements BucketStores +func (h *HybridBucketStores) Stop() error { + return multierror.New(h.parquet.Stop(), h.tsdb.Stop()).Err() +} + +// splitRequestedBlocks splits the block IDs encoded in the request block matchers into +// Parquet-converted and not-yet-converted (TSDB) groups. +func (h *HybridBucketStores) splitRequestedBlocks(userID string, blockMatchers []storepb.LabelMatcher, method string) (parquetIDs, tsdbIDs []string, err error) { + if len(blockMatchers) != 1 || blockMatchers[0].Type != storepb.LabelMatcher_RE || blockMatchers[0].Name != block.BlockIDLabel { + return nil, nil, status.Error(codes.InvalidArgument, "only one block matcher is supported") + } + + // Nil until the tenant's first sync, so every block is routed to the TSDB store in that window, + // which matches how a TSDB-only store-gateway behaves. + parquetBlockIDs := h.tsdb.parquetBlockIDs(userID) + for id := range strings.SplitSeq(blockMatchers[0].Value, "|") { + if id == "" { + continue + } + if _, ok := parquetBlockIDs[id]; ok { + parquetIDs = append(parquetIDs, id) + } else { + tsdbIDs = append(tsdbIDs, id) + } + } + + h.metrics.blocksRoutedTotal.WithLabelValues("parquet").Add(float64(len(parquetIDs))) + h.metrics.blocksRoutedTotal.WithLabelValues("tsdb").Add(float64(len(tsdbIDs))) + switch { + case len(parquetIDs) > 0 && len(tsdbIDs) > 0: + h.metrics.operationsTotal.WithLabelValues("mixed", method).Inc() + case len(parquetIDs) > 0: + h.metrics.operationsTotal.WithLabelValues("parquet", method).Inc() + case len(tsdbIDs) > 0: + h.metrics.operationsTotal.WithLabelValues("tsdb", method).Inc() + } + + return parquetIDs, tsdbIDs, nil +} + +// blockIDsMatcher builds the single regex block matcher understood by the stores. +func blockIDsMatcher(blockIDs []string) []storepb.LabelMatcher { + return []storepb.LabelMatcher{{ + Type: storepb.LabelMatcher_RE, + Name: block.BlockIDLabel, + Value: strings.Join(blockIDs, "|"), + }} +} + +// marshalSubRequestHints marshals the request hints of a sub-request sent to one of the stores. +func marshalSubRequestHints(hints proto.Message) (*types.Any, error) { + anyHints, err := types.MarshalAny(hints) + if err != nil { + return nil, status.Error(codes.Internal, errors.Wrap(err, "marshal sub-request hints").Error()) + } + return anyHints, nil +} + +// seriesRequestForBlocks returns a copy of req whose parsed hints are restricted to blockIDs. +func seriesRequestForBlocks(req *storepb.SeriesRequest, hints *hintspb.SeriesRequestHints, blockIDs []string) (*storepb.SeriesRequest, error) { + h := *hints + h.BlockMatchers = blockIDsMatcher(blockIDs) + anyHints, err := marshalSubRequestHints(&h) + if err != nil { + return nil, err + } + r := *req + r.Hints = anyHints + return &r, nil +} + +// labelNamesRequestForBlocks returns a copy of req whose parsed hints are restricted to blockIDs. +func labelNamesRequestForBlocks(req *storepb.LabelNamesRequest, hints *hintspb.LabelNamesRequestHints, blockIDs []string) (*storepb.LabelNamesRequest, error) { + h := *hints + h.BlockMatchers = blockIDsMatcher(blockIDs) + anyHints, err := marshalSubRequestHints(&h) + if err != nil { + return nil, err + } + r := *req + r.Hints = anyHints + return &r, nil +} + +// labelValuesRequestForBlocks returns a copy of req whose parsed hints are restricted to blockIDs. +func labelValuesRequestForBlocks(req *storepb.LabelValuesRequest, hints *hintspb.LabelValuesRequestHints, blockIDs []string) (*storepb.LabelValuesRequest, error) { + h := *hints + h.BlockMatchers = blockIDsMatcher(blockIDs) + anyHints, err := marshalSubRequestHints(&h) + if err != nil { + return nil, err + } + r := *req + r.Hints = anyHints + return &r, nil +} + +// seriesWithTSDBStore serves a Series request by splitting the requested blocks between the +// Parquet store and the TSDB store, then merging the (sorted) results. +func (h *HybridBucketStores) seriesWithTSDBStore(ctx context.Context, userID string, req *storepb.SeriesRequest, store *parquetBucketStore, srv storepb.Store_SeriesServer) error { + reqHints := &hintspb.SeriesRequestHints{} + if req.Hints != nil { + if err := types.UnmarshalAny(req.Hints, reqHints); err != nil { + return status.Error(codes.InvalidArgument, errors.Wrap(err, "unmarshal series request hints").Error()) + } + } + + parquetIDs, tsdbIDs, err := h.splitRequestedBlocks(userID, reqHints.BlockMatchers, "Series") + if err != nil { + return err + } + + // If only one store has blocks to serve, delegate directly without buffering/merging. + switch { + case len(tsdbIDs) == 0: + return store.Series(req, srv) + case len(parquetIDs) == 0: + return h.tsdb.Series(req, srv) + } + + // Both stores return series sorted by labels. We run them concurrently, each pushing into a + // channel-backed SeriesSet, and stream-merge their outputs so we never buffer either store's + // full result set in memory. + mergeCtx, cancel := context.WithCancel(ctx) + defer cancel() + + g, gCtx := errgroup.WithContext(mergeCtx) + + batchSize := int(req.ResponseBatchSize) + // The Parquet store builds every series from memory it owns and never recycles, so its series + // are retained as is. The TSDB store returns pooled chunk bytes once Series() returns. + parquetSet := newChannelSeriesServer(gCtx, batchSize, false) + tsdbSet := newChannelSeriesServer(gCtx, batchSize, true) + g.Go(func() error { + r, err := seriesRequestForBlocks(req, reqHints, parquetIDs) + if err == nil { + err = store.Series(r, parquetSet) + } + parquetSet.Close(err) + return err + }) + g.Go(func() error { + r, err := seriesRequestForBlocks(req, reqHints, tsdbIDs) + if err == nil { + err = h.tsdb.Series(r, tsdbSet) + } + tsdbSet.Close(err) + return err + }) + + outSrv := newFlushableServer(newBatchableServer(srv, batchSize)) + + var sendErr error + merged := storepb.MergeSeriesSets(parquetSet, tsdbSet) + for merged.Next() { + lset, chks := merged.At() + if err := outSrv.Send(storepb.NewSeriesResponse(&storepb.Series{ + Labels: labelpb.ZLabelsFromPromLabels(lset), + Chunks: chks, + })); err != nil { + sendErr = status.Error(codes.Unknown, errors.Wrap(err, "send merged series response").Error()) + break + } + } + + // Unblock and wait for the producers before inspecting their errors. + cancel() + // Use the errgroup error rather than merged.Err(): when one producer fails, the other one is + // canceled, and merged.Err() may return that cancellation instead of the root cause. + producerErr := g.Wait() + + // A send failure cancels the producers, so their errors are not the cause. + if sendErr != nil { + return sendErr + } + if producerErr != nil { + return producerErr + } + + // Forward accumulated warnings from both stores. + warnings := parquetSet.warnings + warnings.Merge(tsdbSet.warnings) + for _, w := range warnings { + if err := outSrv.Send(storepb.NewWarnSeriesResponse(w)); err != nil { + return status.Error(codes.Unknown, errors.Wrap(err, "send merged series warning").Error()) + } + } + + resHints := hintspb.SeriesResponseHints{ + QueriedBlocks: append(parquetSet.hints.QueriedBlocks, tsdbSet.hints.QueriedBlocks...), + } + if parquetSet.hints.QueryStats != nil || tsdbSet.hints.QueryStats != nil { + stats := &hintspb.QueryStats{} + if s := parquetSet.hints.QueryStats; s != nil { + stats.Merge(s) + } + if s := tsdbSet.hints.QueryStats; s != nil { + stats.Merge(s) + } + resHints.QueryStats = stats + } + anyHints, err := types.MarshalAny(&resHints) + if err != nil { + return status.Error(codes.Unknown, errors.Wrap(err, "marshal series response hints").Error()) + } + if err := outSrv.Send(storepb.NewHintsSeriesResponse(anyHints)); err != nil { + return status.Error(codes.Unknown, errors.Wrap(err, "send series response hints").Error()) + } + + return outSrv.Flush() +} + +// labelNamesWithTSDBStore serves a LabelNames request across the Parquet store and the TSDB store. +func (h *HybridBucketStores) labelNamesWithTSDBStore(ctx context.Context, userID string, req *storepb.LabelNamesRequest, store *parquetBucketStore) (*storepb.LabelNamesResponse, error) { + reqHints := &hintspb.LabelNamesRequestHints{} + if req.Hints != nil { + if err := types.UnmarshalAny(req.Hints, reqHints); err != nil { + return nil, status.Error(codes.InvalidArgument, errors.Wrap(err, "unmarshal label names request hints").Error()) + } + } + + parquetIDs, tsdbIDs, err := h.splitRequestedBlocks(userID, reqHints.BlockMatchers, "LabelNames") + if err != nil { + return nil, err + } + + switch { + case len(tsdbIDs) == 0: + return store.LabelNames(ctx, req) + case len(parquetIDs) == 0: + return h.tsdb.LabelNames(ctx, req) + } + + var parquetResp, tsdbResp *storepb.LabelNamesResponse + g, gCtx := errgroup.WithContext(ctx) + g.Go(func() error { + r, err := labelNamesRequestForBlocks(req, reqHints, parquetIDs) + if err != nil { + return err + } + parquetResp, err = store.LabelNames(gCtx, r) + return err + }) + g.Go(func() error { + r, err := labelNamesRequestForBlocks(req, reqHints, tsdbIDs) + if err != nil { + return err + } + tsdbResp, err = h.tsdb.LabelNames(gCtx, r) + return err + }) + if err := g.Wait(); err != nil { + return nil, err + } + + anyHints, err := mergeLabelNamesHints(parquetResp, tsdbResp) + if err != nil { + return nil, err + } + return &storepb.LabelNamesResponse{ + Names: parquet_util.MergeUnsortedSlices(int(req.Limit), parquetResp.Names, tsdbResp.Names), + Warnings: append(parquetResp.Warnings, tsdbResp.Warnings...), + Hints: anyHints, + }, nil +} + +func mergeLabelNamesHints(a, b *storepb.LabelNamesResponse) (*types.Any, error) { + merged := &hintspb.LabelNamesResponseHints{} + for _, resp := range []*storepb.LabelNamesResponse{a, b} { + if resp == nil || resp.Hints == nil { + continue + } + hints := hintspb.LabelNamesResponseHints{} + if err := types.UnmarshalAny(resp.Hints, &hints); err != nil { + return nil, errors.Wrap(err, "unmarshal label names response hints") + } + merged.QueriedBlocks = append(merged.QueriedBlocks, hints.QueriedBlocks...) + } + return types.MarshalAny(merged) +} + +// labelValuesWithTSDBStore serves a LabelValues request across the Parquet store and the TSDB store. +func (h *HybridBucketStores) labelValuesWithTSDBStore(ctx context.Context, userID string, req *storepb.LabelValuesRequest, store *parquetBucketStore) (*storepb.LabelValuesResponse, error) { + reqHints := &hintspb.LabelValuesRequestHints{} + if req.Hints != nil { + if err := types.UnmarshalAny(req.Hints, reqHints); err != nil { + return nil, status.Error(codes.InvalidArgument, errors.Wrap(err, "unmarshal label values request hints").Error()) + } + } + + parquetIDs, tsdbIDs, err := h.splitRequestedBlocks(userID, reqHints.BlockMatchers, "LabelValues") + if err != nil { + return nil, err + } + + switch { + case len(tsdbIDs) == 0: + return store.LabelValues(ctx, req) + case len(parquetIDs) == 0: + return h.tsdb.LabelValues(ctx, req) + } + + var parquetResp, tsdbResp *storepb.LabelValuesResponse + g, gCtx := errgroup.WithContext(ctx) + g.Go(func() error { + r, err := labelValuesRequestForBlocks(req, reqHints, parquetIDs) + if err != nil { + return err + } + parquetResp, err = store.LabelValues(gCtx, r) + return err + }) + g.Go(func() error { + r, err := labelValuesRequestForBlocks(req, reqHints, tsdbIDs) + if err != nil { + return err + } + tsdbResp, err = h.tsdb.LabelValues(gCtx, r) + return err + }) + if err := g.Wait(); err != nil { + return nil, err + } + + anyHints, err := mergeLabelValuesHints(parquetResp, tsdbResp) + if err != nil { + return nil, err + } + return &storepb.LabelValuesResponse{ + Values: parquet_util.MergeUnsortedSlices(int(req.Limit), parquetResp.Values, tsdbResp.Values), + Warnings: append(parquetResp.Warnings, tsdbResp.Warnings...), + Hints: anyHints, + }, nil +} + +func mergeLabelValuesHints(a, b *storepb.LabelValuesResponse) (*types.Any, error) { + merged := &hintspb.LabelValuesResponseHints{} + for _, resp := range []*storepb.LabelValuesResponse{a, b} { + if resp == nil || resp.Hints == nil { + continue + } + hints := hintspb.LabelValuesResponseHints{} + if err := types.UnmarshalAny(resp.Hints, &hints); err != nil { + return nil, errors.Wrap(err, "unmarshal label values response hints") + } + merged.QueriedBlocks = append(merged.QueriedBlocks, hints.QueriedBlocks...) + } + return types.MarshalAny(merged) +} diff --git a/pkg/storegateway/hybrid_bucket_stores_test.go b/pkg/storegateway/hybrid_bucket_stores_test.go new file mode 100644 index 00000000000..83bd851d690 --- /dev/null +++ b/pkg/storegateway/hybrid_bucket_stores_test.go @@ -0,0 +1,898 @@ +package storegateway + +import ( + "bytes" + "context" + "errors" + "fmt" + "math" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/go-kit/log" + "github.com/gogo/protobuf/types" + ulidv2 "github.com/oklog/ulid/v2" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/prometheus/common/promslog" + "github.com/prometheus/prometheus/model/labels" + "github.com/prometheus/prometheus/tsdb" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/thanos-io/objstore" + "github.com/thanos-io/thanos/pkg/block" + thanos_metadata "github.com/thanos-io/thanos/pkg/block/metadata" + "github.com/thanos-io/thanos/pkg/extprom" + "github.com/thanos-io/thanos/pkg/store/hintspb" + "github.com/thanos-io/thanos/pkg/store/storepb" + "github.com/weaveworks/common/user" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/cortexproject/cortex/pkg/storage/bucket" + "github.com/cortexproject/cortex/pkg/storage/bucket/filesystem" + cortex_parquet "github.com/cortexproject/cortex/pkg/storage/parquet" + cortex_tsdb "github.com/cortexproject/cortex/pkg/storage/tsdb" + "github.com/cortexproject/cortex/pkg/storage/tsdb/bucketindex" + "github.com/cortexproject/cortex/pkg/util/validation" +) + +func TestHybridBucketStores_Series_ShouldReturnErrorIfMaxInflightRequestIsReached(t *testing.T) { + cfg := prepareStorageConfig(t) + cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + cfg.BucketStore.MaxInflightRequests = 10 + reg := prometheus.NewPedanticRegistry() + storageDir := t.TempDir() + generateStorageBlock(t, storageDir, "user_id", "series_1", 0, 100, 15) + bucket, err := filesystem.NewBucketClient(filesystem.Config{Directory: storageDir}) + require.NoError(t, err) + + stores, err := NewBucketStores(cfg, NewNoShardingStrategy(log.NewNopLogger(), nil), objstore.WithNoopInstr(bucket), defaultLimitsOverrides(t), mockLoggingLevel(), log.NewNopLogger(), reg) + require.NoError(t, err) + require.NoError(t, stores.InitialSync(context.Background())) + + hybridStores := stores.(*HybridBucketStores) + // Set inflight requests to the limit + for range 10 { + hybridStores.inflightRequests.Inc() + } + series, warnings, err := querySeries(stores, "user_id", "series_1", 0, 100) + assert.ErrorIs(t, err, ErrTooManyInflightRequests) + assert.Empty(t, series) + assert.Empty(t, warnings) +} + +func TestHybridBucketStores_Series_ShouldNotCheckMaxInflightRequestsIfTheLimitIsDisabled(t *testing.T) { + cfg := prepareStorageConfig(t) + cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + reg := prometheus.NewPedanticRegistry() + storageDir := t.TempDir() + userId := "user_id" + generateStorageBlock(t, storageDir, userId, "series_1", 0, 100, 15) + bkt, err := filesystem.NewBucketClient(filesystem.Config{Directory: storageDir}) + require.NoError(t, err) + + stores, err := NewBucketStores(cfg, NewNoShardingStrategy(log.NewNopLogger(), nil), objstore.WithNoopInstr(bkt), defaultLimitsOverrides(t), mockLoggingLevel(), log.NewNopLogger(), reg) + require.NoError(t, err) + require.NoError(t, stores.InitialSync(context.Background())) + + hybridStores := stores.(*HybridBucketStores) + // Set inflight requests to the limit (max_inflight_request is set to 0 by default = disabled) + for range 10 { + hybridStores.inflightRequests.Inc() + } + + userPath := fmt.Sprintf("%s/%s", storageDir, userId) + + limits := validation.Limits{} + overrides := validation.NewOverrides(limits, nil) + uBucket := bucket.NewUserBucketClient(userId, bkt, overrides) + blockIds, err := convertToParquetBlocksForTesting(userPath, uBucket) + require.NoError(t, err) + + series, _, err := querySeries(stores, userId, "series_1", 0, 100, blockIds...) + require.NoError(t, err) + assert.Equal(t, 1, len(series)) +} + +// TestHybridBucketStores_SharesCaches verifies that, in Parquet mode, the Parquet +// store and its TSDB fallback share the caching bucket and the matchers cache. +func TestHybridBucketStores_SharesCaches(t *testing.T) { + cfg := prepareStorageConfig(t) + cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + // Enable the caches that both stores would otherwise register independently. + cfg.BucketStore.ChunksCache.Backend = cortex_tsdb.CacheBackendInMemory + cfg.BucketStore.MetadataCache.Backend = cortex_tsdb.CacheBackendInMemory + cfg.BucketStore.MatchersCacheMaxItems = 100 + + bkt, err := filesystem.NewBucketClient(filesystem.Config{Directory: t.TempDir()}) + require.NoError(t, err) + + // A pedantic registry makes any duplicate metric registration fail. + reg := prometheus.NewPedanticRegistry() + stores, err := NewBucketStores(cfg, NewNoShardingStrategy(log.NewNopLogger(), nil), objstore.WithNoopInstr(bkt), defaultLimitsOverrides(t), mockLoggingLevel(), log.NewNopLogger(), reg) + require.NoError(t, err) + require.NotNil(t, stores) +} + +// setupHybridParquetTSDB creates one Parquet-converted block ([0,100) with metricParquet) and one +// plain TSDB block ([100,200) with metricTSDB) in Parquet store-gateway mode, writes a bucket index +// recording the Parquet conversion, then builds and initial-syncs the hybrid stores. +func setupHybridParquetTSDB(t *testing.T, userID string, metricParquet, metricTSDB string) (BucketStores, *prometheus.Registry, []string) { + t.Helper() + + storageDir := t.TempDir() + cfg := prepareStorageConfig(t) + cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + cfg.BucketStore.BucketIndex.Enabled = true + + // Parquet block [0,100): create alone so only this block is converted. + generateStorageBlock(t, storageDir, userID, metricParquet, 0, 100, 15) + + bkt, err := filesystem.NewBucketClient(filesystem.Config{Directory: storageDir}) + require.NoError(t, err) + overrides := validation.NewOverrides(validation.Limits{}, nil) + uBucket := bucket.NewUserBucketClient(userID, bkt, overrides) + userPath := filepath.Join(storageDir, userID) + + parquetBlockIDs, err := convertToParquetBlocksForTesting(userPath, uBucket) + require.NoError(t, err) + require.Len(t, parquetBlockIDs, 1) + + // TSDB block [100,200): not converted. + generateStorageBlock(t, storageDir, userID, metricTSDB, 100, 200, 15) + + entries, err := os.ReadDir(userPath) + require.NoError(t, err) + var tsdbBlockIDs []string + for _, e := range entries { + if _, parseErr := ulidv2.Parse(e.Name()); parseErr == nil && e.Name() != parquetBlockIDs[0] { + tsdbBlockIDs = append(tsdbBlockIDs, e.Name()) + } + } + require.Len(t, tsdbBlockIDs, 1) + allBlockIDs := append(parquetBlockIDs, tsdbBlockIDs...) + + writeParquetBucketIndex(t, bkt, uBucket, userID, parquetBlockIDs[0]) + + reg := prometheus.NewRegistry() + stores, err := NewBucketStores(cfg, NewNoShardingStrategy(log.NewNopLogger(), nil), objstore.WithNoopInstr(bkt), defaultLimitsOverrides(t), mockLoggingLevel(), log.NewNopLogger(), reg) + require.NoError(t, err) + require.NoError(t, stores.InitialSync(context.Background())) + + return stores, reg, allBlockIDs +} + +// writeParquetBucketIndex writes a bucket index recording parquetBlockID as converted, so the +// TSDB sub-store drops it via IgnoreParquetBlocksFilter and hybrid routing serves it from Parquet. +func writeParquetBucketIndex(t *testing.T, bkt objstore.Bucket, uBucket objstore.Bucket, userID, parquetBlockID string) { + t.Helper() + + parquetUID, err := ulidv2.Parse(parquetBlockID) + require.NoError(t, err) + require.NoError(t, uBucket.Upload(context.Background(), bucketindex.ConverterMarkFilePath(parquetUID), bytes.NewReader([]byte("{}")))) + idx, _, _, err := bucketindex.NewUpdater(bkt, userID, nil, log.NewNopLogger()).EnableParquet().UpdateIndex(context.Background(), nil) + require.NoError(t, err) + require.NoError(t, bucketindex.WriteIndex(context.Background(), bkt, userID, nil, idx)) +} + +// TestHybridBucketStores_Series verifies that with one Parquet-converted block and one plain TSDB +// block, both are served correctly in Parquet store-gateway mode, and that the same series +// spanning both stores is merged into one. +func TestHybridBucketStores_Series(t *testing.T) { + const userID = "user-1" + + assertBothServed := func(t *testing.T, stores BucketStores, allBlockIDs []string) { + t.Helper() + seriesFromParquet, _, err := querySeries(stores, userID, "series_parquet", 0, 100, allBlockIDs...) + require.NoError(t, err) + require.Len(t, seriesFromParquet, 1, "series from Parquet-converted block must be returned") + + seriesFromTSDB, _, err := querySeries(stores, userID, "series_tsdb", 100, 200, allBlockIDs...) + require.NoError(t, err) + require.Len(t, seriesFromTSDB, 1, "series from TSDB block must be returned") + } + + t.Run("parquet and tsdb blocks both served", func(t *testing.T) { + stores, _, allBlockIDs := setupHybridParquetTSDB(t, userID, "series_parquet", "series_tsdb") + assertBothServed(t, stores, allBlockIDs) + }) + + t.Run("same series spanning both stores is merged into one", func(t *testing.T) { + stores, _, allBlockIDs := setupHybridParquetTSDB(t, userID, "same_series", "same_series") + + // Same label set in both blocks: MergeSeriesSets must yield one series covering [0,200). + series, _, err := querySeries(stores, userID, "same_series", 0, 200, allBlockIDs...) + require.NoError(t, err) + require.Len(t, series, 1, "same series across two blocks must be merged into one") + + require.NotEmpty(t, series[0].Chunks) + minT, maxT := int64(math.MaxInt64), int64(math.MinInt64) + for _, chk := range series[0].Chunks { + if chk.MinTime < minT { + minT = chk.MinTime + } + if chk.MaxTime > maxT { + maxT = chk.MaxTime + } + } + assert.Less(t, minT, int64(100), "chunks must cover the Parquet block [0, 100)") + assert.GreaterOrEqual(t, maxT, int64(100), "chunks must cover the TSDB block [100, 200)") + }) +} + +// TestHybridBucketStores_SyncedBlocksMetrics verifies that IgnoreParquetBlocksFilter excludes the +// converted block from the TSDB sub-store sync (loaded=1, parquet-converted=1). +func TestHybridBucketStores_SyncedBlocksMetrics(t *testing.T) { + const userID = "user-1" + + _, reg, _ := setupHybridParquetTSDB(t, userID, "series_parquet", "series_tsdb") + require.NoError(t, testutil.GatherAndCompare(reg, strings.NewReader(` + # HELP cortex_blocks_meta_synced Reflects current state of synced blocks (over all tenants). + # TYPE cortex_blocks_meta_synced gauge + cortex_blocks_meta_synced{state="corrupted-bucket-index"} 0 + cortex_blocks_meta_synced{state="corrupted-meta-json"} 0 + cortex_blocks_meta_synced{state="duplicate"} 0 + cortex_blocks_meta_synced{state="failed"} 0 + cortex_blocks_meta_synced{state="label-excluded"} 0 + cortex_blocks_meta_synced{state="loaded"} 1 + cortex_blocks_meta_synced{state="marked-for-deletion"} 0 + cortex_blocks_meta_synced{state="marked-for-no-compact"} 0 + cortex_blocks_meta_synced{state="no-bucket-index"} 0 + cortex_blocks_meta_synced{state="no-meta-json"} 0 + cortex_blocks_meta_synced{state="parquet-converted"} 1 + cortex_blocks_meta_synced{state="parquet-migrated"} 0 + cortex_blocks_meta_synced{state="time-excluded"} 0 + cortex_blocks_meta_synced{state="too-fresh"} 0 + `), "cortex_blocks_meta_synced")) +} + +// newHybridStoresForRouting returns hybrid stores whose TSDB sub-store has the given Parquet filters, +// which is all splitRequestedBlocks needs. +func newHybridStoresForRouting(filters map[string]*IgnoreParquetBlocksFilter) *HybridBucketStores { + return &HybridBucketStores{ + tsdb: &ThanosBucketStores{parquetFilters: filters}, + metrics: newHybridBucketStoresMetrics(prometheus.NewRegistry()), + } +} + +// TestHybridBucketStores_splitRequestedBlocks_UsesParquetBlockIDs verifies that routing is driven by +// the Parquet block IDs recorded by the TSDB store's Parquet filter, so a block is routed to the +// Parquet store iff it is converted in the bucket index. +func TestHybridBucketStores_splitRequestedBlocks_UsesParquetBlockIDs(t *testing.T) { + const userID = "user-1" + + converted := ulidv2.MustNew(1, nil) // converted -> excluded from TSDB -> parquet + kept := ulidv2.MustNew(2, nil) // not converted -> stays in TSDB -> tsdb + unknown := ulidv2.MustNew(3, nil) // not in the index -> tsdb + + // Run the filter over a fresh index so it records the Parquet block IDs. + filter := NewIgnoreParquetBlocksFilter(log.NewNopLogger()) + metas := map[ulidv2.ULID]*thanos_metadata.Meta{ + converted: {}, + kept: {}, + } + idx := &bucketindex.Index{ + Blocks: bucketindex.Blocks{ + {ID: converted, Parquet: &cortex_parquet.ConverterMarkMeta{Version: cortex_parquet.CurrentVersion}}, + {ID: kept}, + }, + } + require.NoError(t, filter.FilterWithBucketIndex(context.Background(), metas, idx, extprom.NewTxGaugeVec(nil, prometheus.GaugeOpts{Name: "synced"}, []string{"state"}))) + + h := newHybridStoresForRouting(map[string]*IgnoreParquetBlocksFilter{userID: filter}) + + parquetIDs, tsdbIDs, err := h.splitRequestedBlocks(userID, blockIDsMatcher([]string{converted.String(), kept.String(), unknown.String()}), "Series") + require.NoError(t, err) + assert.Equal(t, []string{converted.String()}, parquetIDs, "converted block must be routed to parquet") + assert.Equal(t, []string{kept.String(), unknown.String()}, tsdbIDs, "non-converted and unknown blocks must be routed to tsdb") +} + +// TestHybridBucketStores_splitRequestedBlocks_UnknownParquetBlockIDsRoutesTSDB verifies that every +// block is routed to the TSDB store when the Parquet block IDs are not known yet (e.g. the user has +// not been synced yet). +func TestHybridBucketStores_splitRequestedBlocks_UnknownParquetBlockIDsRoutesTSDB(t *testing.T) { + const userID = "user-1" + + b1 := ulidv2.MustNew(1, nil) + b2 := ulidv2.MustNew(2, nil) + + for name, filters := range map[string]map[string]*IgnoreParquetBlocksFilter{ + "no filter for user": {}, + "filter registered, never run": {userID: NewIgnoreParquetBlocksFilter(log.NewNopLogger())}, + } { + t.Run(name, func(t *testing.T) { + h := newHybridStoresForRouting(filters) + + parquetIDs, tsdbIDs, err := h.splitRequestedBlocks(userID, blockIDsMatcher([]string{b1.String(), b2.String()}), "Series") + require.NoError(t, err) + assert.Empty(t, parquetIDs) + assert.Equal(t, []string{b1.String(), b2.String()}, tsdbIDs, "blocks must be routed to TSDB when Parquet block IDs are unknown") + }) + } +} + +// TestNewBucketStores_ParquetFallbackDisabled verifies that disabling the Parquet fallback keeps +// the Parquet-only store, which runs without the TSDB sub-store and does not need the bucket index. +func TestNewBucketStores_ParquetFallbackDisabled(t *testing.T) { + cfg := prepareStorageConfig(t) + cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + cfg.BucketStore.ParquetFallbackDisabled = true + cfg.BucketStore.BucketIndex.Enabled = false + + bkt, err := filesystem.NewBucketClient(filesystem.Config{Directory: t.TempDir()}) + require.NoError(t, err) + + stores, err := NewBucketStores(cfg, NewNoShardingStrategy(log.NewNopLogger(), nil), objstore.WithNoopInstr(bkt), defaultLimitsOverrides(t), mockLoggingLevel(), log.NewNopLogger(), prometheus.NewPedanticRegistry()) + require.NoError(t, err) + assert.IsType(t, &ParquetBucketStores{}, stores) +} + +func TestThanosBucketStores_parquetBlockIDs(t *testing.T) { + const userID = "user-1" + + t.Run("no filter for user -> nil", func(t *testing.T) { + u := &ThanosBucketStores{parquetFilters: map[string]*IgnoreParquetBlocksFilter{}} + assert.Nil(t, u.parquetBlockIDs(userID)) + }) + + t.Run("filter registered but never run -> nil", func(t *testing.T) { + u := &ThanosBucketStores{parquetFilters: map[string]*IgnoreParquetBlocksFilter{ + userID: NewIgnoreParquetBlocksFilter(log.NewNopLogger()), + }} + assert.Nil(t, u.parquetBlockIDs(userID)) + }) + + t.Run("filter run -> recorded set", func(t *testing.T) { + parquetBlock := ulidv2.MustNew(1, nil) + filter := NewIgnoreParquetBlocksFilter(log.NewNopLogger()) + require.NoError(t, filter.FilterWithBucketIndex(context.Background(), + map[ulidv2.ULID]*thanos_metadata.Meta{parquetBlock: {}}, + &bucketindex.Index{Blocks: bucketindex.Blocks{ + {ID: parquetBlock, Parquet: &cortex_parquet.ConverterMarkMeta{Version: cortex_parquet.CurrentVersion}}, + }}, + extprom.NewTxGaugeVec(nil, prometheus.GaugeOpts{Name: "synced"}, []string{"state"}), + )) + + u := &ThanosBucketStores{parquetFilters: map[string]*IgnoreParquetBlocksFilter{userID: filter}} + assert.Contains(t, u.parquetBlockIDs(userID), parquetBlock.String()) + }) +} + +func TestHybridBucketStores_splitRequestedBlocks_NoParquetBlocksRoutesTSDB(t *testing.T) { + const userID = "user-1" + + b1 := ulidv2.MustNew(1, nil) + b2 := ulidv2.MustNew(2, nil) + + filter := NewIgnoreParquetBlocksFilter(log.NewNopLogger()) + require.NoError(t, filter.FilterWithBucketIndex(context.Background(), + map[ulidv2.ULID]*thanos_metadata.Meta{b1: {}, b2: {}}, + &bucketindex.Index{Blocks: bucketindex.Blocks{{ID: b1}, {ID: b2}}}, // no Parquet blocks + extprom.NewTxGaugeVec(nil, prometheus.GaugeOpts{Name: "synced"}, []string{"state"}), + )) + + h := newHybridStoresForRouting(map[string]*IgnoreParquetBlocksFilter{userID: filter}) + + parquetIDs, tsdbIDs, err := h.splitRequestedBlocks(userID, blockIDsMatcher([]string{b1.String(), b2.String()}), "Series") + require.NoError(t, err) + assert.Empty(t, parquetIDs) + assert.Equal(t, []string{b1.String(), b2.String()}, tsdbIDs, "blocks must be routed to TSDB when no block is converted") +} + +// TestHybridBucketStores_Series_MergedSortedByLabels verifies that when distinct series come +// from both the Parquet store and the TSDB store in the same query, the hybrid layer returns +// them as a single label-sorted stream. +func TestHybridBucketStores_Series_MergedSortedByLabels(t *testing.T) { + const ( + userID = "user-1" + metricName = "merge_metric" + ) + + storageDir := t.TempDir() + cfg := prepareStorageConfig(t) + cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + cfg.BucketStore.BucketIndex.Enabled = true + + // Parquet block [0,100): series "a" and "c". + generateStorageBlockWithSeriesValues(t, storageDir, userID, metricName, []string{"a", "c"}, 0, 100, 15) + + bkt, err := filesystem.NewBucketClient(filesystem.Config{Directory: storageDir}) + require.NoError(t, err) + overrides := validation.NewOverrides(validation.Limits{}, nil) + uBucket := bucket.NewUserBucketClient(userID, bkt, overrides) + userPath := filepath.Join(storageDir, userID) + + parquetBlockIDs, err := convertToParquetBlocksForTesting(userPath, uBucket) + require.NoError(t, err) + require.Len(t, parquetBlockIDs, 1) + + // TSDB block [0,100): series "b" and "d", which interleave with the Parquet series once sorted. + generateStorageBlockWithSeriesValues(t, storageDir, userID, metricName, []string{"b", "d"}, 0, 100, 15) + + entries, err := os.ReadDir(userPath) + require.NoError(t, err) + var tsdbBlockIDs []string + for _, e := range entries { + if _, parseErr := ulidv2.Parse(e.Name()); parseErr == nil && e.Name() != parquetBlockIDs[0] { + tsdbBlockIDs = append(tsdbBlockIDs, e.Name()) + } + } + require.Len(t, tsdbBlockIDs, 1) + allBlockIDs := append(parquetBlockIDs, tsdbBlockIDs...) + + // The bucket index routes the Parquet block to the Parquet store and the TSDB block to the TSDB store. + writeParquetBucketIndex(t, bkt, uBucket, userID, parquetBlockIDs[0]) + + reg := prometheus.NewRegistry() + stores, err := NewBucketStores(cfg, NewNoShardingStrategy(log.NewNopLogger(), nil), objstore.WithNoopInstr(bkt), defaultLimitsOverrides(t), mockLoggingLevel(), log.NewNopLogger(), reg) + require.NoError(t, err) + require.NoError(t, stores.InitialSync(context.Background())) + + // Exercise both the unbatched path (batchSize 0/1 => individual Series responses) and the + // batched path (batchSize >= 2 => Batch responses). + for _, batchSize := range []int64{0, 1, 2, 3} { + t.Run(fmt.Sprintf("batchSize=%d", batchSize), func(t *testing.T) { + series, _, err := querySeriesWithBatchSize(stores, userID, metricName, 0, 100, batchSize, allBlockIDs...) + require.NoError(t, err) + + // Union across both stores: a (parquet), b (tsdb), c (parquet), d (tsdb). + require.Len(t, series, 4, "hybrid must return the union of series from both stores") + + got := make([]string, 0, len(series)) + for i, s := range series { + lset := s.PromLabels() + got = append(got, lset.Get("series")) + if i > 0 { + require.Negative(t, labels.Compare(series[i-1].PromLabels(), lset), + "merged series must be strictly increasing by labels") + } + } + assert.Equal(t, []string{"a", "b", "c", "d"}, got, "series must be label-sorted and interleave both stores") + }) + } +} + +// TestHybridBucketStores_LabelNamesAndValues_Merged verifies that when a query spans both a +// Parquet-converted block and a plain TSDB block, the hybrid layer merges label names and label +// values from both sub-stores (sorted and de-duplicated). +func TestHybridBucketStores_LabelNamesAndValues_Merged(t *testing.T) { + const ( + userID = "user-1" + metricName = "merge_metric" + ) + + storageDir := t.TempDir() + cfg := prepareStorageConfig(t) + cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + cfg.BucketStore.BucketIndex.Enabled = true + + // Parquet block: a Parquet-only label name "pk" and series values a, c. + generateStorageBlockWithLabelSets(t, storageDir, userID, []labels.Labels{ + labels.FromStrings(labels.MetricName, metricName, "series", "a", "pk", "1"), + labels.FromStrings(labels.MetricName, metricName, "series", "c", "pk", "1"), + }, 0, 100, 15) + + bkt, err := filesystem.NewBucketClient(filesystem.Config{Directory: storageDir}) + require.NoError(t, err) + overrides := validation.NewOverrides(validation.Limits{}, nil) + uBucket := bucket.NewUserBucketClient(userID, bkt, overrides) + userPath := filepath.Join(storageDir, userID) + + parquetBlockIDs, err := convertToParquetBlocksForTesting(userPath, uBucket) + require.NoError(t, err) + require.Len(t, parquetBlockIDs, 1) + + // TSDB block: a TSDB-only label name "tk" and series values b, c, d. The value "c" is shared + // with the Parquet block to exercise de-duplication in the merged LabelValues response. + generateStorageBlockWithLabelSets(t, storageDir, userID, []labels.Labels{ + labels.FromStrings(labels.MetricName, metricName, "series", "b", "tk", "1"), + labels.FromStrings(labels.MetricName, metricName, "series", "c", "tk", "1"), + labels.FromStrings(labels.MetricName, metricName, "series", "d", "tk", "1"), + }, 0, 100, 15) + + entries, err := os.ReadDir(userPath) + require.NoError(t, err) + var tsdbBlockIDs []string + for _, e := range entries { + if _, parseErr := ulidv2.Parse(e.Name()); parseErr == nil && e.Name() != parquetBlockIDs[0] { + tsdbBlockIDs = append(tsdbBlockIDs, e.Name()) + } + } + require.Len(t, tsdbBlockIDs, 1) + allBlockIDs := append(parquetBlockIDs, tsdbBlockIDs...) + + // The bucket index routes the Parquet block to the Parquet store and the TSDB block to the TSDB store. + writeParquetBucketIndex(t, bkt, uBucket, userID, parquetBlockIDs[0]) + + reg := prometheus.NewRegistry() + stores, err := NewBucketStores(cfg, NewNoShardingStrategy(log.NewNopLogger(), nil), objstore.WithNoopInstr(bkt), defaultLimitsOverrides(t), mockLoggingLevel(), log.NewNopLogger(), reg) + require.NoError(t, err) + require.NoError(t, stores.InitialSync(context.Background())) + + t.Run("LabelNames merges names from both stores", func(t *testing.T) { + resp, err := queryLabelsNamesWithBlocks(stores, userID, metricName, 0, 100, allBlockIDs...) + require.NoError(t, err) + // __name__ and series from both, pk only from Parquet, tk only from TSDB. + assert.Equal(t, []string{labels.MetricName, "pk", "series", "tk"}, resp.Names) + }) + + t.Run("LabelValues merges and de-duplicates values from both stores", func(t *testing.T) { + resp, err := queryLabelsValuesWithBlocks(stores, userID, "series", metricName, 0, 100, allBlockIDs...) + require.NoError(t, err) + // a,c (Parquet) + b,c,d (TSDB) -> deduplicated, sorted union. + assert.Equal(t, []string{"a", "b", "c", "d"}, resp.Values) + }) +} + +// TestHybridBucketStores_Series_StaleIndexRoutingGap guards against a data-gap that can happen +// right after a block is converted to Parquet: the TSDB sub-store reads the bucket index fresh on +// every sync and drops the converted block via IgnoreParquetBlocksFilter. The hybrid layer must +// still serve that block. +func TestHybridBucketStores_Series_StaleIndexRoutingGap(t *testing.T) { + const userID = "user-1" + + storageDir := t.TempDir() + cfg := prepareStorageConfig(t) + cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + cfg.BucketStore.BucketIndex.Enabled = true + // A long sync interval keeps background refreshes from interfering with the manual re-sync below. + cfg.BucketStore.SyncInterval = time.Hour + + // Block A [0,100): metric_a. Will be converted to Parquet. + generateStorageBlock(t, storageDir, userID, "metric_a", 0, 100, 15) + + bkt, err := filesystem.NewBucketClient(filesystem.Config{Directory: storageDir}) + require.NoError(t, err) + overrides := validation.NewOverrides(validation.Limits{}, nil) + uBucket := bucket.NewUserBucketClient(userID, bkt, overrides) + userPath := filepath.Join(storageDir, userID) + + parquetBlockIDs, err := convertToParquetBlocksForTesting(userPath, uBucket) + require.NoError(t, err) + require.Len(t, parquetBlockIDs, 1) + parquetUID, err := ulidv2.Parse(parquetBlockIDs[0]) + require.NoError(t, err) + + // v1 = the "before conversion" bucket index snapshot (A recorded as a plain TSDB block). + idxV1, _, _, err := bucketindex.NewUpdater(bkt, userID, nil, log.NewNopLogger()).UpdateIndex(context.Background(), nil) + require.NoError(t, err) + require.NoError(t, bucketindex.WriteIndex(context.Background(), bkt, userID, nil, idxV1)) + + reg := prometheus.NewRegistry() + stores, err := NewBucketStores(cfg, NewNoShardingStrategy(log.NewNopLogger(), nil), objstore.WithNoopInstr(bkt), defaultLimitsOverrides(t), mockLoggingLevel(), log.NewNopLogger(), reg) + require.NoError(t, err) + require.NoError(t, stores.InitialSync(context.Background())) + + hStores := stores.(*HybridBucketStores) + + // v2 = the "after conversion" bucket index snapshot (A recorded as Parquet). It overwrites + // the index file in storage, so the TSDB sub-store drops A on the next sync. + require.NoError(t, uBucket.Upload(context.Background(), bucketindex.ConverterMarkFilePath(parquetUID), bytes.NewReader([]byte("{}")))) + idxV2, _, _, err := bucketindex.NewUpdater(bkt, userID, nil, log.NewNopLogger()).EnableParquet().UpdateIndex(context.Background(), nil) + require.NoError(t, err) + require.NoError(t, bucketindex.WriteIndex(context.Background(), bkt, userID, nil, idxV2)) + + // Re-sync the TSDB sub-store: it reads the fresh index, excludes block A, and records A as a + // Parquet block so hybrid routing sends A to the Parquet store. + require.NoError(t, hStores.tsdb.SyncBlocks(context.Background())) + + // Block A was excluded from the TSDB store; its recorded Parquet block ID makes hybrid route it + // to the Parquet store instead. Block A's series must still be returned — otherwise data has silently vanished + // during the TSDB -> Parquet handover. + series, _, err := querySeries(stores, userID, "metric_a", 0, 100, parquetBlockIDs...) + require.NoError(t, err) + require.Len(t, series, 1, "series from the just-converted block must not silently disappear after the TSDB store drops it") +} + +func queryLabelsNamesWithBlocks(stores BucketStores, userID, metricName string, start, end int64, blockIDs ...string) (*storepb.LabelNamesResponse, error) { + req := &storepb.LabelNamesRequest{ + Start: start, + End: end, + Matchers: []storepb.LabelMatcher{{ + Type: storepb.LabelMatcher_EQ, + Name: labels.MetricName, + Value: metricName, + }}, + PartialResponseStrategy: storepb.PartialResponseStrategy_ABORT, + } + if len(blockIDs) > 0 { + hints := &hintspb.LabelNamesRequestHints{ + BlockMatchers: []storepb.LabelMatcher{{ + Type: storepb.LabelMatcher_RE, + Name: block.BlockIDLabel, + Value: strings.Join(blockIDs, "|"), + }}, + } + anyHints, err := types.MarshalAny(hints) + if err != nil { + return nil, err + } + req.Hints = anyHints + } + + ctx := setUserIDToGRPCContext(context.Background(), userID) + return stores.LabelNames(ctx, req) +} + +func queryLabelsValuesWithBlocks(stores BucketStores, userID, labelName, metricName string, start, end int64, blockIDs ...string) (*storepb.LabelValuesResponse, error) { + req := &storepb.LabelValuesRequest{ + Start: start, + End: end, + Label: labelName, + Matchers: []storepb.LabelMatcher{{ + Type: storepb.LabelMatcher_EQ, + Name: labels.MetricName, + Value: metricName, + }}, + PartialResponseStrategy: storepb.PartialResponseStrategy_ABORT, + } + if len(blockIDs) > 0 { + hints := &hintspb.LabelValuesRequestHints{ + BlockMatchers: []storepb.LabelMatcher{{ + Type: storepb.LabelMatcher_RE, + Name: block.BlockIDLabel, + Value: strings.Join(blockIDs, "|"), + }}, + } + anyHints, err := types.MarshalAny(hints) + if err != nil { + return nil, err + } + req.Hints = anyHints + } + + ctx := setUserIDToGRPCContext(context.Background(), userID) + return stores.LabelValues(ctx, req) +} + +// generateStorageBlockWithSeriesValues creates a single TSDB block containing one series per +// provided "series" label value (all sharing the same metric name). It lets tests control the +// exact label ordering across blocks. +func generateStorageBlockWithSeriesValues(t testing.TB, storageDir, userID, metricName string, seriesValues []string, minT, maxT int64, step int) { + t.Helper() + userDir := filepath.Join(storageDir, userID) + if _, err := os.Stat(userDir); os.IsNotExist(err) { + require.NoError(t, os.Mkdir(userDir, os.ModePerm)) + } + + tmpDir := t.TempDir() + db, err := tsdb.Open(tmpDir, promslog.NewNopLogger(), nil, tsdb.DefaultOptions(), nil) + require.NoError(t, err) + defer func() { require.NoError(t, db.Close()) }() + + app := db.Appender(context.Background()) + for i, v := range seriesValues { + lbls := labels.FromStrings(labels.MetricName, metricName, "series", v) + for ts := minT; ts < maxT; ts += int64(step) { + _, err = app.Append(0, lbls, ts, float64(i)) + require.NoError(t, err) + } + } + require.NoError(t, app.Commit()) + require.NoError(t, db.Snapshot(userDir, true)) +} + +// generateStorageBlockWithLabelSets creates a single TSDB block containing exactly the provided +// series label sets. It lets tests control both the label names and values present in each block. +func generateStorageBlockWithLabelSets(t *testing.T, storageDir, userID string, seriesLabels []labels.Labels, minT, maxT int64, step int) { + t.Helper() + userDir := filepath.Join(storageDir, userID) + if _, err := os.Stat(userDir); os.IsNotExist(err) { + require.NoError(t, os.Mkdir(userDir, os.ModePerm)) + } + + tmpDir := t.TempDir() + db, err := tsdb.Open(tmpDir, promslog.NewNopLogger(), nil, tsdb.DefaultOptions(), nil) + require.NoError(t, err) + defer func() { require.NoError(t, db.Close()) }() + + app := db.Appender(context.Background()) + for i, lbls := range seriesLabels { + for ts := minT; ts < maxT; ts += int64(step) { + _, err = app.Append(0, lbls, ts, float64(i)) + require.NoError(t, err) + } + } + require.NoError(t, app.Commit()) + require.NoError(t, db.Snapshot(userDir, true)) +} + +// setupHybridMixedStores creates one Parquet-converted block holding parquetSeries and one plain +// TSDB block holding tsdbSeries (both [0,100) for metricName), writes the bucket index and returns +// the initial-synced hybrid stores along with the storage dir, the Parquet block ID and all block IDs. +func setupHybridMixedStores(t *testing.T, cfg cortex_tsdb.BlocksStorageConfig, userID, metricName string, parquetSeries, tsdbSeries []string) (*HybridBucketStores, string, string, []string) { + t.Helper() + + cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + cfg.BucketStore.BucketIndex.Enabled = true + + storageDir := t.TempDir() + generateStorageBlockWithSeriesValues(t, storageDir, userID, metricName, parquetSeries, 0, 100, 15) + + bkt, err := filesystem.NewBucketClient(filesystem.Config{Directory: storageDir}) + require.NoError(t, err) + uBucket := bucket.NewUserBucketClient(userID, bkt, validation.NewOverrides(validation.Limits{}, nil)) + userPath := filepath.Join(storageDir, userID) + + parquetBlockIDs, err := convertToParquetBlocksForTesting(userPath, uBucket) + require.NoError(t, err) + require.Len(t, parquetBlockIDs, 1) + + generateStorageBlockWithSeriesValues(t, storageDir, userID, metricName, tsdbSeries, 0, 100, 15) + + entries, err := os.ReadDir(userPath) + require.NoError(t, err) + allBlockIDs := append([]string{}, parquetBlockIDs...) + for _, e := range entries { + if _, parseErr := ulidv2.Parse(e.Name()); parseErr == nil && e.Name() != parquetBlockIDs[0] { + allBlockIDs = append(allBlockIDs, e.Name()) + } + } + require.Len(t, allBlockIDs, 2) + + writeParquetBucketIndex(t, bkt, uBucket, userID, parquetBlockIDs[0]) + + stores, err := NewBucketStores(cfg, NewNoShardingStrategy(log.NewNopLogger(), nil), objstore.WithNoopInstr(bkt), defaultLimitsOverrides(t), mockLoggingLevel(), log.NewNopLogger(), prometheus.NewRegistry()) + require.NoError(t, err) + require.NoError(t, stores.InitialSync(context.Background())) + + return stores.(*HybridBucketStores), storageDir, parquetBlockIDs[0], allBlockIDs +} + +func seriesValues(prefix string, n int) []string { + values := make([]string, 0, n) + for i := range n { + values = append(values, fmt.Sprintf("%s%05d", prefix, i)) + } + return values +} + +func newMixedSeriesRequest(t *testing.T, userID, metricName string, enableQueryStats bool, blockIDs []string) (*storepb.SeriesRequest, context.Context) { + t.Helper() + + anyHints, err := types.MarshalAny(&hintspb.SeriesRequestHints{ + BlockMatchers: blockIDsMatcher(blockIDs), + EnableQueryStats: enableQueryStats, + }) + require.NoError(t, err) + + req := &storepb.SeriesRequest{ + MinTime: 0, + MaxTime: 100, + Matchers: []storepb.LabelMatcher{{ + Type: storepb.LabelMatcher_EQ, + Name: labels.MetricName, + Value: metricName, + }}, + PartialResponseStrategy: storepb.PartialResponseStrategy_ABORT, + Hints: anyHints, + } + ctx := user.InjectOrgID(setUserIDToGRPCContext(context.Background(), userID), userID) + return req, ctx +} + +// TestHybridBucketStores_Series_MixedPropagatesSubStoreError verifies that when one sub-store fails +// while serving a mixed query, the client receives that sub-store's error rather than the +// cancellation error of the other sub-store, so the querier can still classify it (retry, limit, ...). +func TestHybridBucketStores_Series_MixedPropagatesSubStoreError(t *testing.T) { + const ( + userID = "user-1" + metricName = "mixed_metric" + ) + + t.Run("tsdb store fails", func(t *testing.T) { + cfg := prepareStorageConfig(t) + cfg.BucketStore.MaxInflightRequests = 1 + // Many Parquet series keep the Parquet producer running when the TSDB store fails, so it + // gets canceled and would mask the TSDB error. + h, _, _, allBlockIDs := setupHybridMixedStores(t, cfg, userID, metricName, seriesValues("p", 2000), seriesValues("t", 1)) + + // Saturate only the TSDB sub-store so it fails with ErrTooManyInflightRequests. + h.tsdb.inflightRequests.Inc() + + for range 5 { + _, _, err := querySeries(h, userID, metricName, 0, 100, allBlockIDs...) + require.ErrorIs(t, err, ErrTooManyInflightRequests) + assert.Equal(t, codes.ResourceExhausted, status.Code(err)) + } + }) + + t.Run("parquet store fails", func(t *testing.T) { + h, storageDir, parquetBlockID, allBlockIDs := setupHybridMixedStores(t, prepareStorageConfig(t), userID, metricName, seriesValues("p", 1), seriesValues("t", 2000)) + + // Remove the Parquet files so the Parquet store fails to open the shard. + parquetFiles, err := filepath.Glob(filepath.Join(storageDir, userID, parquetBlockID, "*.parquet")) + require.NoError(t, err) + require.NotEmpty(t, parquetFiles) + for _, f := range parquetFiles { + require.NoError(t, os.Remove(f)) + } + + _, _, err = querySeries(h, userID, metricName, 0, 100, allBlockIDs...) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to open parquet shard") + assert.NotContains(t, err.Error(), context.Canceled.Error()) + }) +} + +// failingSeriesServer fails every Send once failAfter series have been sent. +type failingSeriesServer struct { + *bucketStoreSeriesServer + + failAfter int + sent int +} + +func (s *failingSeriesServer) Send(r *storepb.SeriesResponse) error { + if r.GetSeries() != nil { + s.sent++ + } + if batch := r.GetBatch(); batch != nil { + s.sent += len(batch.Series) + } + if s.sent > s.failAfter { + return errors.New("client stream closed") + } + return s.bucketStoreSeriesServer.Send(r) +} + +// TestHybridBucketStores_Series_MixedSendFailureStopsProducers verifies that when sending a merged +// series to the client fails, the hybrid store cancels both sub-stores, waits for them, and returns +// the send error. +func TestHybridBucketStores_Series_MixedSendFailureStopsProducers(t *testing.T) { + const ( + userID = "user-1" + metricName = "mixed_metric" + ) + + // Enough series on both sides that both producers are blocked on the channel when the send fails. + h, _, _, allBlockIDs := setupHybridMixedStores(t, prepareStorageConfig(t), userID, metricName, seriesValues("p", 1000), seriesValues("t", 1000)) + + req, ctx := newMixedSeriesRequest(t, userID, metricName, false, allBlockIDs) + srv := &failingSeriesServer{bucketStoreSeriesServer: newBucketStoreSeriesServer(ctx), failAfter: 10} + + errCh := make(chan error, 1) + go func() { errCh <- h.Series(req, srv) }() + + select { + case err := <-errCh: + require.Error(t, err) + assert.Contains(t, err.Error(), "send merged series response") + assert.Contains(t, err.Error(), "client stream closed") + assert.NotContains(t, err.Error(), context.Canceled.Error()) + case <-time.After(30 * time.Second): + t.Fatal("Series did not return after a send failure: sub-store producers are stuck") + } + assert.Len(t, srv.SeriesSet, 10) +} + +// TestHybridBucketStores_Series_MixedMergesHints verifies that the hints sent for a mixed query +// cover the blocks queried by both sub-stores and carry the query stats. +func TestHybridBucketStores_Series_MixedMergesHints(t *testing.T) { + const ( + userID = "user-1" + metricName = "mixed_metric" + ) + + h, _, _, allBlockIDs := setupHybridMixedStores(t, prepareStorageConfig(t), userID, metricName, []string{"a", "c"}, []string{"b", "d"}) + + req, ctx := newMixedSeriesRequest(t, userID, metricName, true, allBlockIDs) + srv := newBucketStoreSeriesServer(ctx) + require.NoError(t, h.Series(req, srv)) + require.Len(t, srv.SeriesSet, 4) + + // The querier consistency check relies on every queried block being reported. + queried := make([]string, 0, len(srv.Hints.QueriedBlocks)) + for _, b := range srv.Hints.QueriedBlocks { + queried = append(queried, b.Id) + } + assert.ElementsMatch(t, allBlockIDs, queried) + + // Only the TSDB store reports query stats; they must survive the merge. + require.NotNil(t, srv.Hints.QueryStats) + assert.Equal(t, int64(1), srv.Hints.QueryStats.BlocksQueried) +} diff --git a/pkg/storegateway/metadata_fetcher_filters.go b/pkg/storegateway/metadata_fetcher_filters.go index 326c544946f..2c8a458ddc2 100644 --- a/pkg/storegateway/metadata_fetcher_filters.go +++ b/pkg/storegateway/metadata_fetcher_filters.go @@ -2,6 +2,7 @@ package storegateway import ( "context" + "sync" "time" "github.com/go-kit/log" @@ -77,6 +78,63 @@ func (f *IgnoreDeletionMarkFilter) FilterWithBucketIndex(_ context.Context, meta return nil } +// IgnoreParquetBlocksFilter drops Parquet-converted blocks from the TSDB store +// sync so they are served by the Parquet store instead. +// +// It also records the IDs of all Parquet-converted blocks in the bucket index and +// exposes them via ParquetBlockIDs. The hybrid bucket store routes a block to the +// Parquet store iff it is recorded here, so excluding a block from the TSDB store and +// routing it to the Parquet store always use the same bucket index snapshot. +type IgnoreParquetBlocksFilter struct { + logger log.Logger + + // parquetBlockIDs is the set of Parquet-converted block IDs in the bucket index as of the + // last sync, including blocks this store-gateway does not own. + parquetBlockIDsMu sync.RWMutex + parquetBlockIDs map[string]struct{} +} + +func NewIgnoreParquetBlocksFilter(logger log.Logger) *IgnoreParquetBlocksFilter { + return &IgnoreParquetBlocksFilter{logger: logger} +} + +// Filter implements block.MetadataFilter. +// +// Without the bucket index we cannot tell whether a block has been converted to +// Parquet, so this is intentionally a no-op. +func (f *IgnoreParquetBlocksFilter) Filter(_ context.Context, _ map[ulid.ULID]*metadata.Meta, _ block.GaugeVec, _ block.GaugeVec) error { + return nil +} + +// FilterWithBucketIndex implements MetadataFilterWithBucketIndex. +func (f *IgnoreParquetBlocksFilter) FilterWithBucketIndex(_ context.Context, metas map[ulid.ULID]*metadata.Meta, idx *bucketindex.Index, synced block.GaugeVec) error { + parquetBlockIDs := make(map[string]struct{}) + for _, b := range idx.ParquetBlocks() { + if _, ok := metas[b.ID]; ok { + level.Debug(f.logger).Log("msg", "ignoring block because it has been converted to parquet", "block", b.ID) + synced.WithLabelValues(parquetConvertedMeta).Inc() + delete(metas, b.ID) + } + + // Record every Parquet block for routing, even one already excluded by a previous filter. + parquetBlockIDs[b.ID.String()] = struct{}{} + } + + f.parquetBlockIDsMu.Lock() + f.parquetBlockIDs = parquetBlockIDs + f.parquetBlockIDsMu.Unlock() + + return nil +} + +// ParquetBlockIDs returns the Parquet-converted block IDs as of the last sync, or nil +// if the filter has not run yet. +func (f *IgnoreParquetBlocksFilter) ParquetBlockIDs() map[string]struct{} { + f.parquetBlockIDsMu.RLock() + defer f.parquetBlockIDsMu.RUnlock() + return f.parquetBlockIDs +} + func NewIgnoreNonQueryableBlocksFilter(logger log.Logger, ignoreWithin time.Duration) *IgnoreNonQueryableBlocksFilter { return &IgnoreNonQueryableBlocksFilter{ logger: logger, diff --git a/pkg/storegateway/metadata_fetcher_filters_test.go b/pkg/storegateway/metadata_fetcher_filters_test.go index 26832fb5854..a7b4f3c6945 100644 --- a/pkg/storegateway/metadata_fetcher_filters_test.go +++ b/pkg/storegateway/metadata_fetcher_filters_test.go @@ -22,6 +22,7 @@ import ( "github.com/prometheus/prometheus/tsdb" "github.com/cortexproject/cortex/pkg/storage/bucket" + cortex_parquet "github.com/cortexproject/cortex/pkg/storage/parquet" "github.com/cortexproject/cortex/pkg/storage/tsdb/bucketindex" cortex_testutil "github.com/cortexproject/cortex/pkg/util/testutil" ) @@ -174,3 +175,48 @@ func TestIgnoreNonQueryableBlocksFilter(t *testing.T) { require.NoError(t, f.Filter(ctx, inputMetas, synced, modified)) assert.Equal(t, expectedMetas, inputMetas) } + +func TestIgnoreParquetBlocksFilter_FilterWithBucketIndex_DropsAndRecords(t *testing.T) { + t.Parallel() + + ctx := context.Background() + logger := log.NewNopLogger() + + var ( + parquetInMetas = ulid.MustNew(1, nil) // parquet + in metas -> dropped, recorded, counted + parquetNotInMetas = ulid.MustNew(2, nil) // parquet but not in metas -> recorded only + plainTSDB = ulid.MustNew(3, nil) // not converted -> always kept, not recorded + ) + + idx := &bucketindex.Index{ + Blocks: bucketindex.Blocks{ + {ID: parquetInMetas, Parquet: &cortex_parquet.ConverterMarkMeta{Version: cortex_parquet.CurrentVersion}}, + {ID: parquetNotInMetas, Parquet: &cortex_parquet.ConverterMarkMeta{Version: cortex_parquet.CurrentVersion}}, + {ID: plainTSDB}, + }, + } + + metas := map[ulid.ULID]*metadata.Meta{ + parquetInMetas: {}, + plainTSDB: {}, + } + synced := extprom.NewTxGaugeVec(nil, prometheus.GaugeOpts{Name: "synced"}, []string{"state"}) + + f := NewIgnoreParquetBlocksFilter(logger) + require.NoError(t, f.FilterWithBucketIndex(ctx, metas, idx, synced)) + + // All Parquet blocks are dropped from the TSDB metas; the plain TSDB block is kept. + assert.NotContains(t, metas, parquetInMetas, "parquet block must be dropped from the TSDB store") + assert.Contains(t, metas, plainTSDB, "non-parquet block must always be kept") + + // Only blocks actually present in metas increment the synced counter. + assert.Equal(t, 1.0, promtest.ToFloat64(synced.WithLabelValues(parquetConvertedMeta))) + + // ParquetBlockIDs records every Parquet block, even one that was never + // in the TSDB metas, and excludes non-parquet blocks. + parquetBlockIDs := f.ParquetBlockIDs() + assert.Contains(t, parquetBlockIDs, parquetInMetas.String()) + assert.Contains(t, parquetBlockIDs, parquetNotInMetas.String()) + assert.NotContains(t, parquetBlockIDs, plainTSDB.String()) + assert.Len(t, parquetBlockIDs, 2) +} diff --git a/pkg/storegateway/parquet_bucket_stores.go b/pkg/storegateway/parquet_bucket_stores.go index 9c6ac89bec5..9809eae3573 100644 --- a/pkg/storegateway/parquet_bucket_stores.go +++ b/pkg/storegateway/parquet_bucket_stores.go @@ -69,12 +69,19 @@ type ParquetBucketStores struct { indexLoader *bucketindex.Loader } -// newParquetBucketStores creates a new multi-tenant parquet bucket stores -func newParquetBucketStores(cfg tsdb.BlocksStorageConfig, bucketClient objstore.InstrumentedBucket, limits *validation.Overrides, logger log.Logger, reg prometheus.Registerer) (*ParquetBucketStores, error) { - // Create caching bucket client for parquet bucket stores - cachingBucket, err := createCachingBucketClientForParquet(cfg, bucketClient, "parquet-storegateway", logger, reg) - if err != nil { - return nil, err +// newParquetBucketStores creates a new multi-tenant parquet bucket stores. +func newParquetBucketStores(cfg tsdb.BlocksStorageConfig, bucketClient objstore.InstrumentedBucket, cachingBucket objstore.InstrumentedBucket, matcherCache storecache.MatchersCache, limits *validation.Overrides, logger log.Logger, reg prometheus.Registerer) (*ParquetBucketStores, error) { + var err error + if cachingBucket == nil { + // Create caching bucket client for parquet bucket stores + if cachingBucket, err = createCachingBucketClientForParquet(cfg, bucketClient, "parquet-storegateway", logger, reg); err != nil { + return nil, err + } + } + if matcherCache == nil { + if matcherCache, err = newMatchersCache(cfg, logger, reg); err != nil { + return nil, err + } } parquetShardCache, err := parquetutil.NewParquetShardCache[parquet_storage.ParquetShard](&cfg.BucketStore.ParquetShardCache, "parquet-shards", reg) @@ -102,17 +109,7 @@ func newParquetBucketStores(cfg tsdb.BlocksStorageConfig, bucketClient objstore. inflightRequests: cortex_util.NewInflightRequestTracker(), parquetShardCache: parquetShardCache, rowRangesCache: rowRangesCache, - } - - if cfg.BucketStore.MatchersCacheMaxItems > 0 { - r := prometheus.NewRegistry() - reg.MustRegister(tsdb.NewMatchCacheMetrics("cortex_storegateway", r, logger)) - u.matcherCache, err = storecache.NewMatchersCache(storecache.WithSize(cfg.BucketStore.MatchersCacheMaxItems), storecache.WithPromRegistry(r)) - if err != nil { - return nil, err - } - } else { - u.matcherCache = storecache.NoopMatchersCache + matcherCache: matcherCache, } if cfg.BucketStore.BucketIndex.Enabled { @@ -163,10 +160,12 @@ func (u *ParquetBucketStores) Series(req *storepb.SeriesRequest, srv storepb.Sto defer u.inflightRequests.Dec() } - return store.Series(req, spanSeriesServer{ + wrappedSrv := spanSeriesServer{ Store_SeriesServer: srv, ctx: spanCtx, - }) + } + + return store.Series(req, wrappedSrv) } // LabelNames implements BucketStores @@ -601,7 +600,7 @@ func chunkToStoreEncoding(in chunkenc.Encoding) storepb.Chunk_Encoding { } // createCachingBucketClientForParquet creates a caching bucket client for parquet bucket stores -func createCachingBucketClientForParquet(storageCfg tsdb.BlocksStorageConfig, bucketClient objstore.InstrumentedBucket, name string, logger log.Logger, reg prometheus.Registerer) (objstore.Bucket, error) { +func createCachingBucketClientForParquet(storageCfg tsdb.BlocksStorageConfig, bucketClient objstore.InstrumentedBucket, name string, logger log.Logger, reg prometheus.Registerer) (objstore.InstrumentedBucket, error) { // Create caching bucket using the existing infrastructure matchers := tsdb.NewMatchers() cachingBucket, err := tsdb.CreateCachingBucket(storageCfg.BucketStore.ChunksCache, storageCfg.BucketStore.MetadataCache, storageCfg.BucketStore.ParquetLabelsCache, matchers, bucketClient, logger, reg) diff --git a/pkg/storegateway/parquet_bucket_stores_test.go b/pkg/storegateway/parquet_bucket_stores_test.go index e7f3b8780c2..ff82f07e523 100644 --- a/pkg/storegateway/parquet_bucket_stores_test.go +++ b/pkg/storegateway/parquet_bucket_stores_test.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "errors" - "fmt" "os" "path/filepath" "strconv" @@ -276,7 +275,7 @@ func TestParquetBucketStoresWithCaching(t *testing.T) { limits := validation.NewOverrides(validation.Limits{}, nil) // Create parquet bucket stores with caching - parquetStores, err := newParquetBucketStores(storageCfg, bucketClient, limits, log.NewNopLogger(), prometheus.NewRegistry()) + parquetStores, err := newParquetBucketStores(storageCfg, bucketClient, nil, nil, limits, log.NewNopLogger(), prometheus.NewRegistry()) require.NoError(t, err) require.NotNil(t, parquetStores) require.NotNil(t, parquetStores.rowRangesCache) @@ -338,14 +337,15 @@ func TestCreateCachingBucketClientForParquet(t *testing.T) { func TestParquetBucketStores_Series_ShouldReturnErrorIfMaxInflightRequestIsReached(t *testing.T) { cfg := prepareStorageConfig(t) cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + cfg.BucketStore.ParquetFallbackDisabled = true cfg.BucketStore.MaxInflightRequests = 10 reg := prometheus.NewPedanticRegistry() storageDir := t.TempDir() generateStorageBlock(t, storageDir, "user_id", "series_1", 0, 100, 15) - bucket, err := filesystem.NewBucketClient(filesystem.Config{Directory: storageDir}) + bkt, err := filesystem.NewBucketClient(filesystem.Config{Directory: storageDir}) require.NoError(t, err) - stores, err := NewBucketStores(cfg, NewNoShardingStrategy(log.NewNopLogger(), nil), objstore.WithNoopInstr(bucket), defaultLimitsOverrides(t), mockLoggingLevel(), log.NewNopLogger(), reg) + stores, err := NewBucketStores(cfg, NewNoShardingStrategy(log.NewNopLogger(), nil), objstore.WithNoopInstr(bkt), defaultLimitsOverrides(t), mockLoggingLevel(), log.NewNopLogger(), reg) require.NoError(t, err) require.NoError(t, stores.InitialSync(context.Background())) @@ -363,10 +363,11 @@ func TestParquetBucketStores_Series_ShouldReturnErrorIfMaxInflightRequestIsReach func TestParquetBucketStores_Series_ShouldNotCheckMaxInflightRequestsIfTheLimitIsDisabled(t *testing.T) { cfg := prepareStorageConfig(t) cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + cfg.BucketStore.ParquetFallbackDisabled = true reg := prometheus.NewPedanticRegistry() storageDir := t.TempDir() - userId := "user_id" - generateStorageBlock(t, storageDir, userId, "series_1", 0, 100, 15) + userID := "user_id" + generateStorageBlock(t, storageDir, userID, "series_1", 0, 100, 15) bkt, err := filesystem.NewBucketClient(filesystem.Config{Directory: storageDir}) require.NoError(t, err) @@ -375,20 +376,16 @@ func TestParquetBucketStores_Series_ShouldNotCheckMaxInflightRequestsIfTheLimitI require.NoError(t, stores.InitialSync(context.Background())) parquetStores := stores.(*ParquetBucketStores) - // Set inflight requests to the limit (max_inflight_request is set to 0 by default = disabled)for range 10 { + // max_inflight_requests is 0 (disabled), so inflight requests must not be checked. for range 10 { parquetStores.inflightRequests.Inc() } - userPath := fmt.Sprintf("%s/%s", storageDir, userId) - - limits := validation.Limits{} - overrides := validation.NewOverrides(limits, nil) - uBucket := bucket.NewUserBucketClient(userId, bkt, overrides) - blockIds, err := convertToParquetBlocksForTesting(userPath, uBucket) + uBucket := bucket.NewUserBucketClient(userID, bkt, validation.NewOverrides(validation.Limits{}, nil)) + blockIDs, err := convertToParquetBlocksForTesting(filepath.Join(storageDir, userID), uBucket) require.NoError(t, err) - series, _, err := querySeries(stores, userId, "series_1", 0, 100, blockIds...) + series, _, err := querySeries(stores, userID, "series_1", 0, 100, blockIDs...) require.NoError(t, err) assert.Equal(t, 1, len(series)) } @@ -488,6 +485,7 @@ func TestParquetBucketStores_Series_MultiShard(t *testing.T) { cfg := prepareStorageConfig(t) cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + cfg.BucketStore.ParquetFallbackDisabled = true storageDir := t.TempDir() @@ -538,6 +536,7 @@ func TestParquetBucketStores_Series_MultiShard_BucketIndex(t *testing.T) { cfg := prepareStorageConfig(t) cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + cfg.BucketStore.ParquetFallbackDisabled = true cfg.BucketStore.BucketIndex.Enabled = true storageDir := t.TempDir() @@ -602,6 +601,7 @@ func TestParquetBucketStores_Series_MultiShard_BucketIndexStale_FallbackToConver cfg := prepareStorageConfig(t) cfg.BucketStore.BucketStoreType = string(cortex_tsdb.ParquetBucketStore) + cfg.BucketStore.ParquetFallbackDisabled = true cfg.BucketStore.BucketIndex.Enabled = true storageDir := t.TempDir() @@ -628,7 +628,7 @@ func TestParquetBucketStores_Series_MultiShard_BucketIndexStale_FallbackToConver require.NoError(t, err) require.Len(t, blockIDs, 1) - stores, err := NewBucketStores(cfg, NewNoShardingStrategy(log.NewNopLogger(), nil), objstore.WithNoopInstr(bkt), defaultLimitsOverrides(t), mockLoggingLevel(), log.NewNopLogger(), prometheus.NewRegistry()) + stores, err := newParquetBucketStores(cfg, objstore.WithNoopInstr(bkt), nil, nil, overrides, log.NewNopLogger(), prometheus.NewRegistry()) require.NoError(t, err) series, _, err := querySeries(stores, userID, metricName, 0, 100, blockIDs...) diff --git a/schemas/cortex-config-schema.json b/schemas/cortex-config-schema.json index b48a37271d3..c580e8dee1e 100644 --- a/schemas/cortex-config-schema.json +++ b/schemas/cortex-config-schema.json @@ -1189,7 +1189,7 @@ }, "bucket_store_type": { "default": "tsdb", - "description": "Type of bucket store to use (tsdb or parquet).", + "description": "Type of bucket store to use (tsdb or parquet). With parquet, blocks converted to Parquet are served from Parquet files and blocks not yet converted are served from TSDB blocks, unless -blocks-storage.bucket-store.parquet-fallback-disabled is set.", "type": "string", "x-cli-flag": "blocks-storage.bucket-store.bucket-store-type" }, @@ -2357,6 +2357,12 @@ }, "type": "object" }, + "parquet_fallback_disabled": { + "default": false, + "description": "[Experimental] Only used when the bucket store type is parquet. If true, the store-gateway serves only blocks converted to Parquet and does not fall back to TSDB blocks for blocks not converted yet, so it keeps no local state. If false, the bucket index must be enabled.", + "type": "boolean", + "x-cli-flag": "blocks-storage.bucket-store.parquet-fallback-disabled" + }, "parquet_labels_cache": { "properties": { "attributes_ttl": { From 16584d9ece9f580d44dae56b71d03af92b0f937b Mon Sep 17 00:00:00 2001 From: SungJin1212 Date: Fri, 23 Oct 2026 19:57:01 +0900 Subject: [PATCH 2/3] refactor: simplify method names in HybridBucketStores for clarity Signed-off-by: SungJin1212 --- pkg/storegateway/hybrid_bucket_stores.go | 20 ++++++++++---------- pkg/storegateway/parquet_bucket_stores.go | 6 ++---- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/pkg/storegateway/hybrid_bucket_stores.go b/pkg/storegateway/hybrid_bucket_stores.go index ca09d1cbc51..d8be66923fc 100644 --- a/pkg/storegateway/hybrid_bucket_stores.go +++ b/pkg/storegateway/hybrid_bucket_stores.go @@ -121,7 +121,7 @@ func (h *HybridBucketStores) Series(req *storepb.SeriesRequest, srv storepb.Stor Store_SeriesServer: srv, ctx: spanCtx, } - return h.seriesWithTSDBStore(spanCtx, userID, req, store, wrappedSrv) + return h.series(spanCtx, userID, req, store, wrappedSrv) } // LabelNames implements BucketStores. @@ -144,7 +144,7 @@ func (h *HybridBucketStores) LabelNames(ctx context.Context, req *storepb.LabelN return nil, status.Error(codes.Internal, err.Error()) } - return h.labelNamesWithTSDBStore(spanCtx, userID, req, store) + return h.labelNames(spanCtx, userID, req, store) } // LabelValues implements BucketStores. @@ -167,7 +167,7 @@ func (h *HybridBucketStores) LabelValues(ctx context.Context, req *storepb.Label return nil, status.Error(codes.Internal, err.Error()) } - return h.labelValuesWithTSDBStore(spanCtx, userID, req, store) + return h.labelValues(spanCtx, userID, req, store) } func (h *HybridBucketStores) checkStoreError(userID string) error { @@ -287,9 +287,9 @@ func labelValuesRequestForBlocks(req *storepb.LabelValuesRequest, hints *hintspb return &r, nil } -// seriesWithTSDBStore serves a Series request by splitting the requested blocks between the -// Parquet store and the TSDB store, then merging the (sorted) results. -func (h *HybridBucketStores) seriesWithTSDBStore(ctx context.Context, userID string, req *storepb.SeriesRequest, store *parquetBucketStore, srv storepb.Store_SeriesServer) error { +// series serves a Series request by splitting the requested blocks between the Parquet store and +// the TSDB store, then merging the (sorted) results when both are involved. +func (h *HybridBucketStores) series(ctx context.Context, userID string, req *storepb.SeriesRequest, store *parquetBucketStore, srv storepb.Store_SeriesServer) error { reqHints := &hintspb.SeriesRequestHints{} if req.Hints != nil { if err := types.UnmarshalAny(req.Hints, reqHints); err != nil { @@ -402,8 +402,8 @@ func (h *HybridBucketStores) seriesWithTSDBStore(ctx context.Context, userID str return outSrv.Flush() } -// labelNamesWithTSDBStore serves a LabelNames request across the Parquet store and the TSDB store. -func (h *HybridBucketStores) labelNamesWithTSDBStore(ctx context.Context, userID string, req *storepb.LabelNamesRequest, store *parquetBucketStore) (*storepb.LabelNamesResponse, error) { +// labelNames serves a LabelNames request across the Parquet store and the TSDB store. +func (h *HybridBucketStores) labelNames(ctx context.Context, userID string, req *storepb.LabelNamesRequest, store *parquetBucketStore) (*storepb.LabelNamesResponse, error) { reqHints := &hintspb.LabelNamesRequestHints{} if req.Hints != nil { if err := types.UnmarshalAny(req.Hints, reqHints); err != nil { @@ -471,8 +471,8 @@ func mergeLabelNamesHints(a, b *storepb.LabelNamesResponse) (*types.Any, error) return types.MarshalAny(merged) } -// labelValuesWithTSDBStore serves a LabelValues request across the Parquet store and the TSDB store. -func (h *HybridBucketStores) labelValuesWithTSDBStore(ctx context.Context, userID string, req *storepb.LabelValuesRequest, store *parquetBucketStore) (*storepb.LabelValuesResponse, error) { +// labelValues serves a LabelValues request across the Parquet store and the TSDB store. +func (h *HybridBucketStores) labelValues(ctx context.Context, userID string, req *storepb.LabelValuesRequest, store *parquetBucketStore) (*storepb.LabelValuesResponse, error) { reqHints := &hintspb.LabelValuesRequestHints{} if req.Hints != nil { if err := types.UnmarshalAny(req.Hints, reqHints); err != nil { diff --git a/pkg/storegateway/parquet_bucket_stores.go b/pkg/storegateway/parquet_bucket_stores.go index 9809eae3573..9b3527ffa10 100644 --- a/pkg/storegateway/parquet_bucket_stores.go +++ b/pkg/storegateway/parquet_bucket_stores.go @@ -160,12 +160,10 @@ func (u *ParquetBucketStores) Series(req *storepb.SeriesRequest, srv storepb.Sto defer u.inflightRequests.Dec() } - wrappedSrv := spanSeriesServer{ + return store.Series(req, spanSeriesServer{ Store_SeriesServer: srv, ctx: spanCtx, - } - - return store.Series(req, wrappedSrv) + }) } // LabelNames implements BucketStores From f093cbf3627057fe073d236f16f24500871bc748 Mon Sep 17 00:00:00 2001 From: SungJin1212 Date: Fri, 23 Oct 2026 20:43:35 +0900 Subject: [PATCH 3/3] fix test Signed-off-by: SungJin1212 --- integration/querier_microservices_mode_test.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/integration/querier_microservices_mode_test.go b/integration/querier_microservices_mode_test.go index 4f9a345b40b..49a24922c29 100644 --- a/integration/querier_microservices_mode_test.go +++ b/integration/querier_microservices_mode_test.go @@ -200,6 +200,8 @@ func TestQuerierWithBlocksStorageRunningInMicroservicesMode(t *testing.T) { flags["-parquet-converter.ring.consul.hostname"] = consul.NetworkHTTPEndpoint() flags["-compactor.block-ranges"] = "1ms,12h" // to convert all blocks to parquet blocks flags["-blocks-storage.bucket-store.parquet-labels-cache.backend"] = testCfg.parquetLabelsCache + // Refresh the bucket index frequently so it records the converted blocks. + flags["-compactor.cleanup-interval"] = "1s" } // Add the cache address to the flags. @@ -322,12 +324,16 @@ func TestQuerierWithBlocksStorageRunningInMicroservicesMode(t *testing.T) { // Wait until the parquet-converter convert blocks require.NoError(t, parquetConverter.WaitSumMetricsWithOptions(e2e.Equals(float64(2)), []string{"cortex_parquet_converter_blocks_converted_total"}, e2e.WaitMissingMetrics)) - // Wait until the compactor's blocks cleaner has completed at least - // one pass, which creates the bucket index. The parquet - // store-gateway discovers blocks on-demand through the bucket index - // rather than pre-loading them, so the bucket index must exist - // before querying. - require.NoError(t, compactor.WaitSumMetricsWithOptions(e2e.Greater(0), []string{"cortex_compactor_block_cleanup_completed_total"}, e2e.WaitMissingMetrics)) + // Wait until the store-gateways have synced the bucket index recording both blocks as + // converted, so they are routed to the Parquet store. Until then, the hybrid + // store-gateway routes them to the TSDB sub-store, which has not loaded them either. + if testCfg.blocksShardingStrategy != "" { + require.NoError(t, storeGateways.WaitSumMetricsWithOptions(e2e.Equals(2), []string{"cortex_blocks_meta_synced"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "state", "parquet-converted")), e2e.SkipMissingMetrics)) + } else { + require.NoError(t, storeGateways.WaitSumMetricsWithOptions(e2e.Equals(float64(2*storeGateways.NumInstances())), []string{"cortex_blocks_meta_synced"}, + e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "state", "parquet-converted")), e2e.WaitMissingMetrics)) + } } // Query back the series (1 only in the storage, 1 only in the ingesters, 1 on both).