review feedback

Author: ywangd

qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/RunningSnapshotIT.java

@@ -12,12 +12,15 @@
 import com.carrotsearch.randomizedtesting.annotations.Name;
 
 import org.elasticsearch.client.Request;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.test.rest.ObjectPath;
 
 import java.io.IOException;
 import java.util.Collection;
 
+import static org.elasticsearch.upgrades.SnapshotBasedRecoveryIT.indexDocs;
+import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.not;
 
@@ -34,18 +37,22 @@ public void testRunningSnapshotCompleteAfterUpgrade() throws Exception {
         final var nodeIds = getNodesInfo(client()).keySet();
 
         if (isOldCluster()) {
+            registerRepository(repositoryName, "fs", randomBoolean(), Settings.builder().put("location", "backup").build());
             // create an index to have one shard per node
             createIndex(indexName, indexSettings(3, 0).put("index.routing.allocation.total_shards_per_node", 1).build());
             ensureGreen(indexName);
+            if (randomBoolean()) {
+                indexDocs(indexName, between(10, 50));
+            }
             flush(indexName, true);
             // Signal shutdown to prevent snapshot from being completed
             putShutdownMetadata(nodeIds);
-            registerRepository(repositoryName, "fs", randomBoolean(), Settings.builder().put("location", "backup").build());
             createSnapshot(repositoryName, snapshotName, false);
             assertRunningSnapshot(repositoryName, snapshotName);
         } else {
             if (isUpgradedCluster()) {
                 deleteShutdownMetadata(nodeIds);
+                assertNoShutdownMetadata(nodeIds);
                 ensureGreen(indexName);
                 assertBusy(() -> assertCompletedSnapshot(repositoryName, snapshotName));
             } else {
@@ -73,6 +80,13 @@ private void deleteShutdownMetadata(Collection<String> nodeIds) throws IOExcepti
         }
     }
 
+    private void assertNoShutdownMetadata(Collection<String> nodeIds) throws IOException {
+        final ObjectPath responsePath = assertOKAndCreateObjectPath(
+            client().performRequest(new Request("GET", "/_nodes/" + Strings.collectionToCommaDelimitedString(nodeIds) + "/shutdown"))
+        );
+        assertThat(responsePath.evaluate("nodes"), empty());
+    }
+
     private void assertRunningSnapshot(String repositoryName, String snapshotName) throws IOException {
         final Request request = new Request("GET", "/_snapshot/" + repositoryName + "/_current");
         final ObjectPath responsePath = assertOKAndCreateObjectPath(client().performRequest(request));

qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/SnapshotBasedRecoveryIT.java

@@ -233,7 +233,7 @@ private static Map<String, Object> search(String index, QueryBuilder query) thro
         return responseAsMap;
     }
 
-    private void indexDocs(String indexName, int numDocs) throws IOException {
+    static void indexDocs(String indexName, int numDocs) throws IOException {
         final StringBuilder bulkBody = new StringBuilder();
         for (int i = 0; i < numDocs; i++) {
             bulkBody.append("{\"index\":{\"_id\":\"").append(i).append("\"}}\n");

server/src/test/java/org/elasticsearch/snapshots/SnapshotsInProgressSerializationTests.java

@@ -91,13 +91,13 @@ public void testSerializationBwc() throws IOException {
         final var oldVersion = TransportVersionUtils.getPreviousVersion(TransportVersions.PROJECT_ID_IN_SNAPSHOT);
         final BytesStreamOutput out = new BytesStreamOutput();
         out.setTransportVersion(oldVersion);
-        final Custom orig = createTestInstance(() -> randomSnapshot(ProjectId.DEFAULT));
-        orig.writeTo(out);
+        final Custom original = createTestInstance(() -> randomSnapshot(ProjectId.DEFAULT));
+        original.writeTo(out);
 
         final var in = out.bytes().streamInput();
         in.setTransportVersion(oldVersion);
         final SnapshotsInProgress fromStream = new SnapshotsInProgress(in);
-        assertThat(fromStream, equalTo(orig));
+        assertThat(fromStream, equalTo(original));
     }
 
     public void testDiffSerializationBwc() throws IOException {