-
Notifications
You must be signed in to change notification settings - Fork 25.4k
Prevent direct upgrade of indices from 6.8 to 8.0 #82689
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
Prevent direct upgrade of indices from 6.8 to 8.0 #82689
Conversation
This PR introduces a check to prevent upgrading to 8.0 without first upgrading to 7.last. Closes elastic#81865
…rom_7.nonlast' into enhancement/prevent_upgrades_from_7.nonlast
…ecurity/SecurityImplicitBehaviorBootstrapCheckTests.java Co-authored-by: David Turner <david.turner@elastic.co>
Upgrading to 8.0 with 6.8 indices causes data corruption, since we modify the indices directories in irreversible manner. This change introduces the following: - code to compute the oldest seen index version - code to check early enough for the incompatible indices and bail before any irreversible changes are made.
Pinging @elastic/es-core-infra (Team:Core/Infra) |
Hi @grcevski, I've created a changelog YAML for you. |
Still a draft because I need to figure out how to test this, other than the manual testing I did. |
() -> TimeValue.nsecToMSec(System.nanoTime()) | ||
); | ||
|
||
PersistedClusterStateService.OnDiskState onDiskState = persistedClusterStateService.loadBestOnDiskState(); |
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 think this is too early in the node's lifecycle for loading the cluster state (that's why you had to create your own NamedXContentRegistry
). Instead we should record the oldest index version in NodeMetadata
, which means adding an entry to the userData
attached to the Lucene commit for the cluster state.
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.
OK, great. I'll try that approach instead.
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.
Looks good except for one tiny thing...
try { | ||
checkForIndexCompatibility(logger, legacyNodeLock.getNodePaths()); | ||
} finally { | ||
legacyNodeLock.close(); |
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 think we should only release the legacyNodeLock
on failure - on success we need to keep hold of it until the end of this method.
We could move the checkForIndexCompatibility
call within the next try
block right?
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.
Ah good point, I was fixing a bug and you are right, I release it too soon.
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
💔 Backport failed
You can use sqren/backport to manually backport by running |
Relates to elastic#82689
The mutation function would rarely fail to mutate the input object. This commit fixes that. Relates elastic#82689
The mutation function would rarely fail to mutate the input object. This commit fixes that. Relates #82689
The mutation function would rarely fail to mutate the input object. This commit fixes that. Relates elastic#82689
The mutation function would rarely fail to mutate the input object. This commit fixes that. Relates elastic#82689
This PR adds a check that relies on 7.17 calculating the overall index age
metadata from the existing indices we have. Once we have the oldest index
age computed, we add a check to prevent upgrades to 8.0 with any indices
that are older than 7.0.
Closes #81326