Browse Source

Ignore BlobStoreIndexShardSnapshot#indexVersion (#96450)

We track the commit generation as `indexVersion` in each
`BlobStoreIndexShardSnapshot`, apparently for "compatibility with v1.0".
This value has been unused for a long time, so with this commit we stop
tracking it and drop it from the metadata blob. This has no bwc
implications because the metadata is parsed in a manner that does not
require this field to be present.
David Turner 2 years ago
parent
commit
4617095486

+ 15 - 23
server/src/main/java/org/elasticsearch/index/snapshots/IndexShardSnapshotStatus.java

@@ -8,6 +8,7 @@
 
 package org.elasticsearch.index.snapshots;
 
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.repositories.ShardGeneration;
 import org.elasticsearch.repositories.ShardSnapshotResult;
 import org.elasticsearch.snapshots.AbortedSnapshotException;
@@ -61,7 +62,6 @@ public class IndexShardSnapshotStatus {
     private long totalSize;
     private long incrementalSize;
     private long processedSize;
-    private long indexVersion;
     private String failure;
 
     private IndexShardSnapshotStatus(
@@ -115,18 +115,20 @@ public class IndexShardSnapshotStatus {
         return asCopy();
     }
 
-    public synchronized Copy moveToFinalize(final long indexVersion) {
-        if (stage.compareAndSet(Stage.STARTED, Stage.FINALIZE)) {
-            this.indexVersion = indexVersion;
-        } else if (isAborted()) {
-            throw new AbortedSnapshotException();
-        } else {
-            assert false : "Should not try to move stage [" + stage.get() + "] to [FINALIZE]";
-            throw new IllegalStateException(
-                "Unable to move the shard snapshot status to [FINALIZE]: " + "expecting [STARTED] but got [" + stage.get() + "]"
-            );
-        }
-        return asCopy();
+    public synchronized Copy moveToFinalize() {
+        final var prevStage = stage.compareAndExchange(Stage.STARTED, Stage.FINALIZE);
+        return switch (prevStage) {
+            case STARTED -> asCopy();
+            case ABORTED -> throw new AbortedSnapshotException();
+            default -> {
+                final var message = Strings.format(
+                    "Unable to move the shard snapshot status to [FINALIZE]: expecting [STARTED] but got [%s]",
+                    prevStage
+                );
+                assert false : message;
+                throw new IllegalStateException(message);
+            }
+        };
     }
 
     public synchronized void moveToDone(final long endTime, final ShardSnapshotResult shardSnapshotResult) {
@@ -206,7 +208,6 @@ public class IndexShardSnapshotStatus {
             incrementalSize,
             totalSize,
             processedSize,
-            indexVersion,
             failure
         );
     }
@@ -262,7 +263,6 @@ public class IndexShardSnapshotStatus {
         private final long totalSize;
         private final long processedSize;
         private final long incrementalSize;
-        private final long indexVersion;
         private final String failure;
 
         public Copy(
@@ -275,7 +275,6 @@ public class IndexShardSnapshotStatus {
             final long incrementalSize,
             final long totalSize,
             final long processedSize,
-            final long indexVersion,
             final String failure
         ) {
             this.stage = stage;
@@ -287,7 +286,6 @@ public class IndexShardSnapshotStatus {
             this.totalSize = totalSize;
             this.processedSize = processedSize;
             this.incrementalSize = incrementalSize;
-            this.indexVersion = indexVersion;
             this.failure = failure;
         }
 
@@ -327,10 +325,6 @@ public class IndexShardSnapshotStatus {
             return processedSize;
         }
 
-        public long getIndexVersion() {
-            return indexVersion;
-        }
-
         public String getFailure() {
             return failure;
         }
@@ -356,8 +350,6 @@ public class IndexShardSnapshotStatus {
                 + totalSize
                 + ", processedSize="
                 + processedSize
-                + ", indexVersion="
-                + indexVersion
                 + ", failure='"
                 + failure
                 + '\''

+ 8 - 14
server/src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java

@@ -360,8 +360,6 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
      */
     private final String snapshot;
 
-    private final long indexVersion;
-
     private final long startTime;
 
     private final long time;
@@ -376,7 +374,6 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
      * Constructs new shard snapshot metadata from snapshot metadata
      *
      * @param snapshot              snapshot name
-     * @param indexVersion          index version
      * @param indexFiles            list of files in the shard
      * @param startTime             snapshot start time
      * @param time                  snapshot running time
@@ -385,7 +382,6 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
      */
     public BlobStoreIndexShardSnapshot(
         String snapshot,
-        long indexVersion,
         List<FileInfo> indexFiles,
         long startTime,
         long time,
@@ -393,9 +389,7 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
         long incrementalSize
     ) {
         assert snapshot != null;
-        assert indexVersion >= 0;
         this.snapshot = snapshot;
-        this.indexVersion = indexVersion;
         this.indexFiles = List.copyOf(indexFiles);
         this.startTime = startTime;
         this.time = time;
@@ -412,7 +406,7 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
      * @param time               time it took to create the clone
      */
     public BlobStoreIndexShardSnapshot asClone(String targetSnapshotName, long startTime, long time) {
-        return new BlobStoreIndexShardSnapshot(targetSnapshotName, indexVersion, indexFiles, startTime, time, 0, 0);
+        return new BlobStoreIndexShardSnapshot(targetSnapshotName, indexFiles, startTime, time, 0, 0);
     }
 
     /**
@@ -480,7 +474,6 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
     }
 
     private static final String NAME = "name";
-    private static final String INDEX_VERSION = "index_version";
     private static final String START_TIME = "start_time";
     private static final String TIME = "time";
     private static final String FILES = "files";
@@ -490,13 +483,16 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
     private static final String INCREMENTAL_SIZE = "total_size";
 
     private static final ParseField PARSE_NAME = new ParseField(NAME);
-    private static final ParseField PARSE_INDEX_VERSION = new ParseField(INDEX_VERSION, "index-version");
     private static final ParseField PARSE_START_TIME = new ParseField(START_TIME);
     private static final ParseField PARSE_TIME = new ParseField(TIME);
     private static final ParseField PARSE_INCREMENTAL_FILE_COUNT = new ParseField(INCREMENTAL_FILE_COUNT);
     private static final ParseField PARSE_INCREMENTAL_SIZE = new ParseField(INCREMENTAL_SIZE);
     private static final ParseField PARSE_FILES = new ParseField(FILES);
 
+    // pre-8.9.0 versions included this (unused) field so we must accept its existence
+    private static final String INDEX_VERSION = "index_version";
+    private static final ParseField PARSE_INDEX_VERSION = new ParseField(INDEX_VERSION, "index-version");
+
     /**
      * Serializes shard snapshot metadata info into JSON
      *
@@ -506,7 +502,7 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.field(NAME, snapshot);
-        builder.field(INDEX_VERSION, indexVersion);
+        builder.field(INDEX_VERSION, 0); // pre-8.9.0 versions require this field to be present and non-negative
         builder.field(START_TIME, startTime);
         builder.field(TIME, time);
         builder.field(INCREMENTAL_FILE_COUNT, incrementalFileCount);
@@ -527,7 +523,6 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
      */
     public static BlobStoreIndexShardSnapshot fromXContent(XContentParser parser) throws IOException {
         String snapshot = null;
-        long indexVersion = -1;
         long startTime = 0;
         long time = 0;
         int incrementalFileCount = 0;
@@ -547,8 +542,8 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
                 if (PARSE_NAME.match(currentFieldName, parser.getDeprecationHandler())) {
                     snapshot = parser.text();
                 } else if (PARSE_INDEX_VERSION.match(currentFieldName, parser.getDeprecationHandler())) {
-                    // The index-version is needed for backward compatibility with v 1.0
-                    indexVersion = parser.longValue();
+                    // pre-8.9.0 versions included this (unused) field so we must accept its existence
+                    parser.longValue();
                 } else if (PARSE_START_TIME.match(currentFieldName, parser.getDeprecationHandler())) {
                     startTime = parser.longValue();
                 } else if (PARSE_TIME.match(currentFieldName, parser.getDeprecationHandler())) {
@@ -573,7 +568,6 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
 
         return new BlobStoreIndexShardSnapshot(
             snapshot,
-            indexVersion,
             indexFiles == null ? List.of() : indexFiles,
             startTime,
             time,

+ 1 - 2
server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java

@@ -2866,13 +2866,12 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp
 
             final StepListener<Collection<Void>> allFilesUploadedListener = new StepListener<>();
             allFilesUploadedListener.whenComplete(v -> {
-                final IndexShardSnapshotStatus.Copy lastSnapshotStatus = snapshotStatus.moveToFinalize(snapshotIndexCommit.getGeneration());
+                final IndexShardSnapshotStatus.Copy lastSnapshotStatus = snapshotStatus.moveToFinalize();
 
                 // now create and write the commit point
                 logger.trace("[{}] [{}] writing shard snapshot file", shardId, snapshotId);
                 final BlobStoreIndexShardSnapshot blobStoreIndexShardSnapshot = new BlobStoreIndexShardSnapshot(
                     snapshotId.getName(),
-                    lastSnapshotStatus.getIndexVersion(),
                     indexCommitPointFiles,
                     lastSnapshotStatus.getStartTime(),
                     threadPool.absoluteTimeInMillis() - lastSnapshotStatus.getStartTime(),

+ 1 - 1
x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/store/SearchableSnapshotDirectoryStatsTests.java

@@ -644,7 +644,7 @@ public class SearchableSnapshotDirectoryStatsTests extends AbstractSearchableSna
             Version.CURRENT.luceneVersion().toString()
         );
         final List<FileInfo> files = List.of(new FileInfo(blobName, metadata, ByteSizeValue.ofBytes(fileContent.length)));
-        final BlobStoreIndexShardSnapshot snapshot = new BlobStoreIndexShardSnapshot(snapshotId.getName(), 0L, files, 0L, 0L, 0, 0L);
+        final BlobStoreIndexShardSnapshot snapshot = new BlobStoreIndexShardSnapshot(snapshotId.getName(), files, 0L, 0L, 0, 0L);
         final Path shardDir = randomShardPath(shardId);
         final ShardPath shardPath = new ShardPath(false, shardDir, shardDir, shardId);
         final Path cacheDir = Files.createDirectories(resolveSnapshotCache(shardDir).resolve(snapshotId.getUUID()));

+ 1 - 1
x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/store/SearchableSnapshotDirectoryTests.java

@@ -740,7 +740,7 @@ public class SearchableSnapshotDirectoryTests extends AbstractSearchableSnapshot
                 );
             }
 
-            final BlobStoreIndexShardSnapshot snapshot = new BlobStoreIndexShardSnapshot("_snapshot", 0L, randomFiles, 0L, 0L, 0, 0L);
+            final BlobStoreIndexShardSnapshot snapshot = new BlobStoreIndexShardSnapshot("_snapshot", randomFiles, 0L, 0L, 0, 0L);
             final BlobContainer blobContainer = new FsBlobContainer(
                 new FsBlobStore(randomIntBetween(1, 8) * 1024, shardSnapshotDir, true),
                 BlobPath.EMPTY,

+ 0 - 2
x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/store/input/CachedBlobContainerIndexInputTests.java

@@ -82,7 +82,6 @@ public class CachedBlobContainerIndexInputTests extends AbstractSearchableSnapsh
 
                 final BlobStoreIndexShardSnapshot snapshot = new BlobStoreIndexShardSnapshot(
                     snapshotId.getName(),
-                    0L,
                     List.of(new BlobStoreIndexShardSnapshot.FileInfo(blobName, metadata, ByteSizeValue.ofBytes(partSize))),
                     0L,
                     0L,
@@ -200,7 +199,6 @@ public class CachedBlobContainerIndexInputTests extends AbstractSearchableSnapsh
 
             final BlobStoreIndexShardSnapshot snapshot = new BlobStoreIndexShardSnapshot(
                 snapshotId.getName(),
-                0L,
                 List.of(new BlobStoreIndexShardSnapshot.FileInfo(blobName, metadata, ByteSizeValue.ofBytes(input.length + 1))),
                 0L,
                 0L,

+ 1 - 1
x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/store/input/FrozenIndexInputTests.java

@@ -138,7 +138,7 @@ public class FrozenIndexInputTests extends AbstractSearchableSnapshotsTestCase {
         ) {
             super(
                 () -> TestUtils.singleBlobContainer(fileInfo.partName(0), fileData),
-                () -> new BlobStoreIndexShardSnapshot("_snapshot_id", 0L, List.of(fileInfo), 0L, 0L, 0, 0L),
+                () -> new BlobStoreIndexShardSnapshot("_snapshot_id", List.of(fileInfo), 0L, 0L, 0, 0L),
                 new TestUtils.SimpleBlobStoreCacheService(),
                 "_repository",
                 snapshotId,