-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
Clarify global checkpoint recovery #21934
Clarify global checkpoint recovery #21934
Conversation
Today when starting a new engine, we read the global checkpoint from the translog only if we are opening an existing translog. This commit clarifies this situation by distinguishing the three cases of engine creation in the constructor leading to clearer code.
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. Thanks @jasontedor
seqNoStats.getMaxSeqNo(), | ||
seqNoStats.getLocalCheckpoint(), | ||
seqNoStats.getGlobalCheckpoint()); | ||
logger.trace( |
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.
nit: seqNoStats has a toString - just use it and be safe?
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 pushed 7a48c39.
@@ -209,6 +219,18 @@ public InternalEngine(EngineConfig engineConfig) throws EngineException { | |||
logger.trace("created new InternalEngine"); | |||
} | |||
|
|||
private static SequenceNumbersService sequenceNumberService( |
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.
this is what you get from the crazy syntax 👅
retest this please |
In the constructor for the internal engine, we log a trace statement showing the recovered max sequence number, local checkpoint, and global checkpoint. This commit simplifies this logging statement to just use the fact that SeqNoStats implements toString.
Thanks @bleskes. |
Today when starting a new engine, we read the global checkpoint from the
translog only if we are opening an existing translog. This commit
clarifies this situation by distinguishing the three cases of engine
creation in the constructor leading to clearer code.
Relates #21254