Improve RepositoryData BwC (elastic#100401)

- Reinstates the few working parts of MultiVersionRepositoryAccessIT - Fixes the version bound for compatibility with v8.9.x - Removes an assertion that blocks fixing BwC properly in 8.11.0 Relates elastic#98454
DaveCTurner · Oct 6, 2023 · 2d513e1 · 2d513e1
1 parent 360aed6
commit 2d513e1
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 6 deletions.
diff --git a/docs/changelog/100401.yaml b/docs/changelog/100401.yaml
@@ -0,0 +1,5 @@
+pr: 100401
+summary: Improve `RepositoryData` BwC
+area: Snapshot/Restore
+type: bug
+issues: []
diff --git a/...ulti-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java b/...ulti-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java
@@ -8,8 +8,8 @@
 
 package org.elasticsearch.upgrades;
 
-import org.apache.lucene.tests.util.LuceneTestCase;
 import org.elasticsearch.ElasticsearchStatusException;
+import org.elasticsearch.Version;
 import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryRequest;
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.Response;
@@ -46,7 +46,6 @@
  * </ul>
  */
 @SuppressWarnings("removal")
-@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/98454")
 public class MultiVersionRepositoryAccessIT extends ESRestTestCase {
 
     private enum TestStep {
@@ -79,6 +78,8 @@ public static TestStep parse(String value) {
 
     private static final TestStep TEST_STEP = TestStep.parse(System.getProperty("tests.rest.suite"));
 
+    private static final Version OLD_CLUSTER_VERSION = Version.fromString(System.getProperty("tests.old_cluster_version"));
+
     @Override
     protected boolean preserveSnapshotsUponCompletion() {
         return true;
@@ -95,6 +96,11 @@ protected boolean preserveTemplatesUponCompletion() {
     }
 
     public void testCreateAndRestoreSnapshot() throws IOException {
+        assumeTrue(
+            "test does not work for downgrades before 8.10.0, see https://github.com/elastic/elasticsearch/issues/98454",
+            OLD_CLUSTER_VERSION.onOrAfter(Version.V_8_10_0)
+        );
+
         final String repoName = getTestName();
         try {
             final int shards = 3;
@@ -141,6 +147,11 @@ public void testCreateAndRestoreSnapshot() throws IOException {
     }
 
     public void testReadOnlyRepo() throws IOException {
+        assumeTrue(
+            "test does not fully work for downgrades before 8.10.0, see https://github.com/elastic/elasticsearch/issues/98454",
+            OLD_CLUSTER_VERSION.onOrAfter(Version.V_8_10_0) || TEST_STEP != TestStep.STEP3_OLD_CLUSTER
+        );
+
         final String repoName = getTestName();
         final int shards = 3;
         final boolean readOnly = TEST_STEP.ordinal() > 1; // only restore from read-only repo in steps 3 and 4
@@ -174,6 +185,11 @@ public void testReadOnlyRepo() throws IOException {
     );
 
     public void testUpgradeMovesRepoToNewMetaVersion() throws IOException {
+        assumeTrue(
+            "test does not work for downgrades before 8.10.0, see https://github.com/elastic/elasticsearch/issues/98454",
+            OLD_CLUSTER_VERSION.onOrAfter(Version.V_8_10_0)
+        );
+
         final String repoName = getTestName();
         try {
             final int shards = 3;

diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java
@@ -19,6 +19,8 @@
 import org.elasticsearch.common.xcontent.XContentParserUtils;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.index.IndexVersion;
+import org.elasticsearch.logging.LogManager;
+import org.elasticsearch.logging.Logger;
 import org.elasticsearch.snapshots.SnapshotId;
 import org.elasticsearch.snapshots.SnapshotInfo;
 import org.elasticsearch.snapshots.SnapshotState;
@@ -740,7 +742,7 @@ public XContentBuilder snapshotsToXContent(final XContentBuilder builder, final
             }
             final IndexVersion version = snapshotDetails.getVersion();
             if (version != null) {
-                if (version.before(IndexVersion.V_8_9_0)) {
+                if (version.before(IndexVersion.V_8_10_0)) {
                     builder.field(VERSION, Version.fromId(version.id()).toString());
                 } else {
                     builder.field(VERSION, version.id());
@@ -953,14 +955,19 @@ private static void parseSnapshots(
         }
     }
 
+    private static final Logger logger = LogManager.getLogger(RepositoryData.class);
+
     private static IndexVersion parseIndexVersion(XContentParser.Token token, XContentParser parser) throws IOException {
         if (token == XContentParser.Token.VALUE_NUMBER) {
             return IndexVersion.fromId(parser.intValue());
         } else {
             XContentParserUtils.ensureExpectedToken(XContentParser.Token.VALUE_STRING, token, parser);
-            Version v = Version.fromString(parser.text());
-            assert v.before(Version.V_8_10_0);
-            return IndexVersion.fromId(v.id);
+            final var versionStr = parser.text();
+            final var versionId = Version.fromString(versionStr).id;
+            if (versionId > 8_11_00_99 && versionId < 8_500_000) {
+                logger.error("found impossible string index version [{}] with id [{}]", versionStr, versionId);
+            }
+            return IndexVersion.fromId(versionId);
         }
     }