Skip to content

Compact idle blocks. #2803

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

Merged
merged 13 commits into from
Jun 30, 2020
Merged

Compact idle blocks. #2803

merged 13 commits into from
Jun 30, 2020

Conversation

pstibrany
Copy link
Contributor

@pstibrany pstibrany commented Jun 26, 2020

What this PR does: This PR adds compaction of blocks that are idle for longer than specified timeout.

Draft: needs tests.

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 ready for review June 29, 2020 06:32
Copy link
Contributor

@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 Peter! You're rocking this work!

Overall LGTM (just left few minor comments).

I think there's one missing thing. This feature is useful when enabling shuffle sharding or a tenant stop writing. The next ingester startup, we don't want to load the TSDB at all if all existing blocks on disk have passed the retention period. This part is missing (perfectly fine doing it in a separate PR, but I think should be done).

@pstibrany
Copy link
Contributor Author

The next ingester startup, we don't want to load the TSDB at all if all existing blocks on disk have passed the retention period. This part is missing (perfectly fine doing it in a separate PR, but I think should be done).

I agree this part is missing. I would like to address that in separate PR.

In this PR, I don't want to actually close idle TSDB, because that would cause issues with the way how we collect metrics, esp. in case we need to reopen it. But ignoring on startup is a good way to handle it, I think.

pstibrany added 12 commits June 30, 2020 15:32
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Copy link
Contributor

@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.

LGTM, thanks!

@pstibrany pstibrany merged commit 7326c6c into cortexproject:master Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 participants