Skip to content

Add lower bound for translog generation threshold#23779

Merged
jasontedor merged 1 commit intoelastic:masterfrom
jasontedor:min-generation-threshold
Mar 28, 2017
Merged

Add lower bound for translog generation threshold#23779
jasontedor merged 1 commit intoelastic:masterfrom
jasontedor:min-generation-threshold

Conversation

@jasontedor
Copy link
Copy Markdown
Member

@jasontedor jasontedor commented Mar 28, 2017

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

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.
@jasontedor jasontedor added :Translog >test Issues or PRs that are addressing/adding tests v6.0.0-alpha1 labels Mar 28, 2017
@jasontedor jasontedor requested a review from bleskes March 28, 2017 17:41
Copy link
Copy Markdown
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@bleskes bleskes left a 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);
Copy link
Copy Markdown
Contributor

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),
Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Member Author

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.

@bleskes
Copy link
Copy Markdown
Contributor

bleskes commented Mar 28, 2017

to be clear - no need for another review.

@jasontedor jasontedor merged commit 7282460 into elastic:master Mar 28, 2017
@jasontedor jasontedor deleted the min-generation-threshold branch March 28, 2017 18:11
@clintongormley clintongormley added :Distributed/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. and removed :Translog :Distributed/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Engine Anything around managing Lucene and the Translog in an open shard. >test Issues or PRs that are addressing/adding tests v6.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants