Address flakiness of testRemoteCleanupDeleteStale

Signed-off-by: Shivansh Arora <hishiv@amazon.com>
opensearch-project · Jun 12, 2024 · a8bf4b0 · a8bf4b0
1 parent 2e13e9c
commit a8bf4b0
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 4 deletions.
diff --git a/...nalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManagerIT.java b/...nalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManagerIT.java
@@ -108,6 +108,9 @@ public void testRemoteCleanupDeleteStale() throws Exception {
             .add("cluster-state")
             .add(getClusterState().metadata().clusterUUID());
         BlobPath manifestContainerPath = baseMetadataPath.add("manifest");
+        RemoteClusterStateCleanupManager remoteClusterStateCleanupManager = internalCluster().getClusterManagerNodeInstance(
+            RemoteClusterStateCleanupManager.class
+        );
 
         // set cleanup interval to 100 ms to make the test faster
         ClusterUpdateSettingsResponse response = client().admin()
@@ -117,6 +120,7 @@ public void testRemoteCleanupDeleteStale() throws Exception {
             .get();
 
         assertTrue(response.isAcknowledged());
+        assertBusy(() -> assertEquals(100, remoteClusterStateCleanupManager.getStaleFileDeletionTask().getInterval().getMillis()));
 
         assertBusy(() -> {
             int manifestFiles = repository.blobStore().blobContainer(manifestContainerPath).listBlobsByPrefix("manifest").size();
@@ -128,7 +132,7 @@ public void testRemoteCleanupDeleteStale() throws Exception {
                 "Current number of manifest files: " + manifestFiles,
                 manifestFiles >= RETAINED_MANIFESTS && manifestFiles < RETAINED_MANIFESTS + 2 * SKIP_CLEANUP_STATE_CHANGES
             );
-        }, 500, TimeUnit.MILLISECONDS);
+        });
 
         // disable the clean up to avoid race condition during shutdown
         response = client().admin()

diff --git a/server/src/main/java/org/opensearch/gateway/remote/model/RemoteClusterMetadataManifest.java b/server/src/main/java/org/opensearch/gateway/remote/model/RemoteClusterMetadataManifest.java
@@ -25,6 +25,7 @@
 import java.util.List;
 
 import static org.opensearch.gateway.remote.RemoteClusterStateUtils.DELIMITER;
+import static org.opensearch.gateway.remote.RemoteClusterStateUtils.PATH_DELIMITER;
 
 /**
  * Wrapper class for uploading/downloading {@link ClusterMetadataManifest} to/from remote blob store
@@ -131,16 +132,18 @@ public ClusterMetadataManifest deserialize(final InputStream inputStream) throws
         return blobStoreFormat.deserialize(blobName, getNamedXContentRegistry(), Streams.readFully(inputStream));
     }
 
-    private int getManifestCodecVersion() {
+    // package private for testing
+    int getManifestCodecVersion() {
         assert blobName != null;
-        String[] splitName = blobName.split(DELIMITER);
+        String[] pathTokens = getBlobPathTokens();
+        String[] splitName = pathTokens[pathTokens.length - 1].split(DELIMITER);
         if (splitName.length == SPLITTED_MANIFEST_FILE_LENGTH) {
             return Integer.parseInt(splitName[splitName.length - 1]); // Last value would be codec version.
         } else if (splitName.length < SPLITTED_MANIFEST_FILE_LENGTH) { // Where codec is not part of file name, i.e. default codec version 0
             // is used.
             return ClusterMetadataManifest.CODEC_V0;
         } else {
-            throw new IllegalArgumentException("Manifest file name is corrupted");
+            throw new IllegalArgumentException("Manifest file name is corrupted : " + blobName);
         }
     }
 

diff --git a/...src/test/java/org/opensearch/gateway/remote/model/RemoteClusterMetadataManifestTests.java b/...src/test/java/org/opensearch/gateway/remote/model/RemoteClusterMetadataManifestTests.java
@@ -41,6 +41,8 @@
 import java.util.stream.Stream;
 
 import static java.util.stream.Collectors.toList;
+import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V0;
+import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V2;
 import static org.opensearch.gateway.remote.model.RemoteClusterMetadataManifest.MANIFEST;
 import static org.opensearch.gateway.remote.model.RemoteClusterMetadataManifest.MANIFEST_CURRENT_CODEC_VERSION;
 import static org.hamcrest.Matchers.greaterThan;
@@ -236,6 +238,26 @@ public void testSerDe() throws IOException {
         assertThrows(IllegalArgumentException.class, () -> invalidRemoteObject.deserialize(new ByteArrayInputStream(new byte[0])));
     }
 
+    public void testGetManifestCodecVersion() {
+        String manifestFileWithDelimiterInPath = "123456789012_test-cluster/cluster-state/dsgYj10__Nkso7/manifest/manifest__9223372036854775806__9223372036854775804__C__9223370319103329556__2";
+        RemoteClusterMetadataManifest remoteManifestForDownload = new RemoteClusterMetadataManifest(
+            manifestFileWithDelimiterInPath,
+            clusterUUID,
+            compressor,
+            namedXContentRegistry
+        );
+        assertEquals(CODEC_V2, remoteManifestForDownload.getManifestCodecVersion());
+
+        String v0ManifestFileWithDelimiterInPath = "123456789012_test-cluster/cluster-state/dsgYj10__Nkso7/manifest/manifest__9223372036854775806__9223372036854775804__C__9223370319103329556";
+        RemoteClusterMetadataManifest remoteManifestV0ForDownload = new RemoteClusterMetadataManifest(
+            v0ManifestFileWithDelimiterInPath,
+            clusterUUID,
+            compressor,
+            namedXContentRegistry
+        );
+        assertEquals(CODEC_V0, remoteManifestV0ForDownload.getManifestCodecVersion());
+    }
+
     private ClusterMetadataManifest getClusterMetadataManifest() {
         return ClusterMetadataManifest.builder()
             .opensearchVersion(Version.CURRENT)