Apply jitter to the configured timeout for performing idle compactions. #3850
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stevesg
commented
Feb 22, 2021
stevesg
commented
Feb 22, 2021
stevesg
force-pushed
the
compact-jitter
branch
from
85caa29 to
eef313b
Compare
|
Rebase |
pracucci
requested review from
pracucci and
pstibrany
and removed request for
pstibrany
pracucci
reviewed
Feb 22, 2021
Attempt to prevent idle compaction from happening concurrently by introducing a 25% jitter to the configured timeout. This jitter only increases the timeout, such that the configured minimum timeout is always adhered to. Signed-off-by: Steve Simpson <[email protected]>
Signed-off-by: Steve Simpson <[email protected]>
Move the setting of TSDB.compactionIdleTimeout just after struct creation. This avoids a race condition in the unit test because it makes calls to compactBlocks directly. Signed-off-by: Steve Simpson <[email protected]>
stevesg
force-pushed
the
compact-jitter
branch
from
22e5fcb to
b9c5cc0
Compare
|
Rebase |
Signed-off-by: Steve Simpson <[email protected]>
alvinlin123
reviewed
Feb 22, 2021
| # If TSDB head is idle for this duration, it is compacted. 0 means disabled. | ||
| # If TSDB head is idle for this duration, it is compacted. Note that up to | ||
| # 25% jitter is added to the value to avoid ingesters compacting |
There was a problem hiding this comment.
I am just wondering, what is the reason to leave the jitter percentage unconfigurable to the user?
There was a problem hiding this comment.
We didn't have a strong opinion here, other than following how jitter has typically been added in other places.
I don't think there would be opposition to be being configurable though.
There was a problem hiding this comment.
I see. Make sense. I agree we can make it configurable when the need arises in a separate PR.
alvinlin123
approved these changes
Feb 22, 2021
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does:
Attempt to prevent idle compaction from happening concurrently by
introducing a 25% jitter to the configured timeout. This jitter only
increases the timeout, such that the configured minimum timeout is
always adhered to.
Which issue(s) this PR fixes:
Fixes #3832
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]