Stream chunks from storegateway to queriers #5182

codesome · 2023-06-06T15:00:10Z

What this PR does

Streams chunks from store gateways to querier.

Store gateway first fetches the series from the storage (only series labels now; the chunk refs are skipped). After sending the series labels to the queriers, store gateway now starts sending the chunks for the series in the same order the series were sent.

In both case, when sending series and chunks, the series are batched together to save on CPU. The max batch size is configurable.

Additional proto fields have been added specifically for this.

Which issue(s) this PR fixes or relates to

Fixes #TODO

Checklist

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

Signed-off-by: Ganesh Vernekar <[email protected]>

charleskorn

Noticed a couple of small things as I was reading through this.

pkg/querier/blocks_store_queryable.go

pkg/storegateway/bucket.go

pkg/querier/blocks_store_querable_streaming.go

Signed-off-by: Ganesh Vernekar <[email protected]>

dimitarvdimitrov

I left a few comments, most of the are around code structure, but there are a few that I think need addressing before we can merge this. Will you take a look?

CHANGELOG.md

pkg/storegateway/bucket.go

pkg/storegateway/series_refs_test.go

pkg/storegateway/stats.go

pkg/storegateway/series_refs_test.go

pkg/querier/streaming_readers.go

Signed-off-by: Ganesh Vernekar <[email protected]>

dimitarvdimitrov

Massive work! Thanks for addressing my comments. I checked the diff since my last review and the changes LGTM.

There are still two points about metrics

the batch load/wait durations that are erroneously reported
the cache lookups, which don't seem deterministic. The assertions on cache lookups in testQuerierWithBlocksStorageRunningInMicroservicesMode become much weaker if we use the GreaterThan instead of Equal. I'd like to understand why they are non-deterministic before merging this as-is.

Signed-off-by: Ganesh Vernekar <[email protected]>

codesome · 2023-07-06T12:07:39Z

the batch load/wait durations that are erroneously reported

This should be fixed now in the last commit

charleskorn

This is looking good, I've done one last pass over the changes to the querier.

CHANGELOG.md

pkg/mimir/modules.go

pkg/querier/block_streaming.go

pkg/querier/querier.go

pkg/querier/blocks_store_queryable.go

Signed-off-by: Ganesh Vernekar <[email protected]>

…5459) * Add test for polluted series hash cache with streaming Signed-off-by: Dimitar Dimitrov <[email protected]> * Fix series hash cache corruption Signed-off-by: Dimitar Dimitrov <[email protected]> * Update pkg/storegateway/series_refs_test.go --------- Signed-off-by: Dimitar Dimitrov <[email protected]> Co-authored-by: Ganesh Vernekar <[email protected]>

Signed-off-by: Ganesh Vernekar <[email protected]>

charleskorn

LGTM modulo one last comment

pkg/querier/block_streaming.go

Signed-off-by: Ganesh Vernekar <[email protected]>

codesome added 2 commits June 6, 2023 20:16

Update protos for streaming

fa4e1c9

Signed-off-by: Ganesh Vernekar <[email protected]>

Send streaming chunks from store-gateway

1d8f18b

Signed-off-by: Ganesh Vernekar <[email protected]>

codesome force-pushed the codesome/stream-store branch from e958e8a to 1d8f18b Compare June 6, 2023 15:09

codesome added 3 commits June 7, 2023 16:00

Reuse postings

d8a29b7

Signed-off-by: Ganesh Vernekar <[email protected]>

Read streaming chunks from storegateway in queriers

8488231

Signed-off-by: Ganesh Vernekar <[email protected]>

Fix lint

e2d189a

Signed-off-by: Ganesh Vernekar <[email protected]>

codesome force-pushed the codesome/stream-store branch from d7464f9 to e2d189a Compare June 8, 2023 12:40

codesome added 3 commits June 8, 2023 19:01

Test if streaming works by making it default

8bc1d12

Signed-off-by: Ganesh Vernekar <[email protected]>

Do not ignore IsEndOfSeriesStream

40f4753

Signed-off-by: Ganesh Vernekar <[email protected]>

Remove the default streaming

c321c2e

Signed-off-by: Ganesh Vernekar <[email protected]>

charleskorn reviewed Jun 9, 2023

View reviewed changes

pkg/querier/blocks_store_queryable.go Show resolved Hide resolved

pkg/storegateway/bucket.go Show resolved Hide resolved

pkg/storegateway/bucket.go Outdated Show resolved Hide resolved

pkg/querier/blocks_store_querable_streaming.go Outdated Show resolved Hide resolved

codesome added 3 commits June 9, 2023 16:35

Extend testBucketStore_e2e to test streaming

32d4bce

Signed-off-by: Ganesh Vernekar <[email protected]>

Merge branch 'main' into codesome/stream-store

ca6ea7c

Fix the case of wrong series in the 1st pass of streaming

3054be1

Signed-off-by: Ganesh Vernekar <[email protected]>

codesome force-pushed the codesome/stream-store branch 2 times, most recently from 9d13f2f to 3054be1 Compare June 12, 2023 07:28

Use the streaming config for storegateway

42c4e08

Signed-off-by: Ganesh Vernekar <[email protected]>

codesome force-pushed the codesome/stream-store branch from cc125c3 to 42c4e08 Compare June 12, 2023 07:55

codesome added 12 commits June 12, 2023 14:58

Updated unit tests for storegateway

52d9c04

Signed-off-by: Ganesh Vernekar <[email protected]>

Send hints and stats back in right order. Unit tests in queriers.

254b4e2

Signed-off-by: Ganesh Vernekar <[email protected]>

Use streaming in TestBlocksStoreQuerier_PromQLExecution

0ca67b9

Signed-off-by: Ganesh Vernekar <[email protected]>

Lint

deafaa5

Signed-off-by: Ganesh Vernekar <[email protected]>

Fix tests

6ea14a4

Signed-off-by: Ganesh Vernekar <[email protected]>

lint

05b9283

Signed-off-by: Ganesh Vernekar <[email protected]>

Actually pass down streaming batch size from querier to storegateway

719b254

Signed-off-by: Ganesh Vernekar <[email protected]>

Add unit tests. Quick self review.

df0a7ea

Signed-off-by: Ganesh Vernekar <[email protected]>

Refactor Series function to make it smaller

0f8c53c

Signed-off-by: Ganesh Vernekar <[email protected]>

Fix lint

513c99b

Signed-off-by: Ganesh Vernekar <[email protected]>

Refactor to send of batches of chunks at a time

766e46f

Signed-off-by: Ganesh Vernekar <[email protected]>

lint

725f759

Signed-off-by: Ganesh Vernekar <[email protected]>

codesome added 10 commits June 30, 2023 18:29

Fix 'cannot reverse reader' bug with more test coverage

4333c6f

Signed-off-by: Ganesh Vernekar <[email protected]>

Merge branch 'main' into codesome/stream-store

8a998b0

Signed-off-by: Ganesh Vernekar <[email protected]>

Fix review comments

9343e31

Signed-off-by: Ganesh Vernekar <[email protected]>

Fix integration tests

92d30a4

Signed-off-by: Ganesh Vernekar <[email protected]>

Add changelog entry

63c3b59

Signed-off-by: Ganesh Vernekar <[email protected]>

Integration tests

a404446

Signed-off-by: Ganesh Vernekar <[email protected]>

Merge branch 'main' into codesome/stream-store

774b0b0

Move and rename stream reader for store gateway

8903d2a

Signed-off-by: Ganesh Vernekar <[email protected]>

Merge branch 'main' into codesome/stream-store

edc87fc

Signed-off-by: Ganesh Vernekar <[email protected]>

Fix StoreGatewayStreamReader and add tests for it

a545503

Signed-off-by: Ganesh Vernekar <[email protected]>

dimitarvdimitrov reviewed Jul 5, 2023

View reviewed changes

codesome added 3 commits July 5, 2023 17:42

Fix comments

e78fc3d

Signed-off-by: Ganesh Vernekar <[email protected]>

Fix review comments part 1

2c5ee20

Signed-off-by: Ganesh Vernekar <[email protected]>

Fix review comments part 2

3002e83

Signed-off-by: Ganesh Vernekar <[email protected]>

codesome force-pushed the codesome/stream-store branch from c2139d0 to 3002e83 Compare July 6, 2023 07:52

dimitarvdimitrov approved these changes Jul 6, 2023

View reviewed changes

Fix preloading metrics

e16bfb2

Signed-off-by: Ganesh Vernekar <[email protected]>

charleskorn reviewed Jul 7, 2023

View reviewed changes

codesome and others added 5 commits July 7, 2023 14:09

Fix logs

7ebf5c0

Signed-off-by: Ganesh Vernekar <[email protected]>

Merge branch 'main' into codesome/stream-store

e4284a1

Signed-off-by: Ganesh Vernekar <[email protected]>

Fix comments

6ed86ac

Signed-off-by: Ganesh Vernekar <[email protected]>

Merge branch 'main' into codesome/stream-store

052dd36

Signed-off-by: Ganesh Vernekar <[email protected]>

charleskorn approved these changes Jul 11, 2023

View reviewed changes

pkg/querier/block_streaming.go Outdated Show resolved Hide resolved

Move initialisation of seriesChunksChan

020248a

Signed-off-by: Ganesh Vernekar <[email protected]>

dimitarvdimitrov mentioned this pull request Jul 14, 2023

store-gateway: wait for query gate after loading blocks #5507

Merged

charleskorn merged commit c8a9ed7 into main Jul 18, 2023

charleskorn deleted the codesome/stream-store branch July 18, 2023 06:45

eyenx mentioned this pull request May 28, 2024

chore(mimir): update to 5.3.0 adfinis/helm-charts#1256

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stream chunks from storegateway to queriers #5182

Stream chunks from storegateway to queriers #5182

codesome commented Jun 6, 2023 •

edited

Loading

charleskorn left a comment

dimitarvdimitrov left a comment

dimitarvdimitrov left a comment

codesome commented Jul 6, 2023

charleskorn left a comment

charleskorn left a comment

Stream chunks from storegateway to queriers #5182

Stream chunks from storegateway to queriers #5182

Conversation

codesome commented Jun 6, 2023 • edited Loading

What this PR does

Which issue(s) this PR fixes or relates to

Checklist

charleskorn left a comment

Choose a reason for hiding this comment

dimitarvdimitrov left a comment

Choose a reason for hiding this comment

dimitarvdimitrov left a comment

Choose a reason for hiding this comment

codesome commented Jul 6, 2023

charleskorn left a comment

Choose a reason for hiding this comment

charleskorn left a comment

Choose a reason for hiding this comment

codesome commented Jun 6, 2023 •

edited

Loading