Store-gateway querier streaming: prevent series hash cache pollution #5459
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Dimitar Dimitrov <[email protected]>
Signed-off-by: Dimitar Dimitrov <[email protected]>
codesome
approved these changes
Jul 10, 2023
charleskorn
pushed a commit
that referenced
this pull request
Jul 18, 2023
* Update protos for streaming Signed-off-by: Ganesh Vernekar <[email protected]> * Send streaming chunks from store-gateway Signed-off-by: Ganesh Vernekar <[email protected]> * Reuse postings Signed-off-by: Ganesh Vernekar <[email protected]> * Read streaming chunks from storegateway in queriers Signed-off-by: Ganesh Vernekar <[email protected]> * Fix lint Signed-off-by: Ganesh Vernekar <[email protected]> * Test if streaming works by making it default Signed-off-by: Ganesh Vernekar <[email protected]> * Do not ignore IsEndOfSeriesStream Signed-off-by: Ganesh Vernekar <[email protected]> * Remove the default streaming Signed-off-by: Ganesh Vernekar <[email protected]> * Extend testBucketStore_e2e to test streaming Signed-off-by: Ganesh Vernekar <[email protected]> * Fix the case of wrong series in the 1st pass of streaming Signed-off-by: Ganesh Vernekar <[email protected]> * Use the streaming config for storegateway Signed-off-by: Ganesh Vernekar <[email protected]> * Updated unit tests for storegateway Signed-off-by: Ganesh Vernekar <[email protected]> * Send hints and stats back in right order. Unit tests in queriers. Signed-off-by: Ganesh Vernekar <[email protected]> * Use streaming in TestBlocksStoreQuerier_PromQLExecution Signed-off-by: Ganesh Vernekar <[email protected]> * Lint Signed-off-by: Ganesh Vernekar <[email protected]> * Fix tests Signed-off-by: Ganesh Vernekar <[email protected]> * lint Signed-off-by: Ganesh Vernekar <[email protected]> * Actually pass down streaming batch size from querier to storegateway Signed-off-by: Ganesh Vernekar <[email protected]> * Add unit tests. Quick self review. Signed-off-by: Ganesh Vernekar <[email protected]> * Refactor Series function to make it smaller Signed-off-by: Ganesh Vernekar <[email protected]> * Fix lint Signed-off-by: Ganesh Vernekar <[email protected]> * Refactor to send of batches of chunks at a time Signed-off-by: Ganesh Vernekar <[email protected]> * lint Signed-off-by: Ganesh Vernekar <[email protected]> * goroutine leak Signed-off-by: Ganesh Vernekar <[email protected]> * Fix most of Charles' comments Signed-off-by: Ganesh Vernekar <[email protected]> * Fix comments, fix stats, extend tests Signed-off-by: Ganesh Vernekar <[email protected]> * Improve tests to run streaming and non-streaming in any order Signed-off-by: Ganesh Vernekar <[email protected]> * Integration test Signed-off-by: Ganesh Vernekar <[email protected]> * Fix race Signed-off-by: Ganesh Vernekar <[email protected]> * Integration test attempt 0 Signed-off-by: Ganesh Vernekar <[email protected]> * Attempt Signed-off-by: Ganesh Vernekar <[email protected]> * Another attempt Signed-off-by: Ganesh Vernekar <[email protected]> * context.Background() Signed-off-by: Ganesh Vernekar <[email protected]> * q.ctx in queryWithConsistencyCheck Signed-off-by: Ganesh Vernekar <[email protected]> * rollback some things Signed-off-by: Ganesh Vernekar <[email protected]> * Update integration test metrics with TODO Signed-off-by: Ganesh Vernekar <[email protected]> * Fix a bunch of comments Signed-off-by: Ganesh Vernekar <[email protected]> * Add seriesIteratorStrategy Signed-off-by: Ganesh Vernekar <[email protected]> * Lint and stuff Signed-off-by: Ganesh Vernekar <[email protected]> * Fix goroutine leak Signed-off-by: Ganesh Vernekar <[email protected]> * Fix a race Signed-off-by: Ganesh Vernekar <[email protected]> * Fix unit test panic Signed-off-by: Ganesh Vernekar <[email protected]> * Fix the use of seriesIteratorStrategy Signed-off-by: Ganesh Vernekar <[email protected]> * Take care of tracing spans Signed-off-by: Ganesh Vernekar <[email protected]> * Fix metrics in integration tests Signed-off-by: Ganesh Vernekar <[email protected]> * Fix flags Signed-off-by: Ganesh Vernekar <[email protected]> * Fix review comments Signed-off-by: Ganesh Vernekar <[email protected]> * Fix more comments Signed-off-by: Ganesh Vernekar <[email protected]> * Optimise streaming of chunks Signed-off-by: Ganesh Vernekar <[email protected]> * Fix tests Signed-off-by: Ganesh Vernekar <[email protected]> * Fix limits calculation in storegateway Signed-off-by: Ganesh Vernekar <[email protected]> * Increase test coverage Signed-off-by: Ganesh Vernekar <[email protected]> * Fix tests Signed-off-by: Ganesh Vernekar <[email protected]> * Fox integration test metrics Signed-off-by: Ganesh Vernekar <[email protected]> * Fix tests Signed-off-by: Ganesh Vernekar <[email protected]> * Fix review comments Signed-off-by: Ganesh Vernekar <[email protected]> * Fix integration tests Signed-off-by: Ganesh Vernekar <[email protected]> * Split streamingSeriesSetForBlocks Signed-off-by: Ganesh Vernekar <[email protected]> * lint Signed-off-by: Ganesh Vernekar <[email protected]> * lint and test Signed-off-by: Ganesh Vernekar <[email protected]> * Fix 'cannot reverse reader' bug with more test coverage Signed-off-by: Ganesh Vernekar <[email protected]> * Fix review comments Signed-off-by: Ganesh Vernekar <[email protected]> * Fix integration tests Signed-off-by: Ganesh Vernekar <[email protected]> * Add changelog entry Signed-off-by: Ganesh Vernekar <[email protected]> * Integration tests Signed-off-by: Ganesh Vernekar <[email protected]> * Move and rename stream reader for store gateway Signed-off-by: Ganesh Vernekar <[email protected]> * Fix StoreGatewayStreamReader and add tests for it Signed-off-by: Ganesh Vernekar <[email protected]> * Fix comments Signed-off-by: Ganesh Vernekar <[email protected]> * Fix review comments part 1 Signed-off-by: Ganesh Vernekar <[email protected]> * Fix review comments part 2 Signed-off-by: Ganesh Vernekar <[email protected]> * Fix preloading metrics Signed-off-by: Ganesh Vernekar <[email protected]> * Fix logs Signed-off-by: Ganesh Vernekar <[email protected]> * Fix comments Signed-off-by: Ganesh Vernekar <[email protected]> * Store-gateway querier streaming: prevent series hash cache pollution (#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]> * Move initialisation of seriesChunksChan Signed-off-by: Ganesh Vernekar <[email protected]> --------- Signed-off-by: Ganesh Vernekar <[email protected]> Signed-off-by: Dimitar Dimitrov <[email protected]> Co-authored-by: Dimitar Dimitrov <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a bug where the
loadingSeriesChunkRefsSetIteratorwould store a wrong series hash cache for a series. This happens becausefitlerSeriesassumes that the number of postings is the same as the number of series. It uses the postings to store the series hash in the series hash cache.However, some series might be filtered out because they don't contain chunks within the time range. In this case the postings would not align with the series, so storing the hash would happen under the incorrect posting.