diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStorePinnedTimestampsGarbageCollectionIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStorePinnedTimestampsGarbageCollectionIT.java index dedc6352f8140..a0e4281a11569 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStorePinnedTimestampsGarbageCollectionIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStorePinnedTimestampsGarbageCollectionIT.java @@ -118,17 +118,15 @@ public void testLiveIndexNoPinnedTimestamps() throws Exception { }); } - public void testLiveIndexNoPinnedTimestampsWithExtraGenSettingWithinLimit() throws Exception { + public void testLiveIndexNoPinnedTimestampsWithMetadataSkippedOnLastDeletionCheck() throws Exception { prepareCluster(1, 1, Settings.EMPTY); - Settings indexSettings = Settings.builder() - .put(remoteStoreIndexSettings(0, 1)) - .put(INDEX_REMOTE_TRANSLOG_KEEP_EXTRA_GEN_SETTING.getKey(), 10) - .build(); + Settings indexSettings = Settings.builder().put(remoteStoreIndexSettings(0, 1)).build(); createIndex(INDEX_NAME, indexSettings); ensureYellowAndNoInitializingShards(INDEX_NAME); ensureGreen(INDEX_NAME); - RemoteStoreSettings.setPinnedTimestampsLookbackInterval(TimeValue.ZERO); + // We don't set look-back interval to 0 as we want GC to skip based on last deletion check + // RemoteStoreSettings.setPinnedTimestampsLookbackInterval(TimeValue.ZERO); RemoteStorePinnedTimestampService remoteStorePinnedTimestampService = internalCluster().getInstance( RemoteStorePinnedTimestampService.class, @@ -173,61 +171,6 @@ public void testLiveIndexNoPinnedTimestampsWithExtraGenSettingWithinLimit() thro }); } - public void testLiveIndexNoPinnedTimestampsWithExtraGenSetting() throws Exception { - prepareCluster(1, 1, Settings.EMPTY); - Settings indexSettings = Settings.builder() - .put(remoteStoreIndexSettings(0, 1)) - .put(INDEX_REMOTE_TRANSLOG_KEEP_EXTRA_GEN_SETTING.getKey(), 3) - .build(); - createIndex(INDEX_NAME, indexSettings); - ensureYellowAndNoInitializingShards(INDEX_NAME); - ensureGreen(INDEX_NAME); - - RemoteStoreSettings.setPinnedTimestampsLookbackInterval(TimeValue.ZERO); - - RemoteStorePinnedTimestampService remoteStorePinnedTimestampService = internalCluster().getInstance( - RemoteStorePinnedTimestampService.class, - primaryNodeName(INDEX_NAME) - ); - - remoteStorePinnedTimestampService.rescheduleAsyncUpdatePinnedTimestampTask(TimeValue.timeValueSeconds(1)); - - int numDocs = 5; - for (int i = 0; i < numDocs; i++) { - keepPinnedTimestampSchedulerUpdated(); - indexSingleDoc(INDEX_NAME, true); - } - - String translogPathFixedPrefix = RemoteStoreSettings.CLUSTER_REMOTE_STORE_TRANSLOG_PATH_PREFIX.get(getNodeSettings()); - String shardDataPath = getShardLevelBlobPath( - client(), - INDEX_NAME, - BlobPath.cleanPath(), - "0", - TRANSLOG, - DATA, - translogPathFixedPrefix - ).buildAsString(); - Path translogDataPath = Path.of(translogRepoPath + "/" + shardDataPath + "/1"); - String shardMetadataPath = getShardLevelBlobPath( - client(), - INDEX_NAME, - BlobPath.cleanPath(), - "0", - TRANSLOG, - METADATA, - translogPathFixedPrefix - ).buildAsString(); - Path translogMetadataPath = Path.of(translogRepoPath + "/" + shardMetadataPath); - - assertBusy(() -> { - List metadataFiles = Files.list(translogMetadataPath).collect(Collectors.toList()); - assertEquals(3, metadataFiles.size()); - - verifyTranslogDataFileCount(metadataFiles, translogDataPath); - }); - } - public void testLiveIndexWithPinnedTimestamps() throws Exception { prepareCluster(1, 1, Settings.EMPTY); Settings indexSettings = Settings.builder() diff --git a/server/src/main/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslog.java b/server/src/main/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslog.java index 9391bc81c437c..cb8e9fbeba4ab 100644 --- a/server/src/main/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslog.java +++ b/server/src/main/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslog.java @@ -61,7 +61,7 @@ public class RemoteFsTimestampAwareTranslog extends RemoteFsTranslog { private final Map> oldFormatMetadataFileGenerationMap; private final Map> oldFormatMetadataFilePrimaryTermMap; private final AtomicLong minPrimaryTermInRemote = new AtomicLong(Long.MAX_VALUE); - private long maxDeletedGenerationOnRemote = 0; + private long lastTimestampOfMetadataDeletionOnRemote = System.currentTimeMillis(); public RemoteFsTimestampAwareTranslog( TranslogConfig config, @@ -148,8 +148,10 @@ protected void trimUnreferencedReaders(boolean indexDeleted, boolean trimLocal) // This code block ensures parity with RemoteFsTranslog. Without this, we will end up making list translog metadata // call in each invocation of trimUnreferencedReaders - long minGenerationToKeep = minRemoteGenReferenced - indexSettings().getRemoteTranslogExtraKeep(); - if (indexDeleted == false && (minGenerationToKeep <= maxDeletedGenerationOnRemote)) { + if (indexDeleted == false + && (System.currentTimeMillis() - lastTimestampOfMetadataDeletionOnRemote <= RemoteStoreSettings + .getPinnedTimestampsLookbackInterval() + .millis() * 2)) { return; } @@ -207,8 +209,6 @@ public void onResponse(List blobMetadata) { logger.debug(() -> "generationsToBeDeleted = " + generationsToBeDeleted); if (generationsToBeDeleted.isEmpty() == false) { - maxDeletedGenerationOnRemote = generationsToBeDeleted.stream().max(Long::compareTo).get(); - // Delete stale generations translogTransferManager.deleteGenerationAsync( primaryTermSupplier.getAsLong(), @@ -220,6 +220,7 @@ public void onResponse(List blobMetadata) { } if (metadataFilesToBeDeleted.isEmpty() == false) { + lastTimestampOfMetadataDeletionOnRemote = System.currentTimeMillis(); // Delete stale metadata files translogTransferManager.deleteMetadataFilesAsync( metadataFilesToBeDeleted, diff --git a/server/src/test/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslogTests.java b/server/src/test/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslogTests.java index 9608246a81d1f..799d858b7dd12 100644 --- a/server/src/test/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslogTests.java +++ b/server/src/test/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslogTests.java @@ -783,7 +783,7 @@ public void testGetMetadataFilesToBeDeletedExclusionBasedOnPinningOnly() throws public void testGetMetadataFilesToBeDeletedExclusionBasedOnAgeAndPinning() throws IOException { long currentTimeInMillis = System.currentTimeMillis(); - String md1Timestamp = RemoteStoreUtils.invertLong(currentTimeInMillis - 200000); + String md1Timestamp = RemoteStoreUtils.invertLong(currentTimeInMillis + 100000); String md2Timestamp = RemoteStoreUtils.invertLong(currentTimeInMillis - 300000); String md3Timestamp = RemoteStoreUtils.invertLong(currentTimeInMillis - 600000);