-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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 in-place downgrades and invalid upgrades #41731
Changes from 10 commits
97ef476
b22d9dd
374ff83
27c14af
4961fd1
d4888db
e805ec3
3320d5a
c154c2e
00abb44
57425f4
9cb083f
e12ca62
ba6ae23
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 |
---|---|---|
|
@@ -44,7 +44,7 @@ | |
public abstract class ElasticsearchNodeCommand extends EnvironmentAwareCommand { | ||
private static final Logger logger = LogManager.getLogger(ElasticsearchNodeCommand.class); | ||
protected final NamedXContentRegistry namedXContentRegistry; | ||
static final String DELIMITER = "------------------------------------------------------------------------\n"; | ||
protected static final String DELIMITER = "------------------------------------------------------------------------\n"; | ||
|
||
static final String STOP_WARNING_MSG = | ||
DELIMITER + | ||
|
@@ -177,6 +177,17 @@ protected void cleanUpOldMetaData(Terminal terminal, Path[] dataPaths, long newG | |
MetaData.FORMAT.cleanupOldFiles(newGeneration, dataPaths); | ||
} | ||
|
||
protected NodeEnvironment.NodePath[] toNodePaths(Path[] dataPaths) { | ||
return Arrays.stream(dataPaths).map(ElasticsearchNodeCommand::createNodePath).toArray(NodeEnvironment.NodePath[]::new); | ||
} | ||
|
||
private static NodeEnvironment.NodePath createNodePath(Path path) { | ||
try { | ||
return new NodeEnvironment.NodePath(path); | ||
} catch (IOException e) { | ||
throw new ElasticsearchException("Unable to investigate path: " + path + ": " + e.getMessage()); | ||
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. why not pass 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. I blindly moved this method here from |
||
} | ||
} | ||
|
||
//package-private for testing | ||
OptionParser getParser() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
|
||
package org.elasticsearch.env; | ||
|
||
import org.elasticsearch.Version; | ||
import org.elasticsearch.common.ParseField; | ||
import org.elasticsearch.common.xcontent.ObjectParser; | ||
import org.elasticsearch.common.xcontent.XContentBuilder; | ||
|
@@ -31,66 +32,104 @@ | |
import java.util.Objects; | ||
|
||
/** | ||
* Metadata associated with this node. Currently only contains the unique uuid describing this node. | ||
* Metadata associated with this node: its persistent node ID and its version. | ||
* The metadata is persisted in the data folder of this node and is reused across restarts. | ||
*/ | ||
public final class NodeMetaData { | ||
|
||
private static final String NODE_ID_KEY = "node_id"; | ||
private static final String NODE_VERSION_KEY = "node_version"; | ||
|
||
private final String nodeId; | ||
|
||
public NodeMetaData(final String nodeId) { | ||
private final Version nodeVersion; | ||
|
||
public NodeMetaData(final String nodeId, final Version nodeVersion) { | ||
this.nodeId = Objects.requireNonNull(nodeId); | ||
this.nodeVersion = Objects.requireNonNull(nodeVersion); | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) { | ||
return true; | ||
} | ||
if (o == null || getClass() != o.getClass()) { | ||
return false; | ||
} | ||
|
||
if (this == o) return true; | ||
if (o == null || getClass() != o.getClass()) return false; | ||
NodeMetaData that = (NodeMetaData) o; | ||
|
||
return Objects.equals(this.nodeId, that.nodeId); | ||
return nodeId.equals(that.nodeId) && | ||
nodeVersion.equals(that.nodeVersion); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return this.nodeId.hashCode(); | ||
return Objects.hash(nodeId, nodeVersion); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "node_id [" + nodeId + "]"; | ||
return "NodeMetaData{" + | ||
"nodeId='" + nodeId + '\'' + | ||
", nodeVersion=" + nodeVersion + | ||
'}'; | ||
} | ||
|
||
private static ObjectParser<Builder, Void> PARSER = new ObjectParser<>("node_meta_data", Builder::new); | ||
|
||
static { | ||
PARSER.declareString(Builder::setNodeId, new ParseField(NODE_ID_KEY)); | ||
PARSER.declareInt(Builder::setNodeVersionId, new ParseField(NODE_VERSION_KEY)); | ||
} | ||
|
||
public String nodeId() { | ||
return nodeId; | ||
} | ||
|
||
public Version nodeVersion() { | ||
return nodeVersion; | ||
} | ||
|
||
public NodeMetaData upgradeToCurrentVersion() { | ||
if (nodeVersion.equals(Version.V_EMPTY)) { | ||
assert Version.CURRENT.major <= Version.V_7_0_0.major + 1 : "version is required in the node metadata from v9 onwards"; | ||
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. Why not to use V_8_0_0 instead of V_7_0_0+1 ? 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. When we upgrade |
||
return new NodeMetaData(nodeId, Version.CURRENT); | ||
} | ||
|
||
if (nodeVersion.before(Version.CURRENT.minimumIndexCompatibilityVersion())) { | ||
throw new IllegalStateException( | ||
"cannot upgrade a node from version [" + nodeVersion + "] directly to version [" + Version.CURRENT + "]"); | ||
} | ||
|
||
if (nodeVersion.after(Version.CURRENT)) { | ||
throw new IllegalStateException( | ||
"cannot downgrade a node from version [" + nodeVersion + "] to version [" + Version.CURRENT + "]"); | ||
} | ||
|
||
return nodeVersion.equals(Version.CURRENT) ? this : new NodeMetaData(nodeId, Version.CURRENT); | ||
} | ||
|
||
private static class Builder { | ||
String nodeId; | ||
Version nodeVersion; | ||
|
||
public void setNodeId(String nodeId) { | ||
this.nodeId = nodeId; | ||
} | ||
|
||
public void setNodeVersionId(int nodeVersionId) { | ||
this.nodeVersion = Version.fromId(nodeVersionId); | ||
} | ||
|
||
public NodeMetaData build() { | ||
return new NodeMetaData(nodeId); | ||
final Version nodeVersion; | ||
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"; | ||
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. Why not to use V_8_0_0 instead of V_7_0_0+1 ? 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. When we upgrade |
||
nodeVersion = Version.V_EMPTY; | ||
} else { | ||
nodeVersion = this.nodeVersion; | ||
} | ||
|
||
return new NodeMetaData(nodeId, nodeVersion); | ||
} | ||
} | ||
|
||
|
||
public static final MetaDataStateFormat<NodeMetaData> FORMAT = new MetaDataStateFormat<NodeMetaData>("node-") { | ||
|
||
@Override | ||
|
@@ -103,10 +142,11 @@ protected XContentBuilder newXContentBuilder(XContentType type, OutputStream str | |
@Override | ||
public void toXContent(XContentBuilder builder, NodeMetaData nodeMetaData) throws IOException { | ||
builder.field(NODE_ID_KEY, nodeMetaData.nodeId); | ||
builder.field(NODE_VERSION_KEY, nodeMetaData.nodeVersion.id); | ||
} | ||
|
||
@Override | ||
public NodeMetaData fromXContent(XContentParser parser) throws IOException { | ||
public NodeMetaData fromXContent(XContentParser parser) { | ||
return PARSER.apply(parser, null).build(); | ||
} | ||
}; | ||
|
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.
how about
override-minimum-version
instead ofoverwrite-version
?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.
The version checks are not simply a minimum. Today they also include a maximum, and in future we may add more complex constraints. But I do prefer
override
, thanks, I adjusted things in ba6ae23.