Address PR comments

Signed-off-by: bansvaru <[email protected]>
opensearch-project · Oct 27, 2023 · b39b696 · b39b696
1 parent 5e40fbc
commit b39b696
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 14 deletions.
diff --git a/...internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java b/...internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java
@@ -30,6 +30,7 @@
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.ExecutionException;
@@ -85,16 +86,22 @@ public void testFullClusterRestore() throws Exception {
         // Step - 1 index some data to generate files in remote directory
         Map<String, Long> indexStats = initialTestSetup(shardCount, replicaCount, dataNodeCount, 1);
         String prevClusterUUID = clusterService().state().metadata().clusterUUID();
-        long prevClusterVersion = clusterService().state().version();
+        long prevClusterStateVersion = clusterService().state().version();
 
         // Step - 2 Replace all nodes in the cluster with new nodes. This ensures new cluster state doesn't have previous index metadata
         resetCluster(dataNodeCount, clusterManagerNodeCount);
 
         String newClusterUUID = clusterService().state().metadata().clusterUUID();
         assert !Objects.equals(newClusterUUID, prevClusterUUID) : "cluster restart not successful. cluster uuid is same";
 
-        assert prevClusterVersion < clusterService().state().version() : "ClusterState version is not restored";
-        // Step - 3 Trigger full cluster restore and validate
+        // Step - 3 validate cluster state restored
+        long newClusterStateVersion = clusterService().state().version();
+        assert prevClusterStateVersion < newClusterStateVersion : String.format(
+            Locale.ROOT,
+            "ClusterState version is not restored. previousClusterVersion: [%s] is greater than current [%s]",
+            prevClusterStateVersion,
+            newClusterStateVersion
+        );
         validateMetadata(List.of(INDEX_NAME));
         verifyRedIndicesAndTriggerRestore(indexStats, INDEX_NAME, true);
 
@@ -124,7 +131,7 @@ public void testFullClusterRestoreDoesntFailWithConflictingLocalState() throws E
         // index some data to generate files in remote directory
         Map<String, Long> indexStats = initialTestSetup(shardCount, replicaCount, dataNodeCount, 1);
         String prevClusterUUID = clusterService().state().metadata().clusterUUID();
-        long prevClusterVersion = clusterService().state().version();
+        long prevClusterStateVersion = clusterService().state().version();
 
         // stop all nodes
         internalCluster().stopAllNodes();
@@ -160,7 +167,14 @@ public Settings onNodeStopped(String nodeName) {
         newClusterUUID = clusterService().state().metadata().clusterUUID();
         assert !Objects.equals(newClusterUUID, ClusterState.UNKNOWN_UUID) : "cluster restart not successful. cluster uuid is still unknown";
         assert !Objects.equals(newClusterUUID, prevClusterUUID) : "cluster restart not successful. cluster uuid is same";
-        assert prevClusterVersion < clusterService().state().version() : "ClusterState version is not restored";
+
+        long newClusterStateVersion = clusterService().state().version();
+        assert prevClusterStateVersion < newClusterStateVersion : String.format(
+            Locale.ROOT,
+            "ClusterState version is not restored. previousClusterVersion: [%s] is greater than current [%s]",
+            prevClusterStateVersion,
+            newClusterStateVersion
+        );
         validateMetadata(List.of(INDEX_NAME));
 
         // start data nodes to trigger index data recovery
@@ -185,16 +199,22 @@ public void testFullClusterRestoreMultipleIndices() throws Exception {
         updateIndexBlock(true, secondIndexName);
 
         String prevClusterUUID = clusterService().state().metadata().clusterUUID();
-        long prevClusterVersion = clusterService().state().version();
+        long prevClusterStateVersion = clusterService().state().version();
 
         // Step - 2 Replace all nodes in the cluster with new nodes. This ensures new cluster state doesn't have previous index metadata
         resetCluster(dataNodeCount, clusterManagerNodeCount);
 
         String newClusterUUID = clusterService().state().metadata().clusterUUID();
         assert !Objects.equals(newClusterUUID, prevClusterUUID) : "cluster restart not successful. cluster uuid is same";
-        assert prevClusterVersion < clusterService().state().version() : "ClusterState version is not restored";
 
-        // Step - 3 Trigger full cluster restore
+        // Step - 3 validate cluster state restored
+        long newClusterStateVersion = clusterService().state().version();
+        assert prevClusterStateVersion < newClusterStateVersion : String.format(
+            Locale.ROOT,
+            "ClusterState version is not restored. previousClusterVersion: [%s] is greater than current [%s]",
+            prevClusterStateVersion,
+            newClusterStateVersion
+        );
         validateMetadata(List.of(INDEX_NAME, secondIndexName));
         verifyRedIndicesAndTriggerRestore(indexStats, INDEX_NAME, false);
         verifyRedIndicesAndTriggerRestore(indexStats2, secondIndexName, false);
@@ -246,6 +266,7 @@ public void testRemoteStateFullRestart() throws Exception {
 
         Map<String, Long> indexStats = initialTestSetup(shardCount, replicaCount, dataNodeCount, clusterManagerNodeCount);
         String prevClusterUUID = clusterService().state().metadata().clusterUUID();
+        long prevClusterStateVersion = clusterService().state().version();
         // Delete index metadata file in remote
         try {
             Files.move(
@@ -264,6 +285,14 @@ public void testRemoteStateFullRestart() throws Exception {
         ensureGreen(INDEX_NAME);
         String newClusterUUID = clusterService().state().metadata().clusterUUID();
         assert Objects.equals(newClusterUUID, prevClusterUUID) : "Full restart not successful. cluster uuid has changed";
+
+        long newClusterStateVersion = clusterService().state().version();
+        assert prevClusterStateVersion < newClusterStateVersion : String.format(
+            Locale.ROOT,
+            "ClusterState version is not restored. previousClusterVersion: [%s] is greater than current [%s]",
+            prevClusterStateVersion,
+            newClusterStateVersion
+        );
         validateCurrentMetadata();
         verifyRedIndicesAndTriggerRestore(indexStats, INDEX_NAME, true);
     }
@@ -316,6 +345,7 @@ public void testFullClusterRestoreGlobalMetadata() throws Exception {
         // Step - 1 index some data to generate files in remote directory
         Map<String, Long> indexStats = initialTestSetup(shardCount, replicaCount, dataNodeCount, 1);
         String prevClusterUUID = clusterService().state().metadata().clusterUUID();
+        long prevClusterStateVersion = clusterService().state().version();
 
         // Create global metadata - register a custom repo
         Path repoPath = registerCustomRepository();
@@ -335,8 +365,16 @@ public void testFullClusterRestoreGlobalMetadata() throws Exception {
         String newClusterUUID = clusterService().state().metadata().clusterUUID();
         assert !Objects.equals(newClusterUUID, prevClusterUUID) : "cluster restart not successful. cluster uuid is same";
 
-        // Step - 3 Trigger full cluster restore and validate
-        // validateCurrentMetadata();
+        // Step - 3 validate cluster state restored
+        long newClusterStateVersion = clusterService().state().version();
+        assert prevClusterStateVersion < newClusterStateVersion : String.format(
+            Locale.ROOT,
+            "ClusterState version is not restored. previousClusterVersion: [%s] is greater than current [%s]",
+            prevClusterStateVersion,
+            newClusterStateVersion
+        );
+
+        validateCurrentMetadata();
         assertEquals(Integer.valueOf(34), SETTING_CLUSTER_MAX_SHARDS_PER_NODE.get(clusterService().state().metadata().settings()));
         assertEquals(true, SETTING_READ_ONLY_SETTING.get(clusterService().state().metadata().settings()));
         assertTrue(clusterService().state().blocks().hasGlobalBlock(CLUSTER_READ_ONLY_BLOCK));

diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java
@@ -743,10 +743,11 @@ public void testReadGlobalMetadata() throws IOException {
         final ClusterState clusterState = generateClusterStateWithGlobalMetadata().nodes(nodesWithLocalNodeClusterManager()).build();
         remoteClusterStateService.start();
 
+        long prevClusterStateVersion = 13L;
         final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder()
             .indices(List.of())
             .clusterTerm(1L)
-            .stateVersion(1L)
+            .stateVersion(prevClusterStateVersion)
             .stateUUID("state-uuid")
             .clusterUUID("cluster-uuid")
             .codecVersion(MANIFEST_CURRENT_CODEC_VERSION)
@@ -759,12 +760,20 @@ public void testReadGlobalMetadata() throws IOException {
         Metadata expactedMetadata = Metadata.builder().persistentSettings(Settings.builder().put("readonly", true).build()).build();
         mockBlobContainerForGlobalMetadata(mockBlobStoreObjects(), expectedManifest, expactedMetadata);
 
-        Metadata metadata = remoteClusterStateService.getLatestClusterState(
+        ClusterState newClusterState = remoteClusterStateService.getLatestClusterState(
             clusterState.getClusterName().value(),
             clusterState.metadata().clusterUUID()
-        ).getMetadata();
+        );
 
-        assertTrue(Metadata.isGlobalStateEquals(metadata, expactedMetadata));
+        assertTrue(Metadata.isGlobalStateEquals(newClusterState.getMetadata(), expactedMetadata));
+
+        long newClusterStateVersion = newClusterState.getVersion();
+        assert prevClusterStateVersion == newClusterStateVersion : String.format(
+            Locale.ROOT,
+            "ClusterState version is not restored. previousClusterVersion: [%s] is not equal to current [%s]",
+            prevClusterStateVersion,
+            newClusterStateVersion
+        );
     }
 
     public void testReadGlobalMetadataIOException() throws IOException {