From 61e2ce372847f1b74cb50808cd19c599170ea3fd Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Wed, 11 Oct 2023 17:40:15 +0530 Subject: [PATCH 1/9] Change filenames for IndexMetadata and Manifest Signed-off-by: Dhwanil Patel --- .../gateway/remote/RemoteClusterStateService.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index 4a8a0618ffa60..c43bdb82c073d 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -120,6 +120,8 @@ public class RemoteClusterStateService implements Closeable { private final AtomicBoolean deleteStaleMetadataRunning = new AtomicBoolean(false); + private static final int INDEX_METADATA_CODEC_VERSION = 1; + private static final int MANIFEST_CODEC_VERSION = 1; public RemoteClusterStateService( String nodeId, Supplier repositoriesService, @@ -490,7 +492,9 @@ private void setSlowWriteLoggingThreshold(TimeValue 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())); + return String.join(DELIMITER, getManifestFileNamePrefix(term, version), + RemoteStoreUtils.invertLong(MANIFEST_CODEC_VERSION), + RemoteStoreUtils.invertLong(System.currentTimeMillis())); } private static String getManifestFileNamePrefix(long term, long version) { @@ -502,7 +506,8 @@ private static String indexMetadataFileName(IndexMetadata indexMetadata) { return String.join( DELIMITER, INDEX_METADATA_FILE_PREFIX, - String.valueOf(indexMetadata.getVersion()), + RemoteStoreUtils.invertLong(indexMetadata.getVersion()), + RemoteStoreUtils.invertLong(INDEX_METADATA_CODEC_VERSION), String.valueOf(System.currentTimeMillis()) ); } @@ -916,8 +921,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()) ); } }); From 70054f13f9d42488801b2bd970c5a8deacb3ec9f Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Thu, 12 Oct 2023 13:42:03 +0530 Subject: [PATCH 2/9] Added UT Signed-off-by: Dhwanil Patel --- CHANGELOG.md | 3 ++- .../remote/RemoteClusterStateService.java | 12 ++++++--- .../RemoteClusterStateServiceTests.java | 27 +++++++++++++++++++ 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 572a8346a0686..a4adff3528553 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Implement Visitor Design pattern in QueryBuilder to enable the capability to traverse through the complex QueryBuilder tree. ([#10110](https://github.com/opensearch-project/OpenSearch/pull/10110)) - Provide service accounts tokens to extensions ([#9618](https://github.com/opensearch-project/OpenSearch/pull/9618)) - Configurable merge policy for index with an option to choose from LogByteSize and Tiered merge policy ([#9992](https://github.com/opensearch-project/OpenSearch/pull/9992)) +- 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 @@ -158,4 +159,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Security [Unreleased 3.0]: https://github.com/opensearch-project/OpenSearch/compare/2.x...HEAD -[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.11...2.x \ No newline at end of file +[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.11...2.x diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index c43bdb82c073d..48d0a3efe9d35 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -122,6 +122,7 @@ public class RemoteClusterStateService implements Closeable { private static final int INDEX_METADATA_CODEC_VERSION = 1; private static final int MANIFEST_CODEC_VERSION = 1; + public RemoteClusterStateService( String nodeId, Supplier repositoriesService, @@ -490,11 +491,14 @@ private void setSlowWriteLoggingThreshold(TimeValue slowWriteLoggingThreshold) { this.slowWriteLoggingThreshold = slowWriteLoggingThreshold; } - private static String getManifestFileName(long term, long version) { + 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), + return String.join( + DELIMITER, + getManifestFileNamePrefix(term, version), RemoteStoreUtils.invertLong(MANIFEST_CODEC_VERSION), - RemoteStoreUtils.invertLong(System.currentTimeMillis())); + RemoteStoreUtils.invertLong(System.currentTimeMillis()) + ); } private static String getManifestFileNamePrefix(long term, long version) { @@ -502,7 +506,7 @@ private static String getManifestFileNamePrefix(long term, long version) { return String.join(DELIMITER, MANIFEST_PATH_TOKEN, RemoteStoreUtils.invertLong(term), RemoteStoreUtils.invertLong(version)); } - private static String indexMetadataFileName(IndexMetadata indexMetadata) { + static String indexMetadataFileName(IndexMetadata indexMetadata) { return String.join( DELIMITER, INDEX_METADATA_FILE_PREFIX, diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index 6ecbc23f75bee..f1ac386048053 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/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,7 @@ import org.mockito.ArgumentMatchers; import static org.opensearch.gateway.remote.RemoteClusterStateService.DELIMITER; +import static org.opensearch.gateway.remote.RemoteClusterStateService.INDEX_METADATA_FILE_PREFIX; 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 +675,31 @@ 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); + assertThat(indexMetadataFileName.split(DELIMITER).length, is(4)); + assertTrue(indexMetadataFileName.contains(INDEX_METADATA_FILE_PREFIX)); + assertTrue(indexMetadataFileName.contains(RemoteStoreUtils.invertLong(indexMetadata.getVersion()))); + + int term = randomInt(10); + int version = randomInt(10); + String manifestFileName = RemoteClusterStateService.getManifestFileName(term, version); + assertThat(manifestFileName.split(DELIMITER).length, is(5)); + assertTrue(manifestFileName.contains(MANIFEST_FILE_PREFIX)); + assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(term))); + assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(version))); + } + private void mockObjectsForGettingPreviousClusterUUID(Map clusterUUIDsPointers) throws IOException { final BlobPath blobPath = mock(BlobPath.class); when((blobStoreRepository.basePath())).thenReturn(blobPath); From 179d32f8bc082ea7edf70e7e4c6884151f9d4b1b Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Mon, 16 Oct 2023 17:03:13 +0530 Subject: [PATCH 3/9] Added commited flag in manifest filename Signed-off-by: Dhwanil Patel --- .../remote/RemoteClusterStateService.java | 24 ++++++++++++------- .../RemoteClusterStateServiceTests.java | 4 ++-- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index 48d0a3efe9d35..aba08eebb9ec4 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -121,7 +121,7 @@ public class RemoteClusterStateService implements Closeable { private final AtomicBoolean deleteStaleMetadataRunning = new AtomicBoolean(false); private static final int INDEX_METADATA_CODEC_VERSION = 1; - private static final int MANIFEST_CODEC_VERSION = 1; + private static final int MANIFEST_CODEC_VERSION = 2; public RemoteClusterStateService( String nodeId, @@ -429,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(), @@ -491,19 +491,25 @@ private void setSlowWriteLoggingThreshold(TimeValue slowWriteLoggingThreshold) { this.slowWriteLoggingThreshold = slowWriteLoggingThreshold; } - static String getManifestFileName(long term, long version) { - // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest_2147483642_2147483637_456536447 + static String getManifestFileName(long term, long version, boolean committed) { + // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest_2147483642_2147483637_1_21473637_456536447 return String.join( DELIMITER, - getManifestFileNamePrefix(term, version), + getManifestFileNamePrefix(term, version, committed), RemoteStoreUtils.invertLong(MANIFEST_CODEC_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)); + private static String getManifestFileNamePrefix(long term, long version, boolean committed) { + // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest_2147483642_2147483637_1 + return String.join( + DELIMITER, + MANIFEST_PATH_TOKEN, + RemoteStoreUtils.invertLong(term), + RemoteStoreUtils.invertLong(version), + String.valueOf(committed ? 1 : 0) + ); } static String indexMetadataFileName(IndexMetadata indexMetadata) { @@ -512,7 +518,7 @@ static String indexMetadataFileName(IndexMetadata indexMetadata) { INDEX_METADATA_FILE_PREFIX, RemoteStoreUtils.invertLong(indexMetadata.getVersion()), RemoteStoreUtils.invertLong(INDEX_METADATA_CODEC_VERSION), - String.valueOf(System.currentTimeMillis()) + RemoteStoreUtils.invertLong(System.currentTimeMillis()) ); } diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index f1ac386048053..4255ac66da373 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -693,8 +693,8 @@ public void testFileNames() { int term = randomInt(10); int version = randomInt(10); - String manifestFileName = RemoteClusterStateService.getManifestFileName(term, version); - assertThat(manifestFileName.split(DELIMITER).length, is(5)); + String manifestFileName = RemoteClusterStateService.getManifestFileName(term, version, true); + assertThat(manifestFileName.split(DELIMITER).length, is(6)); assertTrue(manifestFileName.contains(MANIFEST_FILE_PREFIX)); assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(term))); assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(version))); From 9e2d31062474bdc18116cf12d5a43797ecf972e9 Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Tue, 17 Oct 2023 12:23:50 +0530 Subject: [PATCH 4/9] inverted long for committed flag Signed-off-by: Dhwanil Patel --- .../opensearch/gateway/remote/RemoteClusterStateService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index aba08eebb9ec4..3c5ad7c822514 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -502,13 +502,13 @@ static String getManifestFileName(long term, long version, boolean committed) { } private static String getManifestFileNamePrefix(long term, long version, boolean committed) { - // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest_2147483642_2147483637_1 + // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest_2147483642_2147483637_214748367 return String.join( DELIMITER, MANIFEST_PATH_TOKEN, RemoteStoreUtils.invertLong(term), RemoteStoreUtils.invertLong(version), - String.valueOf(committed ? 1 : 0) + RemoteStoreUtils.invertLong(committed ? 1 : 0) ); } From 64896f2c395f4b7ee94284ba271b4027108b80c2 Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Tue, 17 Oct 2023 14:02:31 +0530 Subject: [PATCH 5/9] Incorporated comments Signed-off-by: Dhwanil Patel --- .../gateway/remote/RemoteClusterStateService.java | 14 +++----------- .../remote/RemoteClusterStateServiceTests.java | 6 ++++-- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index 3c5ad7c822514..3487943ac766a 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -493,22 +493,14 @@ private void setSlowWriteLoggingThreshold(TimeValue slowWriteLoggingThreshold) { static String getManifestFileName(long term, long version, boolean committed) { // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest_2147483642_2147483637_1_21473637_456536447 - return String.join( - DELIMITER, - getManifestFileNamePrefix(term, version, committed), - RemoteStoreUtils.invertLong(MANIFEST_CODEC_VERSION), - RemoteStoreUtils.invertLong(System.currentTimeMillis()) - ); - } - - private static String getManifestFileNamePrefix(long term, long version, boolean committed) { - // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest_2147483642_2147483637_214748367 return String.join( DELIMITER, MANIFEST_PATH_TOKEN, RemoteStoreUtils.invertLong(term), RemoteStoreUtils.invertLong(version), - RemoteStoreUtils.invertLong(committed ? 1 : 0) + RemoteStoreUtils.invertLong(committed ? 1 : 0), + RemoteStoreUtils.invertLong(MANIFEST_CODEC_VERSION), + RemoteStoreUtils.invertLong(System.currentTimeMillis()) ); } diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index 4255ac66da373..5191d41442e8f 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -691,13 +691,15 @@ public void testFileNames() { assertTrue(indexMetadataFileName.contains(INDEX_METADATA_FILE_PREFIX)); assertTrue(indexMetadataFileName.contains(RemoteStoreUtils.invertLong(indexMetadata.getVersion()))); - int term = randomInt(10); - int version = randomInt(10); + int term = randomIntBetween(5, 10); + int version = randomIntBetween(5, 10); String manifestFileName = RemoteClusterStateService.getManifestFileName(term, version, true); assertThat(manifestFileName.split(DELIMITER).length, is(6)); assertTrue(manifestFileName.contains(MANIFEST_FILE_PREFIX)); assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(term))); assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(version))); + assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(1))); // assertion for committed + assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(2))); // assertion for codec version } private void mockObjectsForGettingPreviousClusterUUID(Map clusterUUIDsPointers) throws IOException { From b05beaad48e0d6745c38a69eed62f1ddef82ef61 Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Tue, 17 Oct 2023 15:16:22 +0530 Subject: [PATCH 6/9] Changed flag name for committed to C/P Signed-off-by: Dhwanil Patel --- .../opensearch/gateway/remote/RemoteClusterStateService.java | 2 +- .../gateway/remote/RemoteClusterStateServiceTests.java | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index 3487943ac766a..2154f88c84bde 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -498,7 +498,7 @@ static String getManifestFileName(long term, long version, boolean committed) { MANIFEST_PATH_TOKEN, RemoteStoreUtils.invertLong(term), RemoteStoreUtils.invertLong(version), - RemoteStoreUtils.invertLong(committed ? 1 : 0), + (committed ? "C" : "P"), // C for committed and P for published RemoteStoreUtils.invertLong(MANIFEST_CODEC_VERSION), RemoteStoreUtils.invertLong(System.currentTimeMillis()) ); diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index 5191d41442e8f..d03143e92ca4a 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -698,8 +698,11 @@ public void testFileNames() { assertTrue(manifestFileName.contains(MANIFEST_FILE_PREFIX)); assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(term))); assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(version))); - assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(1))); // assertion for committed + assertTrue(manifestFileName.contains("C")); // assertion for committed assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(2))); // assertion for codec version + + manifestFileName = RemoteClusterStateService.getManifestFileName(term, version, false); + assertTrue(manifestFileName.contains("P")); // assertion for committed false } private void mockObjectsForGettingPreviousClusterUUID(Map clusterUUIDsPointers) throws IOException { From dbf3c33a98279629cd74e2c366e6a4aa2cd0e35c Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Tue, 17 Oct 2023 17:16:52 +0530 Subject: [PATCH 7/9] Incorporated comments Signed-off-by: Dhwanil Patel --- .../remote/RemoteClusterStateService.java | 11 +++++----- .../RemoteClusterStateServiceTests.java | 22 ++++++++++++------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index 2154f88c84bde..47d674ba673d2 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -120,8 +120,8 @@ public class RemoteClusterStateService implements Closeable { private final AtomicBoolean deleteStaleMetadataRunning = new AtomicBoolean(false); - private static final int INDEX_METADATA_CODEC_VERSION = 1; - private static final int MANIFEST_CODEC_VERSION = 2; + public static final int INDEX_METADATA_CURRENT_CODEC_VERSION = 1; + public static final int MANIFEST_CURRENT_CODEC_VERSION = 1; public RemoteClusterStateService( String nodeId, @@ -492,24 +492,25 @@ private void setSlowWriteLoggingThreshold(TimeValue slowWriteLoggingThreshold) { } static String getManifestFileName(long term, long version, boolean committed) { - // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest_2147483642_2147483637_1_21473637_456536447 + // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest______C/P____ 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(MANIFEST_CODEC_VERSION), + RemoteStoreUtils.invertLong(MANIFEST_CURRENT_CODEC_VERSION), RemoteStoreUtils.invertLong(System.currentTimeMillis()) ); } static String indexMetadataFileName(IndexMetadata indexMetadata) { + // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/index//metadata______.dat return String.join( DELIMITER, INDEX_METADATA_FILE_PREFIX, RemoteStoreUtils.invertLong(indexMetadata.getVersion()), - RemoteStoreUtils.invertLong(INDEX_METADATA_CODEC_VERSION), + RemoteStoreUtils.invertLong(INDEX_METADATA_CURRENT_CODEC_VERSION), RemoteStoreUtils.invertLong(System.currentTimeMillis()) ); } diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index d03143e92ca4a..fcd12b41c91db 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -66,7 +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; @@ -687,22 +689,26 @@ public void testFileNames() { .build(); String indexMetadataFileName = RemoteClusterStateService.indexMetadataFileName(indexMetadata); + String[] splittedIndexMetadataFileName = indexMetadataFileName.split(DELIMITER); assertThat(indexMetadataFileName.split(DELIMITER).length, is(4)); - assertTrue(indexMetadataFileName.contains(INDEX_METADATA_FILE_PREFIX)); - assertTrue(indexMetadataFileName.contains(RemoteStoreUtils.invertLong(indexMetadata.getVersion()))); + assertThat(splittedIndexMetadataFileName[0], is(INDEX_METADATA_FILE_PREFIX)); + assertThat(splittedIndexMetadataFileName[1], is(RemoteStoreUtils.invertLong(indexMetadata.getVersion()))); + assertThat(splittedIndexMetadataFileName[2], is(RemoteStoreUtils.invertLong(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)); - assertTrue(manifestFileName.contains(MANIFEST_FILE_PREFIX)); - assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(term))); - assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(version))); - assertTrue(manifestFileName.contains("C")); // assertion for committed - assertTrue(manifestFileName.contains(RemoteStoreUtils.invertLong(2))); // assertion for codec version + 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[4], is(RemoteStoreUtils.invertLong(MANIFEST_CURRENT_CODEC_VERSION))); manifestFileName = RemoteClusterStateService.getManifestFileName(term, version, false); - assertTrue(manifestFileName.contains("P")); // assertion for committed false + splittedName = manifestFileName.split(DELIMITER); + assertThat(splittedName[3], is("P")); } private void mockObjectsForGettingPreviousClusterUUID(Map clusterUUIDsPointers) throws IOException { From 8a7d1909ebe57803385ea0685ce99160fa18b2ba Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Tue, 17 Oct 2023 18:55:09 +0530 Subject: [PATCH 8/9] Moved codec version to last Signed-off-by: Dhwanil Patel --- .../gateway/remote/RemoteClusterStateService.java | 15 +++++++++------ .../remote/RemoteClusterStateServiceTests.java | 4 ++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index 47d674ba673d2..c3aae485e5ee5 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -492,26 +492,29 @@ private void setSlowWriteLoggingThreshold(TimeValue slowWriteLoggingThreshold) { } static String getManifestFileName(long term, long version, boolean committed) { - // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest______C/P____ + // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest______C/P____ 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(MANIFEST_CURRENT_CODEC_VERSION), - RemoteStoreUtils.invertLong(System.currentTimeMillis()) + 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. ); } static String indexMetadataFileName(IndexMetadata indexMetadata) { - // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/index//metadata______.dat + // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/index//metadata______.dat return String.join( DELIMITER, INDEX_METADATA_FILE_PREFIX, RemoteStoreUtils.invertLong(indexMetadata.getVersion()), - RemoteStoreUtils.invertLong(INDEX_METADATA_CURRENT_CODEC_VERSION), - RemoteStoreUtils.invertLong(System.currentTimeMillis()) + 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. ); } diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index fcd12b41c91db..119d19cc34981 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -693,7 +693,7 @@ public void testFileNames() { 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[2], is(RemoteStoreUtils.invertLong(INDEX_METADATA_CURRENT_CODEC_VERSION))); + assertThat(splittedIndexMetadataFileName[3], is(String.valueOf(INDEX_METADATA_CURRENT_CODEC_VERSION))); int term = randomIntBetween(5, 10); int version = randomIntBetween(5, 10); @@ -704,7 +704,7 @@ public void testFileNames() { assertThat(splittedName[1], is(RemoteStoreUtils.invertLong(term))); assertThat(splittedName[2], is(RemoteStoreUtils.invertLong(version))); assertThat(splittedName[3], is("C")); - assertThat(splittedName[4], is(RemoteStoreUtils.invertLong(MANIFEST_CURRENT_CODEC_VERSION))); + assertThat(splittedName[5], is(String.valueOf(MANIFEST_CURRENT_CODEC_VERSION))); manifestFileName = RemoteClusterStateService.getManifestFileName(term, version, false); splittedName = manifestFileName.split(DELIMITER); From a2d9475e494f8c64970190ed5e55b083295c89f0 Mon Sep 17 00:00:00 2001 From: Dhwanil Patel Date: Tue, 17 Oct 2023 20:02:54 +0530 Subject: [PATCH 9/9] Fixed java doc Signed-off-by: Dhwanil Patel --- .../opensearch/gateway/remote/RemoteClusterStateService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index c3aae485e5ee5..0cf97de53d5f3 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -507,7 +507,7 @@ static String getManifestFileName(long term, long version, boolean committed) { static String indexMetadataFileName(IndexMetadata indexMetadata) { // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/index//metadata______.dat + // version> return String.join( DELIMITER, INDEX_METADATA_FILE_PREFIX,