From 8a5f22a1aac3868b38ebe3d8aa285ae0cc5aa532 Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Tue, 4 Jul 2023 10:03:11 +0530 Subject: [PATCH 1/2] Fix flaky tests in restore flow using snapshot interop Signed-off-by: Sachin Kale --- .../snapshots/RestoreSnapshotIT.java | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java index 9f492bbaee01a..ba7782bbd75fc 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java @@ -184,7 +184,7 @@ public void testRestoreRemoteStoreIndicesWithoutRemoteTranslog() throws IOExcept public void testRestoreOperationsShallowCopyEnabled(boolean remoteTranslogEnabled) throws IOException, ExecutionException, InterruptedException { internalCluster().startClusterManagerOnlyNode(); - final String primaryNode = internalCluster().startNode(); + String primary = internalCluster().startDataOnlyNode(); String indexName1 = "testindex1"; String indexName2 = "testindex2"; String snapshotRepoName = "test-restore-snapshot-repo"; @@ -216,7 +216,7 @@ public void testRestoreOperationsShallowCopyEnabled(boolean remoteTranslogEnable indexDocuments(client, indexName2, numDocsInIndex2); ensureGreen(indexName1, indexName2); - final String secondNode = internalCluster().startNode(); + internalCluster().startDataOnlyNode(); logger.info("--> snapshot"); CreateSnapshotResponse createSnapshotResponse = client.admin() .cluster() @@ -273,7 +273,7 @@ public void testRestoreOperationsShallowCopyEnabled(boolean remoteTranslogEnable assertDocsPresentInIndex(client, restoredIndexName2, numDocsInIndex2); // deleting data for restoredIndexName1 and restoring from remote store. - internalCluster().stopRandomNode(InternalTestCluster.nameFilter(primaryNodeName(restoredIndexName1))); + internalCluster().stopRandomNode(InternalTestCluster.nameFilter(primary)); ensureRed(restoredIndexName1); assertAcked(client().admin().indices().prepareClose(restoredIndexName1)); client().admin() @@ -282,6 +282,8 @@ public void testRestoreOperationsShallowCopyEnabled(boolean remoteTranslogEnable ensureYellowAndNoInitializingShards(restoredIndexName1); ensureGreen(restoredIndexName1); assertDocsPresentInIndex(client(), restoredIndexName1, numDocsInIndex1); + // Re-initialize client to make sure we are not using client from stopped node. + client = client(); // indexing some new docs and validating indexDocuments(client, restoredIndexName1, numDocsInIndex1, numDocsInIndex1 + 2); ensureGreen(restoredIndexName1); @@ -348,7 +350,7 @@ public void testRestoreOperationsShallowCopyEnabled(boolean remoteTranslogEnable public void testRestoreInSameRemoteStoreEnabledIndex() throws IOException { internalCluster().startClusterManagerOnlyNode(); - internalCluster().startNode(); + String primary = internalCluster().startDataOnlyNode(); String indexName1 = "testindex1"; String indexName2 = "testindex2"; String snapshotRepoName = "test-restore-snapshot-repo"; @@ -378,7 +380,7 @@ public void testRestoreInSameRemoteStoreEnabledIndex() throws IOException { indexDocuments(client, indexName2, numDocsInIndex2); ensureGreen(indexName1, indexName2); - final String secondNode = internalCluster().startNode(); + internalCluster().startDataOnlyNode(); logger.info("--> snapshot"); CreateSnapshotResponse createSnapshotResponse = client.admin() .cluster() @@ -435,13 +437,15 @@ public void testRestoreInSameRemoteStoreEnabledIndex() throws IOException { assertDocsPresentInIndex(client, restoredIndexName2, numDocsInIndex2); // deleting data for restoredIndexName1 and restoring from remote store. - internalCluster().stopRandomNode(InternalTestCluster.nameFilter(primaryNodeName(indexName1))); + internalCluster().stopRandomNode(InternalTestCluster.nameFilter(primary)); ensureRed(indexName1); assertAcked(client().admin().indices().prepareClose(indexName1)); client().admin().cluster().restoreRemoteStore(new RestoreRemoteStoreRequest().indices(indexName1), PlainActionFuture.newFuture()); ensureYellowAndNoInitializingShards(indexName1); ensureGreen(indexName1); assertDocsPresentInIndex(client(), indexName1, numDocsInIndex1); + // Re-initialize client to make sure we are not using client from stopped node. + client = client(); // indexing some new docs and validating indexDocuments(client, indexName1, numDocsInIndex1, numDocsInIndex1 + 2); ensureGreen(indexName1); @@ -450,7 +454,7 @@ public void testRestoreInSameRemoteStoreEnabledIndex() throws IOException { public void testRestoreShallowCopySnapshotWithDifferentRepo() throws IOException { internalCluster().startClusterManagerOnlyNode(); - final String primaryNode = internalCluster().startNode(); + String primary = internalCluster().startDataOnlyNode(); String indexName1 = "testindex1"; String indexName2 = "testindex2"; String snapshotRepoName = "test-restore-snapshot-repo"; @@ -479,7 +483,7 @@ public void testRestoreShallowCopySnapshotWithDifferentRepo() throws IOException indexDocuments(client, indexName2, numDocsInIndex2); ensureGreen(indexName1, indexName2); - final String secondNode = internalCluster().startNode(); + internalCluster().startDataOnlyNode(); logger.info("--> snapshot"); CreateSnapshotResponse createSnapshotResponse = client.admin() @@ -513,13 +517,15 @@ public void testRestoreShallowCopySnapshotWithDifferentRepo() throws IOException assertDocsPresentInIndex(client(), restoredIndexName1, numDocsInIndex1); // deleting data for restoredIndexName1 and restoring from remote store. - internalCluster().stopRandomNode(InternalTestCluster.nameFilter(primaryNodeName(restoredIndexName1))); + internalCluster().stopRandomNode(InternalTestCluster.nameFilter(primary)); assertAcked(client().admin().indices().prepareClose(restoredIndexName1)); client().admin() .cluster() .restoreRemoteStore(new RestoreRemoteStoreRequest().indices(restoredIndexName1), PlainActionFuture.newFuture()); ensureYellowAndNoInitializingShards(restoredIndexName1); ensureGreen(restoredIndexName1); + // Re-initialize client to make sure we are not using client from stopped node. + client = client(); // indexing some new docs and validating assertDocsPresentInIndex(client, restoredIndexName1, numDocsInIndex1); indexDocuments(client, restoredIndexName1, numDocsInIndex1, numDocsInIndex1 + 2); From 81ca831da44914d7a9d685b41b1625716da983bb Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Tue, 4 Jul 2023 10:44:00 +0530 Subject: [PATCH 2/2] Address PR comments Signed-off-by: Sachin Kale --- .../snapshots/RestoreSnapshotIT.java | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java index ba7782bbd75fc..e362b7f61e8e6 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java @@ -183,7 +183,7 @@ public void testRestoreRemoteStoreIndicesWithoutRemoteTranslog() throws IOExcept public void testRestoreOperationsShallowCopyEnabled(boolean remoteTranslogEnabled) throws IOException, ExecutionException, InterruptedException { - internalCluster().startClusterManagerOnlyNode(); + String clusterManagerNode = internalCluster().startClusterManagerOnlyNode(); String primary = internalCluster().startDataOnlyNode(); String indexName1 = "testindex1"; String indexName2 = "testindex2"; @@ -275,15 +275,15 @@ public void testRestoreOperationsShallowCopyEnabled(boolean remoteTranslogEnable // deleting data for restoredIndexName1 and restoring from remote store. internalCluster().stopRandomNode(InternalTestCluster.nameFilter(primary)); ensureRed(restoredIndexName1); - assertAcked(client().admin().indices().prepareClose(restoredIndexName1)); - client().admin() + // Re-initialize client to make sure we are not using client from stopped node. + client = client(clusterManagerNode); + assertAcked(client.admin().indices().prepareClose(restoredIndexName1)); + client.admin() .cluster() .restoreRemoteStore(new RestoreRemoteStoreRequest().indices(restoredIndexName1), PlainActionFuture.newFuture()); ensureYellowAndNoInitializingShards(restoredIndexName1); ensureGreen(restoredIndexName1); assertDocsPresentInIndex(client(), restoredIndexName1, numDocsInIndex1); - // Re-initialize client to make sure we are not using client from stopped node. - client = client(); // indexing some new docs and validating indexDocuments(client, restoredIndexName1, numDocsInIndex1, numDocsInIndex1 + 2); ensureGreen(restoredIndexName1); @@ -302,7 +302,7 @@ public void testRestoreOperationsShallowCopyEnabled(boolean remoteTranslogEnable assertEquals(restoreSnapshotResponse3.status(), RestStatus.ACCEPTED); ensureGreen(restoredIndexName1Seg); - GetIndexResponse getIndexResponse = client().admin() + GetIndexResponse getIndexResponse = client.admin() .indices() .getIndex(new GetIndexRequest().indices(restoredIndexName1Seg).includeDefaults(true)) .get(); @@ -333,7 +333,7 @@ public void testRestoreOperationsShallowCopyEnabled(boolean remoteTranslogEnable assertEquals(restoreSnapshotResponse4.status(), RestStatus.ACCEPTED); ensureGreen(restoredIndexName1Doc); - getIndexResponse = client().admin() + getIndexResponse = client.admin() .indices() .getIndex(new GetIndexRequest().indices(restoredIndexName1Doc).includeDefaults(true)) .get(); @@ -349,7 +349,7 @@ public void testRestoreOperationsShallowCopyEnabled(boolean remoteTranslogEnable } public void testRestoreInSameRemoteStoreEnabledIndex() throws IOException { - internalCluster().startClusterManagerOnlyNode(); + String clusterManagerNode = internalCluster().startClusterManagerOnlyNode(); String primary = internalCluster().startDataOnlyNode(); String indexName1 = "testindex1"; String indexName2 = "testindex2"; @@ -439,13 +439,13 @@ public void testRestoreInSameRemoteStoreEnabledIndex() throws IOException { // deleting data for restoredIndexName1 and restoring from remote store. internalCluster().stopRandomNode(InternalTestCluster.nameFilter(primary)); ensureRed(indexName1); - assertAcked(client().admin().indices().prepareClose(indexName1)); - client().admin().cluster().restoreRemoteStore(new RestoreRemoteStoreRequest().indices(indexName1), PlainActionFuture.newFuture()); + // Re-initialize client to make sure we are not using client from stopped node. + client = client(clusterManagerNode); + assertAcked(client.admin().indices().prepareClose(indexName1)); + client.admin().cluster().restoreRemoteStore(new RestoreRemoteStoreRequest().indices(indexName1), PlainActionFuture.newFuture()); ensureYellowAndNoInitializingShards(indexName1); ensureGreen(indexName1); assertDocsPresentInIndex(client(), indexName1, numDocsInIndex1); - // Re-initialize client to make sure we are not using client from stopped node. - client = client(); // indexing some new docs and validating indexDocuments(client, indexName1, numDocsInIndex1, numDocsInIndex1 + 2); ensureGreen(indexName1); @@ -453,7 +453,7 @@ public void testRestoreInSameRemoteStoreEnabledIndex() throws IOException { } public void testRestoreShallowCopySnapshotWithDifferentRepo() throws IOException { - internalCluster().startClusterManagerOnlyNode(); + String clusterManagerNode = internalCluster().startClusterManagerOnlyNode(); String primary = internalCluster().startDataOnlyNode(); String indexName1 = "testindex1"; String indexName2 = "testindex2"; @@ -518,14 +518,14 @@ public void testRestoreShallowCopySnapshotWithDifferentRepo() throws IOException // deleting data for restoredIndexName1 and restoring from remote store. internalCluster().stopRandomNode(InternalTestCluster.nameFilter(primary)); - assertAcked(client().admin().indices().prepareClose(restoredIndexName1)); - client().admin() + // Re-initialize client to make sure we are not using client from stopped node. + client = client(clusterManagerNode); + assertAcked(client.admin().indices().prepareClose(restoredIndexName1)); + client.admin() .cluster() .restoreRemoteStore(new RestoreRemoteStoreRequest().indices(restoredIndexName1), PlainActionFuture.newFuture()); ensureYellowAndNoInitializingShards(restoredIndexName1); ensureGreen(restoredIndexName1); - // Re-initialize client to make sure we are not using client from stopped node. - client = client(); // indexing some new docs and validating assertDocsPresentInIndex(client, restoredIndexName1, numDocsInIndex1); indexDocuments(client, restoredIndexName1, numDocsInIndex1, numDocsInIndex1 + 2);