Generating and committing a history_uuid on existing old indices destroys translog recovery info#26734
Merged
bleskes merged 5 commits intoelastic:6.xfrom Sep 21, 2017
Merged
Conversation
…hat expect a commit to have translog info
ywelsch
approved these changes
Sep 21, 2017
Contributor
ywelsch
left a comment
There was a problem hiding this comment.
LGTM
This PR is opened against 6.x . We have the option to leave the code on master as is. Let me know which you prefer for the long term. I can go either way.
I have a weak preference to keep master as is and have this only go into 6.x. I'll leave it up to you.
| // assert we don't loose key entries | ||
| assert commitDataAsMap(writer).containsKey(Translog.TRANSLOG_UUID_KEY) : "commit misses translog uuid"; | ||
| assert commitDataAsMap(writer).containsKey(Translog.TRANSLOG_GENERATION_KEY) : "commit misses translog generation"; | ||
| assert commitDataAsMap(writer).containsKey(MAX_UNSAFE_AUTO_ID_TIMESTAMP_COMMIT_ID) : "commit misses max unsafe times stamps"; |
Contributor
Author
|
Thx @ywelsch . I will leave master as is. Having two commitIndexWriter methods doesn't have a good smell (even with the assertions I added to protect against abuse). |
bleskes
added a commit
that referenced
this pull request
Sep 21, 2017
…roys translog recovery info (#26734) This is a bug introduced in #26694 . The issue comes from the attempt to share code that commits the new history uuid and/or a new translog uuid. This goes wrong an existing 5.6 index that is recovered from store: 1) A new history uuid is generated as it doesn't exist in the index 2) The translog is opened and it's uuid doesn't change. 3) The committing the new history uuid used the standard commitIndexWriter method. 4) The latter asks the translog for the oldest file generation that is required to recover from the local checkpoint + 1 5) The local checkpoint on old indices is -1 (until something is indexed) and the translog is asked to recover from position 0. That excludes any operations the translog that do not have a seq no assigned, causing the FullClusterRestart bwc tests to fail. To bypass this commit moves away from the attempt to share the committing code between a new translog uuid and a new history uuid. Instead we do what we did before and open the translog with a potential commit. Afterwards we commit the history uuid if needed. This comes with the expense of opening up the method to commit an index writer in the engine.
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
5 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.
This is a bug introduced in #26694 . The issue comes from the attempt to share code that commits the new history uuid and/or a new translog uuid. This goes wrong an existing 5.6 index that is recovered from store:
To bypass this PR moves away from the attempt to share the committing code between a new translog uuid and a new history uuid. Instead we do what we did before and open the translog with a potential commit. Afterwards we commit the history uuid if needed. This comes with the expense of opening up the method to commit an index writer in the engine.
This PR is opened against 6.x . We have the option to leave the code on master as is. Let me know which you prefer for the long term. I can go either way.