Skip to content

Parquet Store gateway #7046

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Parquet Store gateway #7046

wants to merge 14 commits into from

Conversation

@SungJin1212
Copy link
Member

@SungJin1212 SungJin1212 commented Oct 10, 2025 •
edited
Loading

This PR introduces a new parquet mode for the Store Gateway. In this initial version, the mode operates statelessly and does not perform operations such as block syncing. The primary improvement is to enhance cache utilization by leveraging the Store Gateway's block affinity.

Which issue(s) this PR fixes:
Fixes #6940

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@SungJin1212 SungJin1212 marked this pull request as draft October 10, 2025 09:46
@SungJin1212 SungJin1212 force-pushed the parquet-gateway-poc branch 7 times, most recently from 27d4ca3 to 7ab5bc7 Compare October 15, 2025 06:44
@SungJin1212 SungJin1212 marked this pull request as ready for review October 15, 2025 09:40
yeya24
yeya24 reviewed Oct 19, 2025
View reviewed changes
docs/blocks-storage/store-gateway.md Outdated
# Minimum time to wait for ring stability at startup. 0 to disable.
# CLI flag: -store-gateway.sharding-ring.wait-stability-min-duration
[wait_stability_min_duration: <duration> | default = 1m]
[wait_stability_min_duration: <duration> | default = 0s]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change should be removed

docs/blocks-storage/store-gateway.md Outdated
# start anyway.
# CLI flag: -store-gateway.sharding-ring.wait-stability-max-duration
[wait_stability_max_duration: <duration> | default = 5m]
[wait_stability_max_duration: <duration> | default = 5s]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change should be removed

pkg/storegateway/parquet_bucket_stores_test.go Outdated
assert.Empty(t, warnings)
}

//func TestParquetBucketStores_Series_ShouldNotCheckMaxInflightRequestsIfTheLimitIsDisabled(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to keep this

pkg/storegateway/parquet_bucket_stores_test.go
assert.Equal(t, codes.InvalidArgument, s.Code())
}

func TestChunkToStoreEncoding(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test seems irrelevant?

integration/parquet_gateway_test.go Outdated
cortex_testutil "github.com/cortexproject/cortex/pkg/util/test"
)

func TestParquetGatewayWithBlocksStorageRunningInMicroservicesMode(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer reusing the existing store gateway integration test instead of creating a new parquet gateway test. This adds a lot of duplicated code

@SungJin1212
Copy link
Member Author

@yeya24
I added some commits; the quay.io seems to be in the ' read-only` state temporarily.

@yeya24
Copy link
Contributor

yeya24 commented Oct 23, 2025

Can u please update the PR title and description?

@SungJin1212 SungJin1212 changed the title Parquet gateway poc Parquet Store gateway Oct 23, 2025
yeya24 and others added 11 commits October 24, 2025 10:54
@yeya24 @SungJin1212
parquet store gateway poc
9fe669e 
Signed-off-by: yeya24 <[email protected]>
@SungJin1212
refactor and fix test
945f5d6 
Signed-off-by: SungJin1212 <[email protected]>
@SungJin1212
fix test
1c68550 
Signed-off-by: SungJin1212 <[email protected]>
@SungJin1212
delete unused code
bc221e7 
Signed-off-by: SungJin1212 <[email protected]>
@SungJin1212
add spans
a3f4a5f 
Signed-off-by: SungJin1212 <[email protected]>
@SungJin1212
rollback InitialSync
8fa6164 
Signed-off-by: SungJin1212 <[email protected]>
@SungJin1212
Rollback wait stability max/min duration
b025cc0 
Signed-off-by: SungJin1212 <[email protected]>
@SungJin1212
Add inflight request limit not checked test
7485f94 
Signed-off-by: SungJin1212 <[email protected]>
@SungJin1212
integate parquet bucket storage test with existing test
5e5cc43 
Signed-off-by: SungJin1212 <[email protected]>
@SungJin1212
fix lint
374ef95 
Signed-off-by: SungJin1212 <[email protected]>
@SungJin1212
fix lint
59abb2c 
Signed-off-by: SungJin1212 <[email protected]>
@SungJin1212
add changelog
553e4ea 
Signed-off-by: SungJin1212 <[email protected]>
yeya24
yeya24 reviewed Oct 24, 2025
View reviewed changes
CHANGELOG.md Outdated
* [FEATURE] Query Frontend: Add support /api/v1/format_query API for formatting queries. #6893
* [FEATURE] Query Frontend: Add support for /api/v1/parse_query API (experimental) to parse a PromQL expression and return it as a JSON-formatted AST (abstract syntax tree). #6978
* [FEATURE] StoreGateway: Introduces a new parquet mode. #7046
* [FEATURE] StoreGateway: Introduces a new parquet mode. #7046
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated entries here

pkg/storegateway/bucket_store_metrics.go Outdated
"github.com/cortexproject/cortex/pkg/util"
)

// CortexBucketStoreMetrics common metrics in thanos and parquet block stores (in future)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if parquet store gateway really needs to sync blocks and discover tenants. Do we need to extract those metrics in this PR?

I would prefer to keep those metrics in the legacy store gateway today

pkg/storegateway/parquet_bucket_store.go
bucketOpener := parquet_storage.NewParquetBucketOpener(p.bucket)
noopQuota := search.NewQuota(search.NoopQuotaLimitFunc(ctx))
for _, blockID := range blockIDs {
block, err := p.newParquetBlock(ctx, blockID, bucketOpener, bucketOpener, p.chunksDecoder, noopQuota, noopQuota, noopQuota)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it mean we don't apply any limits from parquet library today? But we still apply the existing store gateway limits, correct

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to add new flags?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add new flags for later if needed.

@SungJin1212
fix changelog
2d1a1dd 
Signed-off-by: SungJin1212 <[email protected]>
@SungJin1212
rollback bucket store metrics
b3aa4e4 
Signed-off-by: SungJin1212 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yeya24 yeya24 yeya24 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

component/store-gateway size/XXL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support Parquet Store Gateway

2 participants

@SungJin1212 @yeya24