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

Proposal: add -compactor.first-level-compaction-wait-period and deprecate -compactor.consistency-delay #4365

Closed
pracucci opened this issue Mar 3, 2023 · 2 comments
Assignees

Comments

@pracucci
Copy link
Collaborator

pracucci commented Mar 3, 2023

After #4364, ingesters could be configured to spread the TSDB head compaction over time. When this happens, ingesters blocks are uploaded at a different time and so compactor could start running the 2nd level compaction before all ingesters uploaded the blocks.

In the past, we tried to mitigate this issue configuring -compactor.consistency-delay but unfortunately it doesn't help with this issue. The reason is that -compactor.consistency-delay just applies a rolling window to source blocks considered for compaction, basically ignoring blocks whose timestamp (read from the ULID) is more recent than the configured -compactor.consistency-delay. For example, if you set the consistency delay to 10m, blocks created in the last 10m are ignored by the compactor, but the ingester blocks are uploaded over time, the compaction will start anyway as soon as there are at least 2 blocks older than 10m (and since ingester blocks upload is spread over time, there may be other blocks already uploaded but ignored because created less than 10m ago).

To mitigate this problem I propose to introduce -compactor.first-level-compaction-wait-period. The wait period is the time the compactor waits for a group of blocks before compacting them. For example, if you set the wait period to 10m, the compactor will not compact blocks in a group at all if in the group there's any block uploaded less than 10m ago. Differently than the consistency delay, all blocks are part of the group, but the whole group won't be compacted until there are no new blocks uploaded in the last "wait period" time.

To sum up, my proposal is:

  • Introduce -compactor.first-level-compaction-wait-period
  • Deprecate -compactor.consistency-delay because was inherited by Thanos, it was used for a different purpose (object storage with no read-after-write guarantee) and we never really used it in Mimir
@pstibrany
Copy link
Member

I agree this is a way to go. "grace period" naming seems very generic and not descriptive, what about "delay-compaction-with-recent-blocks-period" or something similar that would better describe the intent of the option?

@pracucci pracucci changed the title Proposal: add -compactor.compaction-grace-period and deprecate -compactor.consistency-delay Proposal: add -compactor.first-level-compaction-wait-period and deprecate -compactor.consistency-delay Mar 6, 2023
@pracucci pracucci self-assigned this Mar 7, 2023
@pracucci
Copy link
Collaborator Author

pracucci commented Mar 7, 2023

The support for -compactor.first-level-compaction-wait-period (#4401) has been merged. Next steps:

  • Deprecate (not remove) consistency delay (PR)

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

No branches or pull requests

2 participants