Add lower bound for translog flush threshold#28382
Merged
dnhatn merged 4 commits intoelastic:masterfrom Feb 1, 2018
Merged
Conversation
If the translog flush threshold is too small (eg. smaller than the translog header), we may repeatedly flush even there is no uncommitted operation because the shouldFlush condition can still be true after flushing. This is currently avoided by adding an extra guard against the uncommitted operations. However, this extra guard makes the shouldFlush complicated. This commit replaces that extra guard by a lower bound for translog flush threshold. We keep the lower bound small for convenience in testing. Relates elastic#28350 Relates elastic#23606
bleskes
approved these changes
Feb 1, 2018
Contributor
bleskes
left a comment
There was a problem hiding this comment.
LGTM. Left some asks, no need for another review.
| public static final String CHECKPOINT_FILE_NAME = "translog" + CHECKPOINT_SUFFIX; | ||
|
|
||
| static final Pattern PARSE_STRICT_ID_PATTERN = Pattern.compile("^" + TRANSLOG_FILE_PREFIX + "(\\d+)(\\.tlog)$"); | ||
| public static final int DEFAULT_HEADER_SIZE_IN_BYTES = TranslogWriter.getHeaderLength(UUIDs.randomBase64UUID()); |
Contributor
There was a problem hiding this comment.
can we assert somewhere that this is correct?
| * non-zero value after rolling a new translog generation. This can be avoided by checking the actual uncommitted operations. | ||
| */ | ||
| return uncommittedSizeOfNewCommit < uncommittedSizeOfCurrentCommit && translog.uncommittedOperations() > 0; | ||
| return uncommittedSizeOfNewCommit < uncommittedSizeOfCurrentCommit; |
Contributor
There was a problem hiding this comment.
can we add an assertion that the uncommitOps is >0 ?
| * generation threshold. However, small thresholds are useful for testing so we | ||
| * do not add a large lower bound here. | ||
| */ | ||
| new ByteSizeValue(64, ByteSizeUnit.BYTES), |
Contributor
There was a problem hiding this comment.
I think we need to adapt the comment above?
Member
Author
|
Thanks @bleskes for reviewing. |
dnhatn
added a commit
that referenced
this pull request
Feb 1, 2018
If the translog flush threshold is too small (eg. smaller than the translog header), we may repeatedly flush even there is no uncommitted operation because the shouldFlush condition can still be true after flushing. This is currently avoided by adding an extra guard against the uncommitted operations. However, this extra guard makes the shouldFlush complicated. This commit replaces that extra guard by a lower bound for translog flush threshold. We keep the lower bound small for convenience in testing. Relates #28350 Relates #23606
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
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.
If the translog flush threshold is too small (eg. smaller than the
translog header), we may repeatedly flush even there is no uncommitted
operation because the shouldFlush condition can still be true after
flushing. This is currently avoided by adding an extra guard against the
uncommitted operations. However, this extra guard makes the shouldFlush
complicated. This commit replaces that extra guard by a lower bound for
translog flush threshold. We keep the lower bound small for convenience
in testing.
Relates #28350
Relates #23779