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.

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

Let store-gateways ignore blocks that are too young. #502

Merged
merged 9 commits into from
Nov 18, 2021
Merged

Let store-gateways ignore blocks that are too young. #502

merged 9 commits into from
Nov 18, 2021

Conversation

pstibrany
Copy link
Member

@pstibrany pstibrany commented Nov 16, 2021 •
edited
Loading

What this PR does: This PR adds a configurable filter to store-gateways to avoid loading blocks that are too young. Useful when used in combination with -querier.query-store-after option.

Fixes #365.

Checklist

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

@pstibrany pstibrany marked this pull request as draft November 16, 2021 16:00
@pstibrany
Copy link
Member Author

pstibrany commented Nov 16, 2021 •
edited
Loading

TODO: I will add another test that covers use of new filter from bucket stores. Also there is no need to make the filter public.

Update: Done.

@pstibrany
Renamed TimeMetaFilter to minTimeMetaFilter, added it to TestBucketIn…
b359ef8 
…dexMetadataFetcher_Fetch test.

Signed-off-by: Peter Štibraný <[email protected]>
@pstibrany pstibrany marked this pull request as ready for review November 17, 2021 14:16
@pstibrany
Unexport NewMinTimeMetaFilter.
950b576 
Signed-off-by: Peter Štibraný <[email protected]>
stevesg
stevesg reviewed Nov 18, 2021
View reviewed changes
Copy link
Contributor

@stevesg stevesg left a comment

Choose a reason for hiding this comment

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

The timestamp checking logic looks good to me. I've just asked a couple of questions to help my own understanding.

pkg/storage/tsdb/config.go Outdated Show resolved Hide resolved
pkg/storegateway/metadata_fetcher_filters.go Outdated Show resolved Hide resolved
@pstibrany
Enhanced field description.
8bad8ca 
Signed-off-by: Peter Štibraný <[email protected]>
aknuds1
aknuds1 reviewed Nov 18, 2021
View reviewed changes
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

Just suggesting a language improvement :)

docs/configuration/config-file-reference.md Outdated Show resolved Hide resolved
docs/blocks-storage/store-gateway.md Outdated Show resolved Hide resolved
docs/blocks-storage/querier.md Outdated Show resolved Hide resolved
pkg/storage/tsdb/config.go Outdated Show resolved Hide resolved
stevesg
stevesg approved these changes Nov 18, 2021
View reviewed changes
Copy link
Contributor

@stevesg stevesg left a comment

Choose a reason for hiding this comment

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

Looks good assuming the language fix-ups @aknuds1 suggested. Should be a very useful optimization as I understand it.

@pstibrany
Fix wording as suggested by Arve.
019fe8f 
Signed-off-by: Peter Štibraný <[email protected]>
pracucci
pracucci approved these changes Nov 18, 2021
View reviewed changes
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Good job! I left a couple of nits and a comment about the synced metric label value. A part from this, LGTM 👏

pkg/storegateway/bucket_stores.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_stores.go Outdated Show resolved Hide resolved
pkg/storegateway/metadata_fetcher_filters.go Outdated Show resolved Hide resolved
@pstibrany
Address review feedback.
93e3b08 
Signed-off-by: Peter Štibraný <[email protected]>
@pstibrany
Address review feedback.
43735fa 
Signed-off-by: Peter Štibraný <[email protected]>
@pstibrany
Fix tests after changing label value.
e55890f 
Signed-off-by: Peter Štibraný <[email protected]>
pracucci
pracucci approved these changes Nov 18, 2021
View reviewed changes
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Thanks!

@pstibrany
Copy link
Member Author

Thank you all for reviews!

@pstibrany pstibrany enabled auto-merge (squash) November 18, 2021 14:05
@pstibrany pstibrany merged commit 72d6e0c into main Nov 18, 2021
@pstibrany pstibrany deleted the ignore-young-blocks branch November 18, 2021 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aknuds1 aknuds1 aknuds1 left review comments

@pracucci pracucci pracucci approved these changes

@stevesg stevesg stevesg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make store-gateway -querier.query-store-after aware
4 participants
@pstibrany @aknuds1 @pracucci @stevesg