-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add lower bound for translog generation threshold #23779
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
Add lower bound for translog generation threshold #23779
Conversation
The translog already occupies 43 bytes on disk when empty. If the translog generation threshold is below this, the flush thread can get stuck in an infinite repeatedly rolling the generation. This commit adds a lower bound on the translog generation to avoid this problem, however we keep the lower bound small for convenience in testing.
dakrone
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
bleskes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left a suggestion.
| final Translog translog = shard.getEngine().getTranslog(); | ||
| final long generation = translog.currentFileGeneration(); | ||
| for (int i = 0; i < randomIntBetween(32, 128); i++) { | ||
| final int numberOfDocuments = randomIntBetween(32, 128); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:)
| * generation threshold. However, small thresholds are useful for testing so we | ||
| * do not add a large lower bound here. | ||
| */ | ||
| new ByteSizeValue(64, ByteSizeUnit.BYTES), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should wire this to org.elasticsearch.index.translog.TranslogWriter#getHeaderLength(int) with a static constant somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good thought, and I thought about it after your suggestion. I would prefer to keep this predictable for now rather than having the minimum quietly change if we change the translog. I do not expect the header to be changing frequently, but we can revisit this if needed.
|
to be clear - no need for another review. |
The translog already occupies 43 bytes on disk when empty. If the translog generation threshold is below this, the flush thread can get stuck in an infinite loop repeatedly rolling the generation. This commit adds a lower bound on the translog generation to avoid this problem, however we keep the lower bound small for convenience in testing.
Relates #23606