Change filenames for IndexMetadata and Manifest (#10557)

* Change filenames for IndexMetadata and Manifest

Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

CHANGELOG.md

@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Provide service accounts tokens to extensions ([#9618](https://github.com/opensearch-project/OpenSearch/pull/9618))
 - [Admission control] Add enhancements to FS stats to include read/write time, queue size and IO time ([#10541](https://github.com/opensearch-project/OpenSearch/pull/10541))
 - [Admission control] Add Resource usage collector service and resource usage tracker  ([#9890](https://github.com/opensearch-project/OpenSearch/pull/9890))
+- Change file names for remote cluster state ([#10557](https://github.com/opensearch-project/OpenSearch/pull/10557))
 
 ### Dependencies
 - Bump `log4j-core` from 2.18.0 to 2.19.0

server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java

@@ -120,6 +120,9 @@ public class RemoteClusterStateService implements Closeable {
 
     private final AtomicBoolean deleteStaleMetadataRunning = new AtomicBoolean(false);
 
+    public static final int INDEX_METADATA_CURRENT_CODEC_VERSION = 1;
+    public static final int MANIFEST_CURRENT_CODEC_VERSION = 1;
+
     public RemoteClusterStateService(
         String nodeId,
         Supplier<RepositoriesService> repositoriesService,
@@ -426,7 +429,7 @@ private ClusterMetadataManifest uploadManifest(
         boolean committed
     ) throws IOException {
         synchronized (this) {
-            final String manifestFileName = getManifestFileName(clusterState.term(), clusterState.version());
+            final String manifestFileName = getManifestFileName(clusterState.term(), clusterState.version(), committed);
             final ClusterMetadataManifest manifest = new ClusterMetadataManifest(
                 clusterState.term(),
                 clusterState.getVersion(),
@@ -488,22 +491,30 @@ private void setSlowWriteLoggingThreshold(TimeValue slowWriteLoggingThreshold) {
         this.slowWriteLoggingThreshold = slowWriteLoggingThreshold;
     }
 
-    private static String getManifestFileName(long term, long version) {
-        // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest_2147483642_2147483637_456536447
-        return String.join(DELIMITER, getManifestFileNamePrefix(term, version), RemoteStoreUtils.invertLong(System.currentTimeMillis()));
-    }
-
-    private static String getManifestFileNamePrefix(long term, long version) {
-        // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest_2147483642_2147483637
-        return String.join(DELIMITER, MANIFEST_PATH_TOKEN, RemoteStoreUtils.invertLong(term), RemoteStoreUtils.invertLong(version));
+    static String getManifestFileName(long term, long version, boolean committed) {
+        // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest__<inverted_term>__<inverted_version>__C/P__<inverted__timestamp>__<codec_version>
+        return String.join(
+            DELIMITER,
+            MANIFEST_PATH_TOKEN,
+            RemoteStoreUtils.invertLong(term),
+            RemoteStoreUtils.invertLong(version),
+            (committed ? "C" : "P"), // C for committed and P for published
+            RemoteStoreUtils.invertLong(System.currentTimeMillis()),
+            String.valueOf(MANIFEST_CURRENT_CODEC_VERSION) // Keep the codec version at last place only, during read we reads last place to
+                                                           // determine codec version.
+        );
     }
 
-    private static String indexMetadataFileName(IndexMetadata indexMetadata) {
+    static String indexMetadataFileName(IndexMetadata indexMetadata) {
+        // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/index/<index_UUID>/metadata__<inverted_index_metadata_version>__<inverted__timestamp>__<codec
+        // version>
         return String.join(
             DELIMITER,
             INDEX_METADATA_FILE_PREFIX,
-            String.valueOf(indexMetadata.getVersion()),
-            String.valueOf(System.currentTimeMillis())
+            RemoteStoreUtils.invertLong(indexMetadata.getVersion()),
+            RemoteStoreUtils.invertLong(System.currentTimeMillis()),
+            String.valueOf(INDEX_METADATA_CURRENT_CODEC_VERSION) // Keep the codec version at last place only, during read we reads last
+                                                                 // place to determine codec version.
         );
     }
 
@@ -916,8 +927,7 @@ private void deleteClusterMetadata(
                     if (filesToKeep.contains(uploadedIndexMetadata.getUploadedFilename()) == false) {
                         staleIndexMetadataPaths.add(
                             new BlobPath().add(INDEX_PATH_TOKEN).add(uploadedIndexMetadata.getIndexUUID()).buildAsString()
-                                + uploadedIndexMetadata.getUploadedFilename()
-                                + ".dat"
+                                + INDEX_METADATA_FORMAT.blobName(uploadedIndexMetadata.getUploadedFilename())
                         );
                     }
                 });

server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java

@@ -34,6 +34,7 @@
 import org.opensearch.core.index.Index;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.gateway.remote.ClusterMetadataManifest.UploadedIndexMetadata;
+import org.opensearch.index.remote.RemoteStoreUtils;
 import org.opensearch.repositories.FilterRepository;
 import org.opensearch.repositories.RepositoriesService;
 import org.opensearch.repositories.RepositoryMissingException;
@@ -65,6 +66,9 @@
 import org.mockito.ArgumentMatchers;
 
 import static org.opensearch.gateway.remote.RemoteClusterStateService.DELIMITER;
+import static org.opensearch.gateway.remote.RemoteClusterStateService.INDEX_METADATA_CURRENT_CODEC_VERSION;
+import static org.opensearch.gateway.remote.RemoteClusterStateService.INDEX_METADATA_FILE_PREFIX;
+import static org.opensearch.gateway.remote.RemoteClusterStateService.MANIFEST_CURRENT_CODEC_VERSION;
 import static org.opensearch.gateway.remote.RemoteClusterStateService.MANIFEST_FILE_PREFIX;
 import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY;
 import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX;
@@ -673,6 +677,40 @@ public void testDeleteStaleClusterUUIDs() throws IOException {
         }
     }
 
+    public void testFileNames() {
+        final Index index = new Index("test-index", "index-uuid");
+        final Settings idxSettings = Settings.builder()
+            .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
+            .put(IndexMetadata.SETTING_INDEX_UUID, index.getUUID())
+            .build();
+        final IndexMetadata indexMetadata = new IndexMetadata.Builder(index.getName()).settings(idxSettings)
+            .numberOfShards(1)
+            .numberOfReplicas(0)
+            .build();
+
+        String indexMetadataFileName = RemoteClusterStateService.indexMetadataFileName(indexMetadata);
+        String[] splittedIndexMetadataFileName = indexMetadataFileName.split(DELIMITER);
+        assertThat(indexMetadataFileName.split(DELIMITER).length, is(4));
+        assertThat(splittedIndexMetadataFileName[0], is(INDEX_METADATA_FILE_PREFIX));
+        assertThat(splittedIndexMetadataFileName[1], is(RemoteStoreUtils.invertLong(indexMetadata.getVersion())));
+        assertThat(splittedIndexMetadataFileName[3], is(String.valueOf(INDEX_METADATA_CURRENT_CODEC_VERSION)));
+
+        int term = randomIntBetween(5, 10);
+        int version = randomIntBetween(5, 10);
+        String manifestFileName = RemoteClusterStateService.getManifestFileName(term, version, true);
+        assertThat(manifestFileName.split(DELIMITER).length, is(6));
+        String[] splittedName = manifestFileName.split(DELIMITER);
+        assertThat(splittedName[0], is(MANIFEST_FILE_PREFIX));
+        assertThat(splittedName[1], is(RemoteStoreUtils.invertLong(term)));
+        assertThat(splittedName[2], is(RemoteStoreUtils.invertLong(version)));
+        assertThat(splittedName[3], is("C"));
+        assertThat(splittedName[5], is(String.valueOf(MANIFEST_CURRENT_CODEC_VERSION)));
+
+        manifestFileName = RemoteClusterStateService.getManifestFileName(term, version, false);
+        splittedName = manifestFileName.split(DELIMITER);
+        assertThat(splittedName[3], is("P"));
+    }
+
     private void mockObjectsForGettingPreviousClusterUUID(Map<String, String> clusterUUIDsPointers) throws IOException {
         final BlobPath blobPath = mock(BlobPath.class);
         when((blobStoreRepository.basePath())).thenReturn(blobPath);