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

optimize table retetion #3934

Merged
merged 1 commit into from
Jul 2, 2021
Merged

Conversation

sandeepsukhani
Copy link
Contributor

skip applying retention when data in table is still within retention …period and there are no overlapping delete requests

What this PR does / why we need it:
During each compaction, we download and iterate through all the tables and check each table for whether there are any chunks expired due to configured retention policy or a user issued delete request.
This causes the compaction to slow down and wastes the resources. This PR changes the code to check whether a table would have any expired data due to configured retention period or user issued delete request before applying retention to it. If there is no expired data then the whole table would be skipped.

Checklist

  • Tests updated

@@ -29,22 +30,27 @@ import (

type mockChunkClient struct {
mtx sync.Mutex
deletedChunks []string
deletedChunks map[string]struct{}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a flaky test when there can be duplicate chunk ids due to them overlapping 2 days and hence getting indexed in 2 tables. When chunk from each table gets deleted they get added here, hence creating a duplicate entry.

cc @cyriltovena

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome !

…period and there are no overlapping delete requests
Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

Nice one !

@sandeepsukhani sandeepsukhani merged commit 7a7eeb0 into grafana:main Jul 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants