Skip to content
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.

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

fix: query plan optimisation #3100

Merged
merged 2 commits into from
Mar 15, 2024
Merged

fix: query plan optimisation #3100

merged 2 commits into from
Mar 15, 2024

Conversation

kolesnikovae
Copy link
Collaborator

@kolesnikovae kolesnikovae commented Mar 14, 2024

Fixes #3088.

The problem manifests when compactor does not keep up with the ingestion rate, and store-gateways load sharded L2 blocks (with duplicates).

Normally, L3 blocks supersede all the parent blocks in the query plan, and presence of L2 blocks does not pose any issues. However, when an L3 shard is incomplete, all its blocks are removed from the plan before this step, preserving L2 shards in the query plan (partial shards are ignored in the plan). This happens, when L3 compaction delay exceeds blocks-storage.bucket-store.ignore-blocks-within. The way query plan is built expects that L2 blocks do not contain duplicate series, which is not true if blocks are shared. As a result, query returns duplicated data.

Comment on lines +293 to 297
// TODO(kolesnikovae):
// Expiry defaults to -querier.query-store-after which should be deprecated,
// blocks-storage.bucket-store.ignore-blocks-within can be used instead.
expiryTs := time.Now().Add(-dc.policy.Expiry)
return block.Uploaded && ulid.Time(block.ID.Time()).Before(expiryTs)
Copy link
Collaborator Author

@kolesnikovae kolesnikovae Mar 14, 2024

Choose a reason for hiding this comment

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

However, we also need to take into account that a new block does not become available to the store gateway immediately. I'd say that it probably makes sense to delay till the block max time is covered by blocks-storage.bucket-store.ignore-blocks-within + (2 * block-sync-interval)

@kolesnikovae kolesnikovae marked this pull request as ready for review March 14, 2024 06:49
@kolesnikovae kolesnikovae requested a review from a team as a code owner March 14, 2024 06:49
Copy link
Contributor

@aleks-p aleks-p left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this, looks good to me!

@kolesnikovae kolesnikovae merged commit 26b2735 into main Mar 15, 2024
16 checks passed
@kolesnikovae kolesnikovae deleted the fix/overlap branch March 15, 2024 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ingester and store-gateway time ranges overlap
2 participants