From 1730b9dffd8e3d210a2bcc855f7ac3ca8f05a85c Mon Sep 17 00:00:00 2001 From: bansvaru Date: Tue, 24 Oct 2023 10:57:58 +0530 Subject: [PATCH] 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 f8a94b9b23d8b..3c31672ad318f 100644 --- a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateServiceIT.java @@ -102,11 +102,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 7d1719beb555c..d628a264c322f 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -712,22 +712,30 @@ 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()); - return Metadata.builder(globalMetadata).version(clusterMetadataManifest.getStateVersion()).indices(indices).build(); + return ClusterState.builder(ClusterState.EMPTY_STATE) + .version(clusterMetadataManifest.get().getStateVersion()) + .metadata(Metadata.builder(globalMetadata).indices(indices).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 c036bdba4e264..e8ab15d01a273 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -655,11 +655,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 ); } @@ -686,11 +685,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()); } @@ -751,11 +749,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)); } @@ -789,10 +786,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); @@ -824,17 +820,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.getUUID()).getIndex().getName(), index.getName());