Address flakiness of testRemoteCleanupDeleteStale

Signed-off-by: Shivansh Arora <[email protected]>

server/src/internalClusterTest/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()

server/src/main/java/org/opensearch/gateway/remote/model/RemoteClusterMetadataManifest.java

@@ -93,19 +93,27 @@ public String getType() {
 
     @Override
     public String generateBlobFileName() {
-        // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest__<inverted_term>__<inverted_version>__C/P__<inverted__timestamp>__
-        // <codec_version>
-        String blobFileName = String.join(
-            DELIMITER,
-            MANIFEST,
-            RemoteStoreUtils.invertLong(clusterMetadataManifest.getClusterTerm()),
-            RemoteStoreUtils.invertLong(clusterMetadataManifest.getStateVersion()),
-            (clusterMetadataManifest.isCommitted() ? COMMITTED : PUBLISHED),
-            RemoteStoreUtils.invertLong(System.currentTimeMillis()),
-            String.valueOf(clusterMetadataManifest.getCodecVersion())
-            // Keep the codec version at last place only, during we read last place to determine codec version.
-        );
-        this.blobFileName = blobFileName;
+        if (blobFileName != null) {
+            return blobFileName;
+        }
+        // filename is something like: manifest__<inverted_term>__<inverted_version>__C/P__<inverted__timestamp>__<codec_version>
+        if (clusterMetadataManifest != null) {
+            this.blobFileName = String.join(
+                DELIMITER,
+                MANIFEST,
+                RemoteStoreUtils.invertLong(clusterMetadataManifest.getClusterTerm()),
+                RemoteStoreUtils.invertLong(clusterMetadataManifest.getStateVersion()),
+                (clusterMetadataManifest.isCommitted() ? COMMITTED : PUBLISHED),
+                RemoteStoreUtils.invertLong(System.currentTimeMillis()),
+                String.valueOf(clusterMetadataManifest.getCodecVersion())
+                // Keep the codec version at last place only, during we read last place to determine codec version.
+            );
+        } else {
+            String[] pathTokens = getBlobPathTokens();
+            assert pathTokens != null : "path tokens must not be null";
+            assert pathTokens.length > 0 : "path tokens must have at least one segment";
+            this.blobFileName = pathTokens[pathTokens.length - 1];
+        }
         return blobFileName;
     }
 
@@ -131,16 +139,17 @@ 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[] splitName = generateBlobFileName().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);
         }
     }
 

server/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;
@@ -191,6 +193,16 @@ public void testGenerateBlobFileName() {
         assertThat(nameTokens[3], is("C"));
         assertThat(RemoteStoreUtils.invertLong(nameTokens[4]), lessThanOrEqualTo(System.currentTimeMillis()));
         assertThat(nameTokens[5], is(String.valueOf(MANIFEST_CURRENT_CODEC_VERSION)));
+
+        String blobName = "/usr/local/random/path/to/manifest/manifest__1__2__3__4__2";
+        RemoteClusterMetadataManifest remoteObjectForDownload = new RemoteClusterMetadataManifest(
+            blobName,
+            clusterUUID,
+            compressor,
+            namedXContentRegistry
+        );
+        assertEquals("manifest__1__2__3__4__2", remoteObjectForDownload.generateBlobFileName());
+        assertEquals(remoteObjectForDownload.getManifestCodecVersion(), 2);
     }
 
     public void testGetUploadedMetadata() throws IOException {
@@ -236,6 +248,28 @@ 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)