From 30ed15dea695932082d8ac2cdd661c1a669410dc Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Thu, 29 Aug 2024 20:28:43 +0530 Subject: [PATCH] Change RemoteSegmentStoreDirectory init at given timestamp to ignore pinned timestamp setting (#15457) Signed-off-by: Sachin Kale Co-authored-by: Sachin Kale --- .../index/remote/RemoteStoreUtils.java | 33 +++++++++- .../store/RemoteSegmentStoreDirectory.java | 3 +- .../main/java/org/opensearch/node/Node.java | 3 +- .../RemoteSegmentStoreDirectoryTests.java | 56 +++++++++++++++++ ...toreDirectoryWithPinnedTimestampTests.java | 62 ------------------- 5 files changed, 90 insertions(+), 67 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java index b2bc8a0294a49..871e2eb3ce47f 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java @@ -391,15 +391,24 @@ public static boolean isSwitchToStrictCompatibilityMode(ClusterUpdateSettingsReq * @param pinnedTimestampSet A set of timestamps representing pinned points in time. * @param getTimestampFunction A function that extracts the timestamp from a metadata file name. * @param prefixFunction A function that extracts a tuple of prefix information from a metadata file name. + * @param ignorePinnedTimestampEnabledSetting A flag to ignore pinned timestamp enabled setting * @return A set of metadata file names that are implicitly locked based on the pinned timestamps. */ public static Set getPinnedTimestampLockedFiles( List metadataFiles, Set pinnedTimestampSet, Function getTimestampFunction, - Function> prefixFunction + Function> prefixFunction, + boolean ignorePinnedTimestampEnabledSetting ) { - return getPinnedTimestampLockedFiles(metadataFiles, pinnedTimestampSet, new HashMap<>(), getTimestampFunction, prefixFunction); + return getPinnedTimestampLockedFiles( + metadataFiles, + pinnedTimestampSet, + new HashMap<>(), + getTimestampFunction, + prefixFunction, + ignorePinnedTimestampEnabledSetting + ); } /** @@ -431,10 +440,28 @@ public static Set getPinnedTimestampLockedFiles( Map metadataFilePinnedTimestampMap, Function getTimestampFunction, Function> prefixFunction + ) { + return getPinnedTimestampLockedFiles( + metadataFiles, + pinnedTimestampSet, + metadataFilePinnedTimestampMap, + getTimestampFunction, + prefixFunction, + false + ); + } + + private static Set getPinnedTimestampLockedFiles( + List metadataFiles, + Set pinnedTimestampSet, + Map metadataFilePinnedTimestampMap, + Function getTimestampFunction, + Function> prefixFunction, + boolean ignorePinnedTimestampEnabledSetting ) { Set implicitLockedFiles = new HashSet<>(); - if (RemoteStoreSettings.isPinnedTimestampsEnabled() == false) { + if (ignorePinnedTimestampEnabledSetting == false && RemoteStoreSettings.isPinnedTimestampsEnabled() == false) { return implicitLockedFiles; } diff --git a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java index 26871429e41d6..53b43bbfb3bba 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -196,7 +196,8 @@ public RemoteSegmentMetadata initializeToSpecificTimestamp(long timestamp) throw metadataFiles, Set.of(timestamp), MetadataFilenameUtils::getTimestamp, - MetadataFilenameUtils::getNodeIdByPrimaryTermAndGen + MetadataFilenameUtils::getNodeIdByPrimaryTermAndGen, + true ); if (lockedMetadataFiles.isEmpty()) { return null; diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 388e00bedab0c..9c7dfe8850b85 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -307,6 +307,7 @@ import static org.opensearch.env.NodeEnvironment.collectFileCacheDataPath; import static org.opensearch.index.ShardIndexingPressureSettings.SHARD_INDEXING_PRESSURE_ENABLED_ATTRIBUTE_KEY; import static org.opensearch.indices.RemoteStoreSettings.CLUSTER_REMOTE_STORE_PINNED_TIMESTAMP_ENABLED; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteDataAttributePresent; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteStoreAttributePresent; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteStoreClusterStateEnabled; @@ -814,7 +815,7 @@ protected Node( remoteClusterStateCleanupManager = null; } final RemoteStorePinnedTimestampService remoteStorePinnedTimestampService; - if (isRemoteStoreAttributePresent(settings) && CLUSTER_REMOTE_STORE_PINNED_TIMESTAMP_ENABLED.get(settings)) { + if (isRemoteDataAttributePresent(settings) && CLUSTER_REMOTE_STORE_PINNED_TIMESTAMP_ENABLED.get(settings)) { remoteStorePinnedTimestampService = new RemoteStorePinnedTimestampService( repositoriesServiceReference::get, settings, diff --git a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java index 574c5bf620474..336d4bafd4b66 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -1141,6 +1141,62 @@ public void testMetadataFileNameOrder() { assertEquals(14, count); } + public void testInitializeToSpecificTimestampNoMetadataFiles() throws IOException { + when( + remoteMetadataDirectory.listFilesByPrefixInLexicographicOrder( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + Integer.MAX_VALUE + ) + ).thenReturn(new ArrayList<>()); + assertNull(remoteSegmentStoreDirectory.initializeToSpecificTimestamp(1234L)); + } + + public void testInitializeToSpecificTimestampNoMdMatchingTimestamp() throws IOException { + String metadataPrefix = "metadata__1__2__3__4__5__"; + List metadataFiles = new ArrayList<>(); + metadataFiles.add(metadataPrefix + RemoteStoreUtils.invertLong(2000)); + metadataFiles.add(metadataPrefix + RemoteStoreUtils.invertLong(3000)); + metadataFiles.add(metadataPrefix + RemoteStoreUtils.invertLong(4000)); + + when( + remoteMetadataDirectory.listFilesByPrefixInLexicographicOrder( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + Integer.MAX_VALUE + ) + ).thenReturn(metadataFiles); + assertNull(remoteSegmentStoreDirectory.initializeToSpecificTimestamp(1234L)); + } + + public void testInitializeToSpecificTimestampMatchingMdFile() throws IOException { + String metadataPrefix = "metadata__1__2__3__4__5__"; + List metadataFiles = new ArrayList<>(); + metadataFiles.add(metadataPrefix + RemoteStoreUtils.invertLong(1000)); + metadataFiles.add(metadataPrefix + RemoteStoreUtils.invertLong(2000)); + metadataFiles.add(metadataPrefix + RemoteStoreUtils.invertLong(3000)); + + Map metadata = new HashMap<>(); + metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::512::" + Version.LATEST.major); + metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::1024::" + Version.LATEST.major); + + when( + remoteMetadataDirectory.listFilesByPrefixInLexicographicOrder( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + Integer.MAX_VALUE + ) + ).thenReturn(metadataFiles); + when(remoteMetadataDirectory.getBlobStream(metadataPrefix + RemoteStoreUtils.invertLong(1000))).thenReturn( + createMetadataFileBytes(metadata, indexShard.getLatestReplicationCheckpoint(), segmentInfos) + ); + + RemoteSegmentMetadata remoteSegmentMetadata = remoteSegmentStoreDirectory.initializeToSpecificTimestamp(1234L); + assertNotNull(remoteSegmentMetadata); + Map uploadedSegments = remoteSegmentStoreDirectory + .getSegmentsUploadedToRemoteStore(); + assertEquals(2, uploadedSegments.size()); + assertTrue(uploadedSegments.containsKey("_0.cfe")); + assertTrue(uploadedSegments.containsKey("_0.cfs")); + } + private static class WrapperIndexOutput extends IndexOutput { public IndexOutput indexOutput; diff --git a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryWithPinnedTimestampTests.java b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryWithPinnedTimestampTests.java index b4f93d706bb1e..107d59aa97549 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryWithPinnedTimestampTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryWithPinnedTimestampTests.java @@ -8,8 +8,6 @@ package org.opensearch.index.store; -import org.apache.lucene.util.Version; -import org.opensearch.common.UUIDs; import org.opensearch.common.blobstore.BlobMetadata; import org.opensearch.common.blobstore.BlobPath; import org.opensearch.common.blobstore.support.PlainBlobMetadata; @@ -18,8 +16,6 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.gateway.remote.model.RemotePinnedTimestamps; import org.opensearch.gateway.remote.model.RemoteStorePinnedTimestampsBlobStore; -import org.opensearch.index.remote.RemoteStoreUtils; -import org.opensearch.index.store.remote.metadata.RemoteSegmentMetadata; import org.opensearch.index.translog.transfer.BlobStoreTransferService; import org.opensearch.indices.RemoteStoreSettings; import org.opensearch.node.Node; @@ -31,7 +27,6 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.function.Supplier; @@ -40,7 +35,6 @@ import org.mockito.Mockito; import static org.opensearch.indices.RemoteStoreSettings.CLUSTER_REMOTE_STORE_PINNED_TIMESTAMP_ENABLED; -import static org.opensearch.test.RemoteStoreTestUtils.createMetadataFileBytes; import static org.hamcrest.CoreMatchers.is; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.anyInt; @@ -143,62 +137,6 @@ private void metadataWithOlderTimestamp() { ); } - public void testInitializeToSpecificTimestampNoMetadataFiles() throws IOException { - when( - remoteMetadataDirectory.listFilesByPrefixInLexicographicOrder( - RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, - Integer.MAX_VALUE - ) - ).thenReturn(new ArrayList<>()); - assertNull(remoteSegmentStoreDirectory.initializeToSpecificTimestamp(1234L)); - } - - public void testInitializeToSpecificTimestampNoMdMatchingTimestamp() throws IOException { - String metadataPrefix = "metadata__1__2__3__4__5__"; - List metadataFiles = new ArrayList<>(); - metadataFiles.add(metadataPrefix + RemoteStoreUtils.invertLong(2000)); - metadataFiles.add(metadataPrefix + RemoteStoreUtils.invertLong(3000)); - metadataFiles.add(metadataPrefix + RemoteStoreUtils.invertLong(4000)); - - when( - remoteMetadataDirectory.listFilesByPrefixInLexicographicOrder( - RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, - Integer.MAX_VALUE - ) - ).thenReturn(metadataFiles); - assertNull(remoteSegmentStoreDirectory.initializeToSpecificTimestamp(1234L)); - } - - public void testInitializeToSpecificTimestampMatchingMdFile() throws IOException { - String metadataPrefix = "metadata__1__2__3__4__5__"; - List metadataFiles = new ArrayList<>(); - metadataFiles.add(metadataPrefix + RemoteStoreUtils.invertLong(1000)); - metadataFiles.add(metadataPrefix + RemoteStoreUtils.invertLong(2000)); - metadataFiles.add(metadataPrefix + RemoteStoreUtils.invertLong(3000)); - - Map metadata = new HashMap<>(); - metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::512::" + Version.LATEST.major); - metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::1024::" + Version.LATEST.major); - - when( - remoteMetadataDirectory.listFilesByPrefixInLexicographicOrder( - RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, - Integer.MAX_VALUE - ) - ).thenReturn(metadataFiles); - when(remoteMetadataDirectory.getBlobStream(metadataPrefix + RemoteStoreUtils.invertLong(1000))).thenReturn( - createMetadataFileBytes(metadata, indexShard.getLatestReplicationCheckpoint(), segmentInfos) - ); - - RemoteSegmentMetadata remoteSegmentMetadata = remoteSegmentStoreDirectory.initializeToSpecificTimestamp(1234L); - assertNotNull(remoteSegmentMetadata); - Map uploadedSegments = remoteSegmentStoreDirectory - .getSegmentsUploadedToRemoteStore(); - assertEquals(2, uploadedSegments.size()); - assertTrue(uploadedSegments.containsKey("_0.cfe")); - assertTrue(uploadedSegments.containsKey("_0.cfs")); - } - public void testDeleteStaleCommitsNoPinnedTimestampMdFilesLatest() throws Exception { metadataFilename = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename( 12,