Fix test by using non cluster manager for client

Signed-off-by: Gaurav Bafna <gbbafna@amazon.com>
opensearch-project · Sep 25, 2024 · 7b8b9c6 · 7b8b9c6
1 parent 0e80c3b
commit 7b8b9c6
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 20 deletions.
diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java
@@ -11,6 +11,8 @@
 import org.opensearch.action.DocWriteResponse;
 import org.opensearch.action.admin.cluster.remotestore.restore.RestoreRemoteStoreRequest;
 import org.opensearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse;
+import org.opensearch.action.admin.cluster.snapshots.get.GetSnapshotsRequest;
+import org.opensearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse;
 import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse;
 import org.opensearch.action.admin.indices.delete.DeleteIndexRequest;
 import org.opensearch.action.admin.indices.recovery.RecoveryResponse;
@@ -25,6 +27,7 @@
 import org.opensearch.common.blobstore.BlobPath;
 import org.opensearch.common.io.PathUtils;
 import org.opensearch.common.settings.Settings;
+import org.opensearch.common.unit.TimeValue;
 import org.opensearch.common.util.io.IOUtils;
 import org.opensearch.core.common.unit.ByteSizeUnit;
 import org.opensearch.core.index.Index;
@@ -63,6 +66,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -997,7 +1001,7 @@ public void testConcurrentSnapshotV2CreateOperation() throws InterruptedExceptio
         assertThat(repositoryData.getSnapshotIds().size(), greaterThanOrEqualTo(1));
     }
 
-    public void testConcurrentSnapshotV2CreateOperation_MasterChange() throws InterruptedException, ExecutionException, IOException {
+    public void testConcurrentSnapshotV2CreateOperation_MasterChange() throws Exception {
         internalCluster().startClusterManagerOnlyNode(pinnedTimestampSettings());
         internalCluster().startClusterManagerOnlyNode(pinnedTimestampSettings());
         internalCluster().startClusterManagerOnlyNode(pinnedTimestampSettings());
@@ -1033,14 +1037,16 @@ public void testConcurrentSnapshotV2CreateOperation_MasterChange() throws Interr
         Thread thread = new Thread(() -> {
             try {
                 String snapshotName = "snapshot-earlier-master";
-                CreateSnapshotResponse createSnapshotResponse2 = client().admin()
+                internalCluster().nonClusterManagerClient()
+                    .admin()
                     .cluster()
                     .prepareCreateSnapshot(snapshotRepoName, snapshotName)
                     .setWaitForCompletion(true)
+                    .setMasterNodeTimeout(TimeValue.timeValueSeconds(60))
                     .get();
-            } catch (Exception e) {}
-        });
 
+            } catch (Exception ignored) {}
+        });
         thread.start();
 
         // stop existing master
@@ -1049,17 +1055,18 @@ public void testConcurrentSnapshotV2CreateOperation_MasterChange() throws Interr
 
         // Validate that we have greater one snapshot has been created
         String snapshotName = "new-snapshot";
-        CreateSnapshotResponse createSnapshotResponse2 = client().admin()
-            .cluster()
-            .prepareCreateSnapshot(snapshotRepoName, snapshotName)
-            .setWaitForCompletion(false)
-            .get();
-        Repository repository = internalCluster().getInstance(RepositoriesService.class).repository(snapshotRepoName);
-        PlainActionFuture<RepositoryData> repositoryDataPlainActionFuture = new PlainActionFuture<>();
-        repository.getRepositoryData(repositoryDataPlainActionFuture);
+        try {
+            client().admin().cluster().prepareCreateSnapshot(snapshotRepoName, snapshotName).setWaitForCompletion(true).get();
+        } catch (Exception e) {
+            logger.info("Exception while creating new-snapshot", e);
+        }
 
-        RepositoryData repositoryData = repositoryDataPlainActionFuture.get();
-        assertThat(repositoryData.getSnapshotIds().size(), greaterThanOrEqualTo(1));
+        // Validate that snapshot is present in repository data
+        assertBusy(() -> {
+            GetSnapshotsRequest request = new GetSnapshotsRequest(snapshotRepoName);
+            GetSnapshotsResponse response2 = client().admin().cluster().getSnapshots(request).actionGet();
+            assertThat(response2.getSnapshots().size(), greaterThanOrEqualTo(1));
+        }, 30, TimeUnit.SECONDS);
         thread.join();
     }
 

diff --git a/server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java b/server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java
@@ -349,7 +349,7 @@ public Entry(
             );
         }
 
-        public Entry(
+        private Entry(
             Snapshot snapshot,
             boolean includeGlobalState,
             boolean partial,

diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java
@@ -2916,8 +2916,6 @@ public void getRepositoryData(ActionListener<RepositoryData> listener) {
         }
         final Tuple<Long, BytesReference> cached = latestKnownRepositoryData.get();
         // Fast path loading repository data directly from cache if we're in fully consistent mode and the cache matches up with
-        // Fast path loading repository data directly from cache if we're in fully consistent mode and the cache matches up with
-        // Fast path loading repository data directly from cache if we're in fully consistent mode and the cache matches up with
         // the latest known repository generation
         if (bestEffortConsistency == false && cached != null && cached.v1() == latestKnownRepoGen.get()) {
             try {
@@ -3396,7 +3394,6 @@ public ClusterState execute(ClusterState currentState) {
                                     + "]"
                             );
                         }
-                        logger.info("Done Setting it to {} {}", newGen, newGen);
                         return updateRepositoryGenerationsIfNecessary(
                             stateFilter.apply(
                                 ClusterState.builder(currentState)

diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java b/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java
@@ -470,6 +470,7 @@ public void createSnapshotV2(final CreateSnapshotRequest request, final ActionLi
         final String snapshotName = indexNameExpressionResolver.resolveDateMathExpression(request.snapshot());
 
         Repository repository = repositoriesService.repository(repositoryName);
+        validate(repositoryName, snapshotName);
         repository.executeConsistentStateUpdate(repositoryData -> new ClusterStateUpdateTask(Priority.URGENT) {
             private SnapshotsInProgress.Entry newEntry;
 
@@ -482,8 +483,6 @@ public void createSnapshotV2(final CreateSnapshotRequest request, final ActionLi
             @Override
             public ClusterState execute(ClusterState currentState) {
                 // move to in progress
-                validate(repositoryName, snapshotName);
-
                 snapshotId = new SnapshotId(snapshotName, UUIDs.randomBase64UUID()); // new UUID for the snapshot
                 Repository repository = repositoriesService.repository(repositoryName);