Skip to content

Conversation

@grcevski
Copy link
Contributor

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

Nikola Grcevski and others added 15 commits January 6, 2022 11:01
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 <[email protected]>
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.
@grcevski grcevski added >enhancement :Core/Infra/Core Core issues without another label v8.0.0 Team:Core/Infra Meta label for core/infra team auto-backport Automatically create backport pull requests when merged labels Jan 17, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@grcevski grcevski marked this pull request as draft January 17, 2022 17:20
@elasticsearchmachine
Copy link
Collaborator

Hi @grcevski, I've created a changelog YAML for you.

@grcevski
Copy link
Contributor Author

Still a draft because I need to figure out how to test this, other than the manual testing I did.

@grcevski grcevski changed the title Enhancement/prevent 6.8 indices upgrade Prevent 6.8 indices upgrade to 8.0 Jan 17, 2022
@grcevski grcevski added the WIP label Jan 17, 2022
() -> TimeValue.nsecToMSec(System.nanoTime())
);

PersistedClusterStateService.OnDiskState onDiskState = persistedClusterStateService.loadBestOnDiskState();
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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

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?

Copy link
Contributor Author

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.

grcevski added a commit that referenced this pull request Jan 20, 2022
Add oldest index version calculation and record it in NodeMetadata.

This check will be used by future releases to prevent irreversible 
upgrades of incompatible indices, e.g. 6.x indices to version 8.
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM

@grcevski grcevski merged commit be29e29 into elastic:master Jan 20, 2022
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.0 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 82689

grcevski pushed a commit to grcevski/elasticsearch that referenced this pull request Jan 20, 2022
grcevski added a commit that referenced this pull request Jan 20, 2022
grcevski pushed a commit to grcevski/elasticsearch that referenced this pull request Jan 20, 2022
@grcevski grcevski mentioned this pull request Jan 20, 2022
grcevski added a commit that referenced this pull request Jan 20, 2022
@albertzaharovits albertzaharovits changed the title Prevent 6.8 indices upgrade to 8.0 Prevent direct upgrade of indices from 6.8 to 8.0 Jan 25, 2022
@grcevski grcevski deleted the enhancement/prevent_6.8_indices_upgrade branch January 25, 2022 22:54
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Jan 26, 2022
The mutation function would rarely fail to mutate the input object. This
commit fixes that.

Relates elastic#82689
DaveCTurner added a commit that referenced this pull request Jan 27, 2022
The mutation function would rarely fail to mutate the input object. This
commit fixes that.

Relates #82689
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Jan 27, 2022
The mutation function would rarely fail to mutate the input object. This
commit fixes that.

Relates elastic#82689
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Jan 27, 2022
The mutation function would rarely fail to mutate the input object. This
commit fixes that.

Relates elastic#82689
elasticsearchmachine pushed a commit that referenced this pull request Jan 27, 2022
The mutation function would rarely fail to mutate the input object. This
commit fixes that.

Relates #82689
elasticsearchmachine pushed a commit that referenced this pull request Jan 27, 2022
…#83194)

* Fix NodeMetadataTests#testEqualsHashcodeSerialization (#83170)

The mutation function would rarely fail to mutate the input object. This
commit fixes that.

Relates #82689

* Old-skool switch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Core/Infra/Core Core issues without another label >enhancement Team:Core/Infra Meta label for core/infra team v7.17.0 v8.0.0 v8.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recovering from 8.0 upgrade failure due to unmigrated 6.x indices

5 participants