Fix filtering based on age

Signed-off-by: Sachin Kale <[email protected]>

server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java

@@ -20,7 +20,6 @@
 import org.opensearch.cluster.routing.RoutingTable;
 import org.opensearch.common.collect.Tuple;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.unit.TimeValue;
 import org.opensearch.indices.RemoteStoreSettings;
 import org.opensearch.node.remotestore.RemoteStoreNodeAttribute;
 import org.opensearch.node.remotestore.RemoteStoreNodeService;
@@ -491,23 +490,23 @@ public static Set<String> getPinnedTimestampLockedFiles(
     }
 
     /**
-     * Filters out metadata files based on their age.
+     * Filters out metadata files from the given list based on their age.
      *
-     * @param metadataFiles        List of metadata file names to filter.
-     * @param getTimestampFunction Function to extract timestamp from a file name.
-     * @param minimumAge           Minimum age threshold for filtering.
-     * @return A list of metadata file names that are older than the minimum age.
+     * @param metadataFiles            A list of metadata file names.
+     * @param getTimestampFunction     A function that takes a metadata file name as input and returns its timestamp.
+     * @param maximumAllowedTimestamp  The maximum allowed timestamp for a metadata file to be included in the filtered list.
+     *                                 Metadata files with a timestamp greater than or equal to this value will be excluded.
+     * @return A list of metadata file names that have a timestamp less than the specified maximum allowed timestamp.
      */
     public static List<String> filterOutMetadataFilesBasedOnAge(
         List<String> metadataFiles,
         Function<String, Long> getTimestampFunction,
-        TimeValue minimumAge
+        long maximumAllowedTimestamp
     ) {
-        long maxTimestampAllowed = System.currentTimeMillis() - minimumAge.getMillis();
         List<String> metadataFilesWithMinAge = new ArrayList<>();
         for (String metadataFileName : metadataFiles) {
             long metadataTimestamp = getTimestampFunction.apply(metadataFileName);
-            if (metadataTimestamp < maxTimestampAllowed) {
+            if (metadataTimestamp < maximumAllowedTimestamp) {
                 metadataFilesWithMinAge.add(metadataFileName);
             }
         }

server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java

@@ -29,7 +29,6 @@
 import org.opensearch.common.io.VersionedCodecStreamWrapper;
 import org.opensearch.common.logging.Loggers;
 import org.opensearch.common.lucene.store.ByteArrayIndexInput;
-import org.opensearch.common.unit.TimeValue;
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.index.shard.ShardId;
 import org.opensearch.index.remote.RemoteStorePathStrategy;
@@ -862,12 +861,12 @@ public void deleteStaleSegments(int lastNMetadataFilesToKeep) throws IOException
 
         // Along with last N files, we need to keep files since last successful run of scheduler
         long lastSuccessfulFetchOfPinnedTimestamps = pinnedTimestampsState.v1();
-        long minimumAgeInMillis = lastSuccessfulFetchOfPinnedTimestamps + RemoteStoreSettings.getPinnedTimestampsLookbackInterval()
+        long maximumAllowedTimestamp = lastSuccessfulFetchOfPinnedTimestamps - RemoteStoreSettings.getPinnedTimestampsLookbackInterval()
             .getMillis();
         metadataFilesEligibleToDelete = RemoteStoreUtils.filterOutMetadataFilesBasedOnAge(
             metadataFilesEligibleToDelete,
             MetadataFilenameUtils::getTimestamp,
-            TimeValue.timeValueMillis(minimumAgeInMillis)
+            maximumAllowedTimestamp
         );
 
         List<String> metadataFilesToBeDeleted = metadataFilesEligibleToDelete.stream()

server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java

@@ -27,7 +27,6 @@
 import org.opensearch.common.blobstore.support.PlainBlobMetadata;
 import org.opensearch.common.collect.Tuple;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.unit.TimeValue;
 import org.opensearch.core.index.Index;
 import org.opensearch.core.index.shard.ShardId;
 import org.opensearch.index.shard.IndexShardTestUtils;
@@ -961,12 +960,12 @@ public void testFilterOutMetadataFilesBasedOnAge_AllFilesOldEnough() {
             (System.currentTimeMillis() - 450000) + "_file3.txt"
         );
 
-        TimeValue minimumAge = TimeValue.timeValueSeconds(10);
+        long maximumAllowedTimestamp = System.currentTimeMillis() - 10000;
 
         List<String> result = RemoteStoreUtils.filterOutMetadataFilesBasedOnAge(
             metadataFiles,
             file -> Long.valueOf(file.split("_")[0]),
-            minimumAge
+            maximumAllowedTimestamp
         );
         assertEquals(metadataFiles, result);
     }
@@ -977,12 +976,12 @@ public void testFilterOutMetadataFilesBasedOnAge_SomeFilesTooNew() {
         String file3 = (System.currentTimeMillis() + 450000) + "_file3.txt";
 
         List<String> metadataFiles = Arrays.asList(file1, file2, file3);
-        TimeValue minimumAge = TimeValue.timeValueSeconds(10);
+        long maximumAllowedTimestamp = System.currentTimeMillis() - 10000;
 
         List<String> result = RemoteStoreUtils.filterOutMetadataFilesBasedOnAge(
             metadataFiles,
             file -> Long.valueOf(file.split("_")[0]),
-            minimumAge
+            maximumAllowedTimestamp
         );
         List<String> expected = Arrays.asList(file1, file2);
         assertEquals(expected, result);
@@ -994,24 +993,24 @@ public void testFilterOutMetadataFilesBasedOnAge_AllFilesTooNew() {
         String file3 = (System.currentTimeMillis() + 450000) + "_file3.txt";
 
         List<String> metadataFiles = Arrays.asList(file1, file2, file3);
-        TimeValue minimumAge = TimeValue.timeValueSeconds(10);
+        long maximumAllowedTimestamp = System.currentTimeMillis() - 10000;
 
         List<String> result = RemoteStoreUtils.filterOutMetadataFilesBasedOnAge(
             metadataFiles,
             file -> Long.valueOf(file.split("_")[0]),
-            minimumAge
+            maximumAllowedTimestamp
         );
         assertTrue(result.isEmpty());
     }
 
     public void testFilterOutMetadataFilesBasedOnAge_EmptyInputList() {
         List<String> metadataFiles = Arrays.asList();
-        TimeValue minimumAge = TimeValue.timeValueSeconds(10);
+        long maximumAllowedTimestamp = System.currentTimeMillis() - 10000;
 
         List<String> result = RemoteStoreUtils.filterOutMetadataFilesBasedOnAge(
             metadataFiles,
             file -> Long.valueOf(file.split("_")[0]),
-            minimumAge
+            maximumAllowedTimestamp
         );
         assertTrue(result.isEmpty());
     }