Skip to content
5 changes: 2 additions & 3 deletions server/src/main/java/org/elasticsearch/ReleaseVersions.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
package org.elasticsearch;

import org.elasticsearch.common.Strings;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.core.UpdateForV10;
import org.elasticsearch.internal.BuildExtension;
import org.elasticsearch.plugins.ExtensionLoader;

Expand Down Expand Up @@ -114,8 +114,7 @@ private static IntFunction<String> lookupFunction(NavigableMap<Integer, List<Ver
// this will no longer be the case with ES 10 (which won't know about ES v8.x where we introduced separated versions)
// maybe keep the release mapping around in the csv file?
// SEP for now
@UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA)
// @UpdateForV10(owner = UpdateForV10.Owner.CORE_INFRA)
@UpdateForV10(owner = UpdateForV10.Owner.CORE_INFRA)
Version oldVersion = Version.fromId(id);
return oldVersion.toString();
}
Expand Down
20 changes: 9 additions & 11 deletions server/src/main/java/org/elasticsearch/Version.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.core.Assertions;
import org.elasticsearch.core.RestApiVersion;
import org.elasticsearch.core.SuppressForbidden;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.monitor.jvm.JvmInfo;
import org.elasticsearch.xcontent.ToXContentFragment;
import org.elasticsearch.xcontent.XContentBuilder;
Expand Down Expand Up @@ -240,17 +240,15 @@ public class Version implements VersionId<Version>, ToXContentFragment {
VERSION_STRINGS = Map.copyOf(builderByString);
}

@UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA)
// Re-enable this assertion once the rest api version is bumped
private static void assertRestApiVersion() {
// assert RestApiVersion.current().major == CURRENT.major && RestApiVersion.previous().major == CURRENT.major - 1
// : "RestApiVersion must be upgraded "
// + "to reflect major from Version.CURRENT ["
// + CURRENT.major
// + "]"
// + " but is still set to ["
// + RestApiVersion.current().major
// + "]";
assert RestApiVersion.current().major == CURRENT.major && RestApiVersion.previous().major == CURRENT.major - 1
: "RestApiVersion must be upgraded "
+ "to reflect major from Version.CURRENT ["
+ CURRENT.major
+ "]"
+ " but is still set to ["
+ RestApiVersion.current().major
+ "]";
}

public static Version readVersion(StreamInput in) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1037,12 +1037,6 @@ public static ClusterState readFrom(StreamInput in, DiscoveryNode localNode) thr
return builder.build();
}

/**
* If the cluster state does not contain transport version information, this is the version
* that is inferred for all nodes on version 8.8.0 or above.
*/
@UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA)
public static final TransportVersion INFERRED_TRANSPORT_VERSION = TransportVersions.V_8_8_0;
public static final Version VERSION_INTRODUCING_TRANSPORT_VERSIONS = Version.V_8_8_0;

@Override
Expand Down
30 changes: 9 additions & 21 deletions server/src/main/java/org/elasticsearch/env/NodeEnvironment.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.lucene.store.NativeFSLockFactory;
import org.elasticsearch.Build;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
Expand Down Expand Up @@ -86,8 +87,6 @@
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -525,7 +524,7 @@ static void checkForIndexCompatibility(Logger logger, DataPath... dataPaths) thr
logger.info("oldest index version recorded in NodeMetadata {}", metadata.oldestIndexVersion());

if (metadata.oldestIndexVersion().before(IndexVersions.MINIMUM_COMPATIBLE)) {
String bestDowngradeVersion = getBestDowngradeVersion(metadata.previousNodeVersion().toString());
BuildVersion bestDowngradeVersion = getBestDowngradeVersion(metadata.previousNodeVersion());
throw new IllegalStateException(
"Cannot start this node because it holds metadata for indices with version ["
+ metadata.oldestIndexVersion().toReleaseVersion()
Expand Down Expand Up @@ -1504,28 +1503,17 @@ private static void tryWriteTempFile(Path path) throws IOException {
/**
* Get a useful version string to direct a user's downgrade operation
*
* <p>If a user is trying to install 8.0 but has incompatible indices, the user should
* downgrade to 7.17.x. We return 7.17.0, unless the user is trying to upgrade from
* a 7.17.x release, in which case we return the last installed version.
* <p>If a user is trying to install 9.0 (current major) but has incompatible indices, the user should
Copy link
Member

Choose a reason for hiding this comment

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

Could you rewrite this in abstract terms so we don't need to update it with each major release? You could use N.0 and (N-1).last

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

* downgrade to 8.18.x (last minor of the previous major). We return 8.18.0, unless the user is trying to upgrade from
* a 8.18.x release, in which case we return the last installed version.
* @return Version to downgrade to
*/
// visible for testing
static String getBestDowngradeVersion(String previousNodeVersion) {
// this method should only be called in the context of an upgrade to 8.x
assert Build.current().version().startsWith("9.") == false;
Pattern pattern = Pattern.compile("^7\\.(\\d+)\\.\\d+$");
Matcher matcher = pattern.matcher(previousNodeVersion);
if (matcher.matches()) {
try {
int minorVersion = Integer.parseInt(matcher.group(1));
if (minorVersion >= 17) {
return previousNodeVersion;
}
} catch (NumberFormatException e) {
// continue and return default
}
static BuildVersion getBestDowngradeVersion(BuildVersion previousNodeVersion) {
if (previousNodeVersion.onOrAfterMinimumCompatible()) {
return previousNodeVersion;
}
return "7.17.0";
return BuildVersion.fromVersionId(Version.CURRENT.minimumCompatibilityVersion().id);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it ok to use Version here?
In previous PR: #117992 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

We should add this to BuildVersion to avoid inferring build versions from version ids.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}

}
4 changes: 1 addition & 3 deletions server/src/main/java/org/elasticsearch/env/NodeMetadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
package org.elasticsearch.env;

import org.elasticsearch.Build;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.gateway.MetadataStateFormat;
import org.elasticsearch.index.IndexVersion;
import org.elasticsearch.index.IndexVersions;
Expand Down Expand Up @@ -158,12 +157,11 @@ public void setOldestIndexVersion(int oldestIndexVersion) {
this.oldestIndexVersion = IndexVersion.fromId(oldestIndexVersion);
}

@UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) // version is required in the node metadata from v9 onwards
public NodeMetadata build() {
final IndexVersion oldestIndexVersion;

if (this.nodeVersion == null) {
nodeVersion = BuildVersion.fromVersionId(0);
throw new IllegalStateException("Node version is required in node metadata");
}
if (this.previousNodeVersion == null) {
previousNodeVersion = nodeVersion;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,9 @@ public void testIndexCompatibilityChecks() throws IOException {
containsString("it holds metadata for indices with version [" + oldIndexVersion.toReleaseVersion() + "]"),
containsString(
"Revert this node to version ["
+ (previousNodeVersion.major == Version.V_8_0_0.major ? Version.V_7_17_0 : previousNodeVersion)
+ (previousNodeVersion.major == Version.CURRENT.major
? Version.CURRENT.minimumCompatibilityVersion()
: previousNodeVersion)
+ "]"
)
)
Expand Down Expand Up @@ -638,20 +640,31 @@ public void testSymlinkDataDirectory() throws Exception {
env.close();
}

@UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA)
@AwaitsFix(bugUrl = "test won't work until we remove and bump minimum index versions")
public void testGetBestDowngradeVersion() {
assertThat(NodeEnvironment.getBestDowngradeVersion("7.17.0"), Matchers.equalTo("7.17.0"));
assertThat(NodeEnvironment.getBestDowngradeVersion("7.17.5"), Matchers.equalTo("7.17.5"));
assertThat(NodeEnvironment.getBestDowngradeVersion("7.17.1234"), Matchers.equalTo("7.17.1234"));
assertThat(NodeEnvironment.getBestDowngradeVersion("7.18.0"), Matchers.equalTo("7.18.0"));
assertThat(NodeEnvironment.getBestDowngradeVersion("7.17.x"), Matchers.equalTo("7.17.0"));
assertThat(NodeEnvironment.getBestDowngradeVersion("7.17.5-SNAPSHOT"), Matchers.equalTo("7.17.0"));
assertThat(NodeEnvironment.getBestDowngradeVersion("7.17.6b"), Matchers.equalTo("7.17.0"));
assertThat(NodeEnvironment.getBestDowngradeVersion("7.16.0"), Matchers.equalTo("7.17.0"));
// when we get to version 7.2147483648.0 we will have to rethink our approach, but for now we return 7.17.0 with an integer overflow
assertThat(NodeEnvironment.getBestDowngradeVersion("7." + Integer.MAX_VALUE + "0.0"), Matchers.equalTo("7.17.0"));
assertThat(NodeEnvironment.getBestDowngradeVersion("foo"), Matchers.equalTo("7.17.0"));
assertThat(
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.18.0")),
Matchers.equalTo(BuildVersion.fromString("8.18.0"))
);
assertThat(
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.18.5")),
Matchers.equalTo(BuildVersion.fromString("8.18.5"))
);
assertThat(
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.18.12")),
Matchers.equalTo(BuildVersion.fromString("8.18.12"))
);
assertThat(
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.19.0")),
Matchers.equalTo(BuildVersion.fromString("8.19.0"))
);
assertThat(
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.17.0")),
Matchers.equalTo(BuildVersion.fromString("8.18.0"))
);
assertThat(
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("7.17.0")),
Matchers.equalTo(BuildVersion.fromString("8.18.0"))
);
}

private void verifyFailsOnShardData(Settings settings, Path indexPath, String shardDataDirName) {
Expand Down
28 changes: 12 additions & 16 deletions server/src/test/java/org/elasticsearch/env/NodeMetadataTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@
package org.elasticsearch.env;

import org.elasticsearch.Build;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.gateway.MetadataStateFormat;
import org.elasticsearch.index.IndexVersion;
import org.elasticsearch.index.IndexVersions;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.EqualsHashCodeTestUtils;
import org.elasticsearch.test.VersionUtils;
Expand All @@ -28,6 +27,7 @@
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.startsWith;

Expand Down Expand Up @@ -80,22 +80,20 @@ public void testEqualsHashcodeSerialization() {
);
}

@UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA)
@AwaitsFix(bugUrl = "as mentioned in the comment below, the behavior here is changing for 9.0 so this test needs updating")
public void testReadsFormatWithoutVersion() throws IOException {
// the behaviour tested here is only appropriate if the current version is compatible with versions 7 and earlier
assertTrue(IndexVersions.MINIMUM_COMPATIBLE.onOrBefore(IndexVersions.V_7_0_0));
// when the current version is incompatible with version 7, the behaviour should change to reject files like the given resource
// which do not have the version field

public void testFailsToReadFormatWithoutVersion() throws IOException {
final Path tempDir = createTempDir();
final Path stateDir = Files.createDirectory(tempDir.resolve(MetadataStateFormat.STATE_DIR_NAME));
final InputStream resource = this.getClass().getResourceAsStream("testReadsFormatWithoutVersion.binary");
assertThat(resource, notNullValue());
Files.copy(resource, stateDir.resolve(NodeMetadata.FORMAT.getStateFileName(between(0, Integer.MAX_VALUE))));
final NodeMetadata nodeMetadata = NodeMetadata.FORMAT.loadLatestState(logger, xContentRegistry(), tempDir);
assertThat(nodeMetadata.nodeId(), equalTo("y6VUVMSaStO4Tz-B5BxcOw"));
assertThat(nodeMetadata.nodeVersion(), equalTo(BuildVersion.fromVersionId(0)));

ElasticsearchException ex = assertThrows(
Copy link
Member

Choose a reason for hiding this comment

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

assertThrows comes from junit and is a bit wonky in my experience. We use expectThrows in other parts of the codebase (which is our own, and existed before junit added a method for this). Let's prefer using that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

ElasticsearchException.class,
() -> NodeMetadata.FORMAT.loadLatestState(logger, xContentRegistry(), tempDir)
);
Throwable rootCause = ex.getRootCause();
assertThat(rootCause, instanceOf(IllegalStateException.class));
assertThat("Node version is required in node metadata", equalTo(rootCause.getMessage()));
}

public void testUpgradesLegitimateVersions() {
Expand Down Expand Up @@ -155,11 +153,9 @@ public void testDoesNotUpgradeAncientVersion() {
);
}

@UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA)
@AwaitsFix(bugUrl = "Needs to be updated for 9.0 version bump")
public void testUpgradeMarksPreviousVersion() {
final String nodeId = randomAlphaOfLength(10);
final Version version = VersionUtils.randomVersionBetween(random(), Version.CURRENT.minimumCompatibilityVersion(), Version.V_8_0_0);
final Version version = VersionUtils.randomVersionBetween(random(), Version.CURRENT.minimumCompatibilityVersion(), Version.V_9_0_0);
Copy link
Member

Choose a reason for hiding this comment

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

Can this test be rewritten to be built on BuildVersion and IndexVersion? We seem to be still using Version, but NodeMetadata no longer uses that class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see how to do it without big refactoring - BuildVersion currently is essentially a wrapper around Version, so it doesn't seem possible to create it without having a version id within a valid range

final BuildVersion buildVersion = BuildVersion.fromVersionId(version.id());

final NodeMetadata nodeMetadata = new NodeMetadata(nodeId, buildVersion, IndexVersion.current()).upgradeToCurrentVersion();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ public void testMax() {
public void testGetMinimumCompatibleIndexVersion() {
assertThat(IndexVersion.getMinimumCompatibleIndexVersion(7170099), equalTo(IndexVersion.fromId(6000099)));
assertThat(IndexVersion.getMinimumCompatibleIndexVersion(8000099), equalTo(IndexVersion.fromId(7000099)));
assertThat(IndexVersion.getMinimumCompatibleIndexVersion(9000099), equalTo(IndexVersion.fromId(8000099)));
assertThat(IndexVersion.getMinimumCompatibleIndexVersion(10000000), equalTo(IndexVersion.fromId(9000000)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,6 @@
*/
public abstract class MapperTestCase extends MapperServiceTestCase {

public static final IndexVersion DEPRECATED_BOOST_INDEX_VERSION = IndexVersions.V_7_10_0;

protected abstract void minimalMapping(XContentBuilder b) throws IOException;

/**
Expand Down