-
Notifications
You must be signed in to change notification settings - Fork 25.8k
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
Changes from 31 commits
c4b7415
938c28f
543595e
dd07b96
36b917a
f1ad482
a0922d5
eca4b62
6f71d66
ca13dca
18b65f4
58ba36c
fda84e2
8710cef
812d1b6
8427934
dc1ce06
9d57ba4
5380bdf
45a9bda
73f761b
8739150
c27cdf2
0252412
c3faab5
ab7ed54
9293773
bb356d2
1167ed2
4eb4248
19be162
543f1e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| pr: 82689 | ||
| summary: Enhancement/prevent 6.8 indices upgrade | ||
| area: Infra/Core | ||
| type: enhancement | ||
| issues: | ||
| - 81326 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,21 +28,30 @@ public final class NodeMetadata { | |
|
|
||
| static final String NODE_ID_KEY = "node_id"; | ||
| static final String NODE_VERSION_KEY = "node_version"; | ||
| static final String OLDEST_INDEX_VERSION_KEY = "oldest_index_version"; | ||
|
|
||
| private final String nodeId; | ||
|
|
||
| private final Version nodeVersion; | ||
|
|
||
| private final Version previousNodeVersion; | ||
|
|
||
| private NodeMetadata(final String nodeId, final Version nodeVersion, final Version previousNodeVersion) { | ||
| private final Version oldestIndexVersion; | ||
|
|
||
| private NodeMetadata( | ||
| final String nodeId, | ||
| final Version nodeVersion, | ||
| final Version previousNodeVersion, | ||
| final Version oldestIndexVersion | ||
| ) { | ||
| this.nodeId = Objects.requireNonNull(nodeId); | ||
| this.nodeVersion = Objects.requireNonNull(nodeVersion); | ||
| this.previousNodeVersion = Objects.requireNonNull(previousNodeVersion); | ||
| this.oldestIndexVersion = Objects.requireNonNull(oldestIndexVersion); | ||
| } | ||
|
|
||
| public NodeMetadata(final String nodeId, final Version nodeVersion) { | ||
| this(nodeId, nodeVersion, nodeVersion); | ||
| public NodeMetadata(final String nodeId, final Version nodeVersion, final Version oldestIndexVersion) { | ||
| this(nodeId, nodeVersion, nodeVersion, oldestIndexVersion); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -52,12 +61,13 @@ public boolean equals(Object o) { | |
| NodeMetadata that = (NodeMetadata) o; | ||
| return nodeId.equals(that.nodeId) | ||
| && nodeVersion.equals(that.nodeVersion) | ||
| && oldestIndexVersion.equals(that.oldestIndexVersion) | ||
| && Objects.equals(previousNodeVersion, that.previousNodeVersion); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(nodeId, nodeVersion, previousNodeVersion); | ||
| return Objects.hash(nodeId, nodeVersion, previousNodeVersion, oldestIndexVersion); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -70,6 +80,8 @@ public String toString() { | |
| + nodeVersion | ||
| + ", previousNodeVersion=" | ||
| + previousNodeVersion | ||
| + ", oldestIndexVersion=" | ||
| + oldestIndexVersion | ||
| + '}'; | ||
| } | ||
|
|
||
|
|
@@ -91,7 +103,11 @@ public Version previousNodeVersion() { | |
| return previousNodeVersion; | ||
| } | ||
|
|
||
| public NodeMetadata upgradeToCurrentVersion() { | ||
| public Version oldestIndexVersion() { | ||
| return oldestIndexVersion; | ||
| } | ||
|
|
||
| public void verifyUpgradeToCurrentVersion() { | ||
| assert (nodeVersion.equals(Version.V_EMPTY) == false) || (Version.CURRENT.major <= Version.V_7_0_0.major + 1) | ||
| : "version is required in the node metadata from v9 onwards"; | ||
|
|
||
|
|
@@ -113,14 +129,19 @@ public NodeMetadata upgradeToCurrentVersion() { | |
| "cannot downgrade a node from version [" + nodeVersion + "] to version [" + Version.CURRENT + "]" | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| return nodeVersion.equals(Version.CURRENT) ? this : new NodeMetadata(nodeId, Version.CURRENT, nodeVersion); | ||
| public NodeMetadata upgradeToCurrentVersion() { | ||
| verifyUpgradeToCurrentVersion(); | ||
|
|
||
| return nodeVersion.equals(Version.CURRENT) ? this : new NodeMetadata(nodeId, Version.CURRENT, nodeVersion, oldestIndexVersion); | ||
| } | ||
|
|
||
| private static class Builder { | ||
| String nodeId; | ||
| Version nodeVersion; | ||
| Version previousNodeVersion; | ||
| Version oldestIndexVersion; | ||
|
|
||
| public void setNodeId(String nodeId) { | ||
| this.nodeId = nodeId; | ||
|
|
@@ -134,8 +155,13 @@ public void setPreviousNodeVersionId(int previousNodeVersionId) { | |
| this.previousNodeVersion = Version.fromId(previousNodeVersionId); | ||
| } | ||
|
|
||
| public void setOldestIndexVersion(int oldestIndexVersion) { | ||
| this.oldestIndexVersion = Version.fromId(oldestIndexVersion); | ||
| } | ||
|
|
||
| public NodeMetadata build() { | ||
| final Version nodeVersion; | ||
| final Version oldestIndexVersion; | ||
| if (this.nodeVersion == null) { | ||
| assert Version.CURRENT.major <= Version.V_7_0_0.major + 1 : "version is required in the node metadata from v9 onwards"; | ||
| nodeVersion = Version.V_EMPTY; | ||
|
|
@@ -145,8 +171,13 @@ public NodeMetadata build() { | |
| if (this.previousNodeVersion == null) { | ||
| previousNodeVersion = nodeVersion; | ||
| } | ||
| if (this.oldestIndexVersion == null) { | ||
| oldestIndexVersion = Version.V_EMPTY; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is only used in the legacy format which we shouldn't really be reading here, but I think this means that the index compatibility check will fail before the node version check happens, so the error will say that there are some ancient indices rather than that the previous node version is too old. I wonder if it would be better to check the index version in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes exactly. I was testing direct to 8 upgrade from 6.8 and this showed up. Right now it says you have indices from version 0.0.0, which does the job but isn't the best. Good suggestion, I'll extract the upgrade/downgrade check on the metadata in a separate method and call that ahead. |
||
| } else { | ||
| oldestIndexVersion = this.oldestIndexVersion; | ||
| } | ||
|
|
||
| return new NodeMetadata(nodeId, nodeVersion, previousNodeVersion); | ||
| return new NodeMetadata(nodeId, nodeVersion, previousNodeVersion, oldestIndexVersion); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -163,6 +194,7 @@ static class NodeMetadataStateFormat extends MetadataStateFormat<NodeMetadata> { | |
| objectParser = new ObjectParser<>("node_meta_data", ignoreUnknownFields, Builder::new); | ||
| objectParser.declareString(Builder::setNodeId, new ParseField(NODE_ID_KEY)); | ||
| objectParser.declareInt(Builder::setNodeVersionId, new ParseField(NODE_VERSION_KEY)); | ||
| objectParser.declareInt(Builder::setOldestIndexVersion, new ParseField(OLDEST_INDEX_VERSION_KEY)); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -176,6 +208,7 @@ protected XContentBuilder newXContentBuilder(XContentType type, OutputStream str | |
| public void toXContent(XContentBuilder builder, NodeMetadata nodeMetadata) throws IOException { | ||
| builder.field(NODE_ID_KEY, nodeMetadata.nodeId); | ||
| builder.field(NODE_VERSION_KEY, nodeMetadata.nodeVersion.id); | ||
| builder.field(OLDEST_INDEX_VERSION_KEY, nodeMetadata.oldestIndexVersion.id); | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
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
legacyNodeLockon failure - on success we need to keep hold of it until the end of this method.We could move the
checkForIndexCompatibilitycall within the nexttryblock 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.