From 0405a9f4642cb9f0d97363307f48351269cac08d Mon Sep 17 00:00:00 2001 From: bansvaru Date: Mon, 23 Oct 2023 15:18:13 +0530 Subject: [PATCH 1/6] Restore ClusterState version during remote state restore Signed-off-by: bansvaru --- .../remote/RemoteClusterStateServiceIT.java | 19 +++++++++++- .../remote/RemoteClusterStateService.java | 13 +++------ .../recovery/RemoteStoreRestoreService.java | 21 +++++++++++++- .../RemoteClusterStateServiceTests.java | 29 +++++++++++++------ 4 files changed, 62 insertions(+), 20 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java index 59eef3c06844b..19cd8d2962970 100644 --- a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java @@ -21,7 +21,9 @@ import java.nio.charset.StandardCharsets; import java.util.Base64; +import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; @@ -90,9 +92,24 @@ public void testFullClusterRestoreStaleDelete() throws Exception { assertEquals(10, repository.blobStore().blobContainer(baseMetadataPath.add("manifest")).listBlobsByPrefix("manifest").size()); + Optional clusterMetadataManifest = remoteClusterStateService.getLatestClusterMetadataManifest( + clusterService().state().getClusterName().value(), + getClusterState().metadata().clusterUUID() + ); + if (clusterMetadataManifest.isEmpty()) { + throw new IllegalStateException( + String.format( + Locale.ROOT, + "Latest cluster metadata manifest is not present for the provided clusterUUID: %s", + getClusterState().metadata().clusterUUID() + ) + ); + } + Map indexMetadataMap = remoteClusterStateService.getLatestMetadata( cluster().getClusterName(), - getClusterState().metadata().clusterUUID() + getClusterState().metadata().clusterUUID(), + clusterMetadataManifest.get() ).getIndices(); assertEquals(0, indexMetadataMap.values().stream().findFirst().get().getNumberOfReplicas()); assertEquals(shardCount, indexMetadataMap.values().stream().findFirst().get().getNumberOfShards()); diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index 329ebd0dcd2b8..0dde9ff354cb3 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -760,19 +760,14 @@ private IndexMetadata getIndexMetadata(String clusterName, String clusterUUID, U * @param clusterName name of the cluster * @return {@link IndexMetadata} */ - public Metadata getLatestMetadata(String clusterName, String clusterUUID) { + public Metadata getLatestMetadata(String clusterName, String clusterUUID, ClusterMetadataManifest clusterMetadataManifest) { start(); - Optional clusterMetadataManifest = getLatestClusterMetadataManifest(clusterName, clusterUUID); - if (!clusterMetadataManifest.isPresent()) { - throw new IllegalStateException( - String.format(Locale.ROOT, "Latest cluster metadata manifest is not present for the provided clusterUUID: %s", clusterUUID) - ); - } + // Fetch Global Metadata - Metadata globalMetadata = getGlobalMetadata(clusterName, clusterUUID, clusterMetadataManifest.get()); + Metadata globalMetadata = getGlobalMetadata(clusterName, clusterUUID, clusterMetadataManifest); // Fetch Index Metadata - Map indices = getIndexMetadataMap(clusterName, clusterUUID, clusterMetadataManifest.get()); + Map indices = getIndexMetadataMap(clusterName, clusterUUID, clusterMetadataManifest); Map indexMetadataMap = new HashMap<>(); indices.values().forEach(indexMetadata -> { indexMetadataMap.put(indexMetadata.getIndex().getName(), indexMetadata); }); diff --git a/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java b/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java index 9541d13421e27..93d4d730e3dea 100644 --- a/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java +++ b/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java @@ -32,6 +32,7 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.core.action.ActionListener; import org.opensearch.core.index.shard.ShardId; +import org.opensearch.gateway.remote.ClusterMetadataManifest; import org.opensearch.gateway.remote.RemoteClusterStateService; import org.opensearch.indices.ShardLimitValidator; import org.opensearch.repositories.IndexId; @@ -149,7 +150,24 @@ public RemoteRestoreResult restore( if (currentState.metadata().clusterUUID().equals(restoreClusterUUID)) { throw new IllegalArgumentException("clusterUUID to restore from should be different from current cluster UUID"); } - remoteMetadata = remoteClusterStateService.getLatestMetadata(currentState.getClusterName().value(), restoreClusterUUID); + Optional clusterMetadataManifest = remoteClusterStateService.getLatestClusterMetadataManifest( + currentState.getClusterName().value(), + restoreClusterUUID + ); + if (clusterMetadataManifest.isEmpty()) { + throw new IllegalStateException( + String.format( + Locale.ROOT, + "Latest cluster metadata manifest is not present for the provided clusterUUID: %s", + restoreClusterUUID + ) + ); + } + remoteMetadata = remoteClusterStateService.getLatestMetadata( + currentState.getClusterName().value(), + restoreClusterUUID, + clusterMetadataManifest.get() + ); remoteMetadata.getIndices().values().forEach(indexMetadata -> { indexMetadataMap.put(indexMetadata.getIndex().getName(), new Tuple<>(true, indexMetadata)); }); @@ -255,6 +273,7 @@ private RemoteRestoreResult executeRestore( } private void restoreGlobalMetadata(Metadata.Builder mdBuilder, Metadata remoteMetadata) { + mdBuilder.version(remoteMetadata.version()); if (remoteMetadata.persistentSettings() != null) { Settings settings = remoteMetadata.persistentSettings(); clusterService.getClusterSettings().validateUpdate(settings); diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index 5a43864f40c0c..f4bad6d2d7584 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -659,9 +659,11 @@ public void testReadLatestMetadataManifestSuccessButNoIndexMetadata() throws IOE remoteClusterStateService.start(); assertEquals( - remoteClusterStateService.getLatestMetadata(clusterState.getClusterName().value(), clusterState.metadata().clusterUUID()) - .getIndices() - .size(), + remoteClusterStateService.getLatestMetadata( + clusterState.getClusterName().value(), + clusterState.metadata().clusterUUID(), + expectedManifest + ).getIndices().size(), 0 ); } @@ -688,8 +690,11 @@ public void testReadLatestMetadataManifestSuccessButIndexMetadataFetchIOExceptio remoteClusterStateService.start(); Exception e = assertThrows( IllegalStateException.class, - () -> remoteClusterStateService.getLatestMetadata(clusterState.getClusterName().value(), clusterState.metadata().clusterUUID()) - .getIndices() + () -> remoteClusterStateService.getLatestMetadata( + clusterState.getClusterName().value(), + clusterState.metadata().clusterUUID(), + expectedManifest + ).getIndices() ); assertEquals(e.getMessage(), "Error while downloading IndexMetadata - " + uploadedIndexMetadata.getUploadedFilename()); } @@ -752,7 +757,8 @@ public void testReadGlobalMetadata() throws IOException { Metadata metadata = remoteClusterStateService.getLatestMetadata( clusterState.getClusterName().value(), - clusterState.metadata().clusterUUID() + clusterState.metadata().clusterUUID(), + expectedManifest ); assertTrue(Metadata.isGlobalStateEquals(metadata, expactedMetadata)); @@ -787,7 +793,11 @@ public void testReadGlobalMetadataIOException() throws IOException { remoteClusterStateService.start(); Exception e = assertThrows( IllegalStateException.class, - () -> remoteClusterStateService.getLatestMetadata(clusterState.getClusterName().value(), clusterState.metadata().clusterUUID()) + () -> remoteClusterStateService.getLatestMetadata( + clusterState.getClusterName().value(), + clusterState.metadata().clusterUUID(), + expectedManifest + ) ); assertEquals(e.getMessage(), "Error while downloading Global Metadata - " + globalIndexMetadataName); } @@ -818,7 +828,7 @@ public void testReadLatestIndexMetadataSuccess() throws IOException { .nodeId("nodeA") .opensearchVersion(VersionUtils.randomOpenSearchVersion(random())) .previousClusterUUID("prev-cluster-uuid") - .globalMetadataFileName("global-metadata-file") + // .globalMetadataFileName("global-metadata-file") .codecVersion(ClusterMetadataManifest.CODEC_V0) .build(); @@ -826,7 +836,8 @@ public void testReadLatestIndexMetadataSuccess() throws IOException { Map indexMetadataMap = remoteClusterStateService.getLatestMetadata( clusterState.getClusterName().value(), - clusterState.metadata().clusterUUID() + clusterState.metadata().clusterUUID(), + expectedManifest ).getIndices(); assertEquals(indexMetadataMap.size(), 1); From 978a0736842d2a1b388983c779fffda5150f05fe Mon Sep 17 00:00:00 2001 From: bansvaru Date: Tue, 24 Oct 2023 10:57:58 +0530 Subject: [PATCH 2/6] refactor code for better extensibility in future Signed-off-by: bansvaru --- .../remote/RemoteClusterStateServiceIT.java | 7 ++-- .../remote/RemoteClusterStateService.java | 20 +++++++---- .../recovery/RemoteStoreRestoreService.java | 34 +++++------------- .../RemoteClusterStateServiceTests.java | 36 ++++++++----------- 4 files changed, 40 insertions(+), 57 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java index 19cd8d2962970..48824d5b29b06 100644 --- a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java @@ -106,11 +106,10 @@ public void testFullClusterRestoreStaleDelete() throws Exception { ); } - Map indexMetadataMap = remoteClusterStateService.getLatestMetadata( + Map indexMetadataMap = remoteClusterStateService.getLatestClusterState( cluster().getClusterName(), - getClusterState().metadata().clusterUUID(), - clusterMetadataManifest.get() - ).getIndices(); + getClusterState().metadata().clusterUUID() + ).getMetadata().getIndices(); assertEquals(0, indexMetadataMap.values().stream().findFirst().get().getNumberOfReplicas()); assertEquals(shardCount, indexMetadataMap.values().stream().findFirst().get().getNumberOfShards()); } diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index 0dde9ff354cb3..2ce3a6bcbf6ed 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -754,25 +754,33 @@ private IndexMetadata getIndexMetadata(String clusterName, String clusterUUID, U } /** - * Fetch latest metadata from remote cluster state including global metadata and index metadata + * Fetch latest ClusterState from remote, including global metadata, index metadata and cluster state version * * @param clusterUUID uuid of cluster state to refer to in remote * @param clusterName name of the cluster * @return {@link IndexMetadata} */ - public Metadata getLatestMetadata(String clusterName, String clusterUUID, ClusterMetadataManifest clusterMetadataManifest) { + public ClusterState getLatestClusterState(String clusterName, String clusterUUID) { start(); - + Optional clusterMetadataManifest = getLatestClusterMetadataManifest(clusterName, clusterUUID); + if (clusterMetadataManifest.isEmpty()) { + throw new IllegalStateException( + String.format(Locale.ROOT, "Latest cluster metadata manifest is not present for the provided clusterUUID: %s", clusterUUID) + ); + } // Fetch Global Metadata - Metadata globalMetadata = getGlobalMetadata(clusterName, clusterUUID, clusterMetadataManifest); + Metadata globalMetadata = getGlobalMetadata(clusterName, clusterUUID, clusterMetadataManifest.get()); // Fetch Index Metadata - Map indices = getIndexMetadataMap(clusterName, clusterUUID, clusterMetadataManifest); + Map indices = getIndexMetadataMap(clusterName, clusterUUID, clusterMetadataManifest.get()); Map indexMetadataMap = new HashMap<>(); indices.values().forEach(indexMetadata -> { indexMetadataMap.put(indexMetadata.getIndex().getName(), indexMetadata); }); - return Metadata.builder(globalMetadata).indices(indexMetadataMap).build(); + return ClusterState.builder(ClusterState.EMPTY_STATE) + .version(clusterMetadataManifest.get().getStateVersion()) + .metadata(Metadata.builder(globalMetadata).indices(indexMetadataMap).build()) + .build(); } private Metadata getGlobalMetadata(String clusterName, String clusterUUID, ClusterMetadataManifest clusterMetadataManifest) { diff --git a/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java b/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java index 93d4d730e3dea..05fdea5650ddb 100644 --- a/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java +++ b/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java @@ -32,7 +32,6 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.core.action.ActionListener; import org.opensearch.core.index.shard.ShardId; -import org.opensearch.gateway.remote.ClusterMetadataManifest; import org.opensearch.gateway.remote.RemoteClusterStateService; import org.opensearch.indices.ShardLimitValidator; import org.opensearch.repositories.IndexId; @@ -139,7 +138,7 @@ public RemoteRestoreResult restore( String[] indexNames ) { Map> indexMetadataMap = new HashMap<>(); - Metadata remoteMetadata = null; + ClusterState remoteState = null; boolean metadataFromRemoteStore = (restoreClusterUUID == null || restoreClusterUUID.isEmpty() || restoreClusterUUID.isBlank()) == false; @@ -150,25 +149,8 @@ public RemoteRestoreResult restore( if (currentState.metadata().clusterUUID().equals(restoreClusterUUID)) { throw new IllegalArgumentException("clusterUUID to restore from should be different from current cluster UUID"); } - Optional clusterMetadataManifest = remoteClusterStateService.getLatestClusterMetadataManifest( - currentState.getClusterName().value(), - restoreClusterUUID - ); - if (clusterMetadataManifest.isEmpty()) { - throw new IllegalStateException( - String.format( - Locale.ROOT, - "Latest cluster metadata manifest is not present for the provided clusterUUID: %s", - restoreClusterUUID - ) - ); - } - remoteMetadata = remoteClusterStateService.getLatestMetadata( - currentState.getClusterName().value(), - restoreClusterUUID, - clusterMetadataManifest.get() - ); - remoteMetadata.getIndices().values().forEach(indexMetadata -> { + remoteState = remoteClusterStateService.getLatestClusterState(currentState.getClusterName().value(), restoreClusterUUID); + remoteState.getMetadata().getIndices().values().forEach(indexMetadata -> { indexMetadataMap.put(indexMetadata.getIndex().getName(), new Tuple<>(true, indexMetadata)); }); } catch (Exception e) { @@ -194,7 +176,7 @@ public RemoteRestoreResult restore( } } } - return executeRestore(currentState, indexMetadataMap, restoreAllShards, remoteMetadata); + return executeRestore(currentState, indexMetadataMap, restoreAllShards, remoteState); } /** @@ -208,7 +190,7 @@ private RemoteRestoreResult executeRestore( ClusterState currentState, Map> indexMetadataMap, boolean restoreAllShards, - Metadata remoteMetadata + ClusterState remoteState ) { final String restoreUUID = UUIDs.randomBase64UUID(); List indicesToBeRestored = new ArrayList<>(); @@ -258,8 +240,9 @@ private RemoteRestoreResult executeRestore( totalShards += updatedIndexMetadata.getNumberOfShards(); } - if (remoteMetadata != null) { - restoreGlobalMetadata(mdBuilder, remoteMetadata); + if (remoteState != null) { + restoreGlobalMetadata(mdBuilder, remoteState.getMetadata()); + builder.version(remoteState.version()); } RestoreInfo restoreInfo = new RestoreInfo("remote_store", indicesToBeRestored, totalShards, totalShards); @@ -273,7 +256,6 @@ private RemoteRestoreResult executeRestore( } private void restoreGlobalMetadata(Metadata.Builder mdBuilder, Metadata remoteMetadata) { - mdBuilder.version(remoteMetadata.version()); if (remoteMetadata.persistentSettings() != null) { Settings settings = remoteMetadata.persistentSettings(); clusterService.getClusterSettings().validateUpdate(settings); diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index f4bad6d2d7584..2432b3b6663aa 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -659,11 +659,10 @@ public void testReadLatestMetadataManifestSuccessButNoIndexMetadata() throws IOE remoteClusterStateService.start(); assertEquals( - remoteClusterStateService.getLatestMetadata( - clusterState.getClusterName().value(), - clusterState.metadata().clusterUUID(), - expectedManifest - ).getIndices().size(), + remoteClusterStateService.getLatestClusterState(clusterState.getClusterName().value(), clusterState.metadata().clusterUUID()) + .getMetadata() + .getIndices() + .size(), 0 ); } @@ -690,11 +689,10 @@ public void testReadLatestMetadataManifestSuccessButIndexMetadataFetchIOExceptio remoteClusterStateService.start(); Exception e = assertThrows( IllegalStateException.class, - () -> remoteClusterStateService.getLatestMetadata( + () -> remoteClusterStateService.getLatestClusterState( clusterState.getClusterName().value(), - clusterState.metadata().clusterUUID(), - expectedManifest - ).getIndices() + clusterState.metadata().clusterUUID() + ).getMetadata().getIndices() ); assertEquals(e.getMessage(), "Error while downloading IndexMetadata - " + uploadedIndexMetadata.getUploadedFilename()); } @@ -755,11 +753,10 @@ public void testReadGlobalMetadata() throws IOException { Metadata expactedMetadata = Metadata.builder().persistentSettings(Settings.builder().put("readonly", true).build()).build(); mockBlobContainerForGlobalMetadata(mockBlobStoreObjects(), expectedManifest, expactedMetadata); - Metadata metadata = remoteClusterStateService.getLatestMetadata( + Metadata metadata = remoteClusterStateService.getLatestClusterState( clusterState.getClusterName().value(), - clusterState.metadata().clusterUUID(), - expectedManifest - ); + clusterState.metadata().clusterUUID() + ).getMetadata(); assertTrue(Metadata.isGlobalStateEquals(metadata, expactedMetadata)); } @@ -793,10 +790,9 @@ public void testReadGlobalMetadataIOException() throws IOException { remoteClusterStateService.start(); Exception e = assertThrows( IllegalStateException.class, - () -> remoteClusterStateService.getLatestMetadata( + () -> remoteClusterStateService.getLatestClusterState( clusterState.getClusterName().value(), - clusterState.metadata().clusterUUID(), - expectedManifest + clusterState.metadata().clusterUUID() ) ); assertEquals(e.getMessage(), "Error while downloading Global Metadata - " + globalIndexMetadataName); @@ -828,17 +824,15 @@ public void testReadLatestIndexMetadataSuccess() throws IOException { .nodeId("nodeA") .opensearchVersion(VersionUtils.randomOpenSearchVersion(random())) .previousClusterUUID("prev-cluster-uuid") - // .globalMetadataFileName("global-metadata-file") .codecVersion(ClusterMetadataManifest.CODEC_V0) .build(); mockBlobContainer(mockBlobStoreObjects(), expectedManifest, Map.of(index.getUUID(), indexMetadata)); - Map indexMetadataMap = remoteClusterStateService.getLatestMetadata( + Map indexMetadataMap = remoteClusterStateService.getLatestClusterState( clusterState.getClusterName().value(), - clusterState.metadata().clusterUUID(), - expectedManifest - ).getIndices(); + clusterState.metadata().clusterUUID() + ).getMetadata().getIndices(); assertEquals(indexMetadataMap.size(), 1); assertEquals(indexMetadataMap.get(index.getName()).getIndex().getName(), index.getName()); From c530d2e5ee13f99300bfcadb46b30051b23ecff6 Mon Sep 17 00:00:00 2001 From: bansvaru Date: Wed, 25 Oct 2023 10:16:26 +0530 Subject: [PATCH 3/6] update ITs to test for ClusterState version restore Signed-off-by: bansvaru --- .../remotestore/RemoteStoreClusterStateRestoreIT.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java index e9afd6d36bb87..795d75f87a947 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java @@ -85,6 +85,7 @@ public void testFullClusterRestore() throws Exception { // Step - 1 index some data to generate files in remote directory Map indexStats = initialTestSetup(shardCount, replicaCount, dataNodeCount, 1); String prevClusterUUID = clusterService().state().metadata().clusterUUID(); + long prevClusterVersion = 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); @@ -92,9 +93,11 @@ public void testFullClusterRestore() throws Exception { 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 validateMetadata(List.of(INDEX_NAME)); verifyRedIndicesAndTriggerRestore(indexStats, INDEX_NAME, true); + } /** @@ -121,6 +124,7 @@ public void testFullClusterRestoreDoesntFailWithConflictingLocalState() throws E // index some data to generate files in remote directory Map indexStats = initialTestSetup(shardCount, replicaCount, dataNodeCount, 1); String prevClusterUUID = clusterService().state().metadata().clusterUUID(); + long prevClusterVersion = clusterService().state().version(); // stop all nodes internalCluster().stopAllNodes(); @@ -156,6 +160,7 @@ 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"; validateMetadata(List.of(INDEX_NAME)); // start data nodes to trigger index data recovery @@ -180,12 +185,14 @@ public void testFullClusterRestoreMultipleIndices() throws Exception { updateIndexBlock(true, secondIndexName); String prevClusterUUID = clusterService().state().metadata().clusterUUID(); + long prevClusterVersion = 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 validateMetadata(List.of(INDEX_NAME, secondIndexName)); From d2d97bf5b22710b72a2faf0582ba33bbfcdf1dab Mon Sep 17 00:00:00 2001 From: bansvaru Date: Wed, 25 Oct 2023 11:02:15 +0530 Subject: [PATCH 4/6] remove stale code Signed-off-by: bansvaru --- .../remote/RemoteClusterStateServiceIT.java | 16 ---------------- .../recovery/RemoteStoreRestoreService.java | 2 ++ 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java index 48824d5b29b06..b5589d964e8ea 100644 --- a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java @@ -21,9 +21,7 @@ import java.nio.charset.StandardCharsets; import java.util.Base64; -import java.util.Locale; import java.util.Map; -import java.util.Optional; import java.util.stream.Collectors; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; @@ -92,20 +90,6 @@ public void testFullClusterRestoreStaleDelete() throws Exception { assertEquals(10, repository.blobStore().blobContainer(baseMetadataPath.add("manifest")).listBlobsByPrefix("manifest").size()); - Optional clusterMetadataManifest = remoteClusterStateService.getLatestClusterMetadataManifest( - clusterService().state().getClusterName().value(), - getClusterState().metadata().clusterUUID() - ); - if (clusterMetadataManifest.isEmpty()) { - throw new IllegalStateException( - String.format( - Locale.ROOT, - "Latest cluster metadata manifest is not present for the provided clusterUUID: %s", - getClusterState().metadata().clusterUUID() - ) - ); - } - Map indexMetadataMap = remoteClusterStateService.getLatestClusterState( cluster().getClusterName(), getClusterState().metadata().clusterUUID() diff --git a/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java b/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java index 05fdea5650ddb..4d17ac44e8061 100644 --- a/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java +++ b/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java @@ -242,6 +242,8 @@ private RemoteRestoreResult executeRestore( if (remoteState != null) { restoreGlobalMetadata(mdBuilder, remoteState.getMetadata()); + // Restore ClusterState version + logger.info("Restoring ClusterState with Remote State version [{}]", remoteState.version()); builder.version(remoteState.version()); } From ca4e51f63a186da1e25cedad74650d12dc61d0c7 Mon Sep 17 00:00:00 2001 From: bansvaru Date: Wed, 25 Oct 2023 16:54:12 +0530 Subject: [PATCH 5/6] add changelog Signed-off-by: bansvaru --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b40878066960a..ee91bb9f73f4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Remote cluster state] Restore global metadata from remote store when local state is lost after quorum loss ([#10404](https://github.com/opensearch-project/OpenSearch/pull/10404)) - [AdmissionControl] Added changes for AdmissionControl Interceptor and AdmissionControlService for RateLimiting ([#9286](https://github.com/opensearch-project/OpenSearch/pull/9286)) - GHA to verify checklist items completion in PR descriptions ([#10800](https://github.com/opensearch-project/OpenSearch/pull/10800)) +- [Remote cluster state] Restore cluster state version during remote state auto restore ([#10853](https://github.com/opensearch-project/OpenSearch/pull/10853)) ### Dependencies - Bump `log4j-core` from 2.18.0 to 2.19.0 From b39b6960a3296f60761966b395123f83dbc21e6a Mon Sep 17 00:00:00 2001 From: bansvaru Date: Fri, 27 Oct 2023 14:09:24 +0530 Subject: [PATCH 6/6] Address PR comments Signed-off-by: bansvaru --- .../RemoteStoreClusterStateRestoreIT.java | 58 +++++++++++++++---- .../RemoteClusterStateServiceTests.java | 17 ++++-- 2 files changed, 61 insertions(+), 14 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java index 795d75f87a947..c61e2ec6e4f6c 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java +++ b/server/src/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,7 +86,7 @@ public void testFullClusterRestore() throws Exception { // Step - 1 index some data to generate files in remote directory Map 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); @@ -93,8 +94,14 @@ public void testFullClusterRestore() throws Exception { 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 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 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 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 index 3fa4a4e2683a2..4efd1b8a62970 100644 --- 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 {