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

Changed -compactor.max-compaction-time default from 0s to 1h #2514

Merged
merged 2 commits into from
Jul 22, 2022

Conversation

pracucci
Copy link
Collaborator

@pracucci pracucci commented Jul 22, 2022

What this PR does

I propose to change -compactor.max-compaction-time default from 0s to 1h, to aim for a better fairness between tenants (when running a small number of compactor replicas) and address #2302 too.

I think enabling it (and setting to 1h) doesn't have any significant downside. For example, let's consider a single tenant cluster, with very long compactions. After 1h in a compaction cycle, the compactor stops compacting more blocks. However, since -compactor.compaction-interval=1h, the ticker already put a message in the channel so the next compaction cycle starts right after the previous one ended because the 1h limit has been reached. Basically, excluding the time it takes to scan the bucket, there's no expected "delay" introduced by setting -compactor.max-compaction-time when it triggers. Makes sense?

Which issue(s) this PR fixes or relates to

Part of #2302

Checklist

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

Signed-off-by: Marco Pracucci <marco@pracucci.com>
@pracucci pracucci requested a review from pstibrany July 22, 2022 07:53
Copy link
Member

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Given our default compaction interval is 1h, it would make sense to me to set max-compaction-time for tenant to the same value. But 2h is good start too.

@pracucci pracucci changed the title Changed -compactor.max-compaction-time default from 0s to 2h Changed -compactor.max-compaction-time default from 0s to 1h Jul 22, 2022
…terval

Signed-off-by: Marco Pracucci <marco@pracucci.com>
@pracucci pracucci merged commit 0b35fc6 into main Jul 22, 2022
@pracucci pracucci deleted the change-max-compaction-time-default branch July 22, 2022 08:31
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.

None yet

2 participants