From 084404a2e8fe6a5a9f01abce1b448c8456ff1216 Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Fri, 20 Oct 2023 15:23:21 +0530 Subject: [PATCH 1/5] Fix remote cluster restore for data stream Signed-off-by: Dhwanil Patel --- .../datastream/DataStreamTestCase.java | 21 ++++++++++++++++++- .../RemoteStoreClusterStateRestoreIT.java | 9 ++++++++ .../remote/RemoteClusterStateService.java | 5 ++++- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/datastream/DataStreamTestCase.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/datastream/DataStreamTestCase.java index 50ff76c6b62f3..8da78763509dd 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/datastream/DataStreamTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/datastream/DataStreamTestCase.java @@ -30,13 +30,23 @@ import static org.opensearch.test.OpenSearchIntegTestCase.Scope; import static org.hamcrest.Matchers.is; -@ClusterScope(scope = Scope.TEST, numDataNodes = 2) +@ClusterScope(scope = Scope.TEST, numDataNodes = DataStreamTestCase.DATA_NODE_COUNT) public class DataStreamTestCase extends OpenSearchIntegTestCase { + protected static final int DATA_NODE_COUNT = 2; + + private void resetCluster() { + internalCluster().stopAllNodes(); + internalCluster().startNodes(DATA_NODE_COUNT); + } + public AcknowledgedResponse createDataStream(String name) throws Exception { CreateDataStreamAction.Request request = new CreateDataStreamAction.Request(name); AcknowledgedResponse response = client().admin().indices().createDataStream(request).get(); assertThat(response.isAcknowledged(), is(true)); + if (performRemoteStateRestore()) { + resetCluster(); + } return response; } @@ -67,6 +77,12 @@ public RolloverResponse rolloverDataStream(String name) throws Exception { RolloverResponse response = client().admin().indices().rolloverIndex(request).get(); assertThat(response.isAcknowledged(), is(true)); assertThat(response.isRolledOver(), is(true)); + if (performRemoteStateRestore()) { + String clusterUUIDBefore = clusterService().state().metadata().clusterUUID(); + resetCluster(); + String clusterUUIDAfter = clusterService().state().metadata().clusterUUID(); + assertFalse(clusterUUIDBefore.equals(clusterUUIDAfter)); + } return response; } @@ -110,4 +126,7 @@ public AcknowledgedResponse deleteIndexTemplate(String name) throws Exception { return response; } + protected boolean performRemoteStateRestore() { + return false; + } } diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java index 3a3e293de9b13..479ae16bf040b 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java @@ -9,6 +9,7 @@ package org.opensearch.remotestore; import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; +import org.opensearch.action.admin.indices.datastream.DataStreamRolloverIT; import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequest; import org.opensearch.action.admin.indices.template.put.PutIndexTemplateRequest; import org.opensearch.cluster.ClusterState; @@ -215,6 +216,14 @@ private void validateCurrentMetadata() throws Exception { }); } + public void testDataStreamWithRemoteStateRestore() throws Exception { + new DataStreamRolloverIT() { + protected boolean performRemoteStateRestore() { + return true; + } + }.testDataStreamRollover(); + } + public void testFullClusterRestoreGlobalMetadata() throws Exception { int shardCount = randomIntBetween(1, 2); int replicaCount = 1; 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 b9d06c8fbb1c1..1f3d27b95e4e2 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -725,7 +725,10 @@ public Metadata getLatestMetadata(String clusterName, String clusterUUID) { // Fetch Index Metadata Map indices = getIndexMetadataMap(clusterName, clusterUUID, clusterMetadataManifest.get()); - return Metadata.builder(globalMetadata).indices(indices).build(); + Map indexMetadataMap = new HashMap<>(); + indices.values().forEach(indexMetadata -> { indexMetadataMap.put(indexMetadata.getIndex().getName(), indexMetadata); }); + + return Metadata.builder(globalMetadata).indices(indexMetadataMap).build(); } private Metadata getGlobalMetadata(String clusterName, String clusterUUID, ClusterMetadataManifest clusterMetadataManifest) { From 62c945867baa09d7d6568f8fe868a1c255dfb610 Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Fri, 20 Oct 2023 16:20:52 +0530 Subject: [PATCH 2/5] Fixed UT Signed-off-by: Dhwanil Patel --- .../gateway/remote/RemoteClusterStateServiceTests.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 49b7f0ff8d1a9..7b5daade15a79 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -807,9 +807,9 @@ public void testReadLatestIndexMetadataSuccess() throws IOException { ).getIndices(); assertEquals(indexMetadataMap.size(), 1); - assertEquals(indexMetadataMap.get(index.getUUID()).getIndex().getName(), index.getName()); - assertEquals(indexMetadataMap.get(index.getUUID()).getNumberOfShards(), indexMetadata.getNumberOfShards()); - assertEquals(indexMetadataMap.get(index.getUUID()).getNumberOfReplicas(), indexMetadata.getNumberOfReplicas()); + assertEquals(indexMetadataMap.get(index.getName()).getIndex().getName(), index.getName()); + assertEquals(indexMetadataMap.get(index.getName()).getNumberOfShards(), indexMetadata.getNumberOfShards()); + assertEquals(indexMetadataMap.get(index.getName()).getNumberOfReplicas(), indexMetadata.getNumberOfReplicas()); } public void testMarkLastStateAsCommittedSuccess() throws IOException { From 220d131ce1985abe25b5fb8d3f3c380f1df129b1 Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Sun, 22 Oct 2023 16:48:31 +0530 Subject: [PATCH 3/5] Incorporated comments Signed-off-by: Dhwanil Patel --- .../datastream/DataStreamTestCase.java | 24 +++---------------- .../RemoteStoreClusterStateRestoreIT.java | 2 +- .../opensearch/test/InternalTestCluster.java | 12 ++++++++++ .../test/OpenSearchIntegTestCase.java | 24 +++++++++++++++++++ 4 files changed, 40 insertions(+), 22 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/datastream/DataStreamTestCase.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/datastream/DataStreamTestCase.java index 8da78763509dd..82ab5b0118c0e 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/datastream/DataStreamTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/datastream/DataStreamTestCase.java @@ -30,23 +30,14 @@ import static org.opensearch.test.OpenSearchIntegTestCase.Scope; import static org.hamcrest.Matchers.is; -@ClusterScope(scope = Scope.TEST, numDataNodes = DataStreamTestCase.DATA_NODE_COUNT) +@ClusterScope(scope = Scope.TEST, numDataNodes = 2) public class DataStreamTestCase extends OpenSearchIntegTestCase { - protected static final int DATA_NODE_COUNT = 2; - - private void resetCluster() { - internalCluster().stopAllNodes(); - internalCluster().startNodes(DATA_NODE_COUNT); - } - public AcknowledgedResponse createDataStream(String name) throws Exception { CreateDataStreamAction.Request request = new CreateDataStreamAction.Request(name); AcknowledgedResponse response = client().admin().indices().createDataStream(request).get(); assertThat(response.isAcknowledged(), is(true)); - if (performRemoteStateRestore()) { - resetCluster(); - } + performRemoteStoreTestAction(); return response; } @@ -77,12 +68,7 @@ public RolloverResponse rolloverDataStream(String name) throws Exception { RolloverResponse response = client().admin().indices().rolloverIndex(request).get(); assertThat(response.isAcknowledged(), is(true)); assertThat(response.isRolledOver(), is(true)); - if (performRemoteStateRestore()) { - String clusterUUIDBefore = clusterService().state().metadata().clusterUUID(); - resetCluster(); - String clusterUUIDAfter = clusterService().state().metadata().clusterUUID(); - assertFalse(clusterUUIDBefore.equals(clusterUUIDAfter)); - } + performRemoteStoreTestAction(); return response; } @@ -125,8 +111,4 @@ public AcknowledgedResponse deleteIndexTemplate(String name) throws Exception { assertThat(response.isAcknowledged(), is(true)); return response; } - - protected boolean performRemoteStateRestore() { - return false; - } } diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java index 479ae16bf040b..c74d914b9a66f 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java @@ -216,7 +216,7 @@ private void validateCurrentMetadata() throws Exception { }); } - public void testDataStreamWithRemoteStateRestore() throws Exception { + public void testDataStreamPostRemoteStateRestore() throws Exception { new DataStreamRolloverIT() { protected boolean performRemoteStateRestore() { return true; diff --git a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java index 898e125b94954..dae9b75a62bbb 100644 --- a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java @@ -1871,6 +1871,18 @@ public void stopAllNodes() { } } + /** + * Replace all nodes by stopping all current node and starting new node. + * Used for remote store test cases, where remote state is restored. + */ + public void resetCluster() { + int totalClusterManagerNodes = numClusterManagerNodes(); + int totalDataNodes = numDataNodes(); + stopAllNodes(); + startClusterManagerOnlyNodes(totalClusterManagerNodes); + startDataOnlyNodes(totalDataNodes); + } + private synchronized void startAndPublishNodesAndClients(List nodeAndClients) { if (nodeAndClients.size() > 0) { final int newClusterManagers = (int) nodeAndClients.stream() diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java index c16cc1d2a5fba..586e9e269d881 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java @@ -789,6 +789,30 @@ protected Settings featureFlagSettings() { return featureSettings.build(); } + /** + * Represent if it needs to trigger remote state restore or not. + * For tests with remote store enabled domain, it will be overridden to true. + * + * @return if needs to perform remote state restore or not + */ + protected boolean triggerRemoteStateRestore() { + return false; + } + + /** + * For tests with remote cluster state, it will reset the cluster and cluster state will be + * restored from remote. + */ + protected void performRemoteStoreTestAction() { + if(triggerRemoteStateRestore()) { + String clusterUUIDBefore = clusterService().state().metadata().clusterUUID(); + internalCluster().resetCluster(); + String clusterUUIDAfter = clusterService().state().metadata().clusterUUID(); + // assertion that UUID is changed post restore. + assertFalse(clusterUUIDBefore.equals(clusterUUIDAfter)); + } + } + /** * Creates one or more indices and asserts that the indices are acknowledged. If one of the indices * already exists this method will fail and wipe all the indices created so far. From 11c7d4d42b58b97e4d3c49112e6c74eaa6d47d16 Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Sun, 22 Oct 2023 19:00:59 +0530 Subject: [PATCH 4/5] Fix test framework spotless Signed-off-by: Dhwanil Patel --- .../main/java/org/opensearch/test/OpenSearchIntegTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java index 586e9e269d881..ad27d9834f159 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java @@ -804,7 +804,7 @@ protected boolean triggerRemoteStateRestore() { * restored from remote. */ protected void performRemoteStoreTestAction() { - if(triggerRemoteStateRestore()) { + if (triggerRemoteStateRestore()) { String clusterUUIDBefore = clusterService().state().metadata().clusterUUID(); internalCluster().resetCluster(); String clusterUUIDAfter = clusterService().state().metadata().clusterUUID(); From 9ffc943f514dddb27459b11ab443b06e709c19c1 Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Sun, 22 Oct 2023 23:04:09 +0530 Subject: [PATCH 5/5] minor fix Signed-off-by: Dhwanil Patel --- .../remotestore/RemoteStoreClusterStateRestoreIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java index c74d914b9a66f..ee2178dd9fc9e 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreClusterStateRestoreIT.java @@ -218,7 +218,7 @@ private void validateCurrentMetadata() throws Exception { public void testDataStreamPostRemoteStateRestore() throws Exception { new DataStreamRolloverIT() { - protected boolean performRemoteStateRestore() { + protected boolean triggerRemoteStateRestore() { return true; } }.testDataStreamRollover();