Add more logs and fix test setup

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

Author: Sachin Kale

server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java

@@ -694,15 +694,19 @@ public void onResponse(List<BlobMetadata> blobMetadata) {
                         return;
                     }
 
+                    logger.debug("metadataFilesToBeDeleted = {}", metadataFilesToBeDeleted);
                     // For all the files that we are keeping, fetch min and max generations
                     List<String> metadataFilesNotToBeDeleted = new ArrayList<>(metadataFiles);
                     metadataFilesNotToBeDeleted.removeAll(metadataFilesToBeDeleted);
 
+                    logger.debug("metadataFilesNotToBeDeleted = {}", metadataFilesNotToBeDeleted);
                     Set<Long> generationsToBeDeleted = getGenerationsToBeDeleted(
                         metadataFilesNotToBeDeleted,
                         metadataFilesToBeDeleted,
                         indexDeleted
                     );
+
+                    logger.debug("generationsToBeDeleted = {}", generationsToBeDeleted);
                     if (generationsToBeDeleted.isEmpty() == false) {
                         // Delete stale generations
                         translogTransferManager.deleteGenerationAsync(
@@ -782,6 +786,12 @@ protected List<String> getMetadataFilesToBeDeleted(List<String> metadataFiles) {
             pinnedTimestampsState.v1()
         );
 
+        logger.trace(
+            "metadataFiles.size = {}, metadataFilesToBeDeleted based on age based filtering = {}",
+            metadataFiles.size(),
+            metadataFilesToBeDeleted.size()
+        );
+
         // Get md files matching pinned timestamps
         Set<String> implicitLockedFiles = RemoteStoreUtils.getPinnedTimestampLockedFiles(
             metadataFilesToBeDeleted,
@@ -794,6 +804,12 @@ protected List<String> getMetadataFilesToBeDeleted(List<String> metadataFiles) {
         // Filter out metadata files matching pinned timestamps
         metadataFilesToBeDeleted.removeAll(implicitLockedFiles);
 
+        logger.trace(
+            "implicitLockedFiles.size = {}, metadataFilesToBeDeleted based on pinned timestamp filtering = {}",
+            implicitLockedFiles.size(),
+            metadataFilesToBeDeleted.size()
+        );
+
         return metadataFilesToBeDeleted;
     }
 

server/src/test/java/org/opensearch/index/translog/RemoteFsTranslogWithPinnedTimestampTests.java

@@ -97,8 +97,13 @@ public void setUp() throws Exception {
 
         ThreadPool threadPool = mock(ThreadPool.class);
         when(threadPool.schedule(any(), any(), any())).then(invocationOnMock -> {
-            updatePinnedTimstampTask = invocationOnMock.getArgument(0);
-            updatePinnedTimstampTask.run();
+            Runnable updateTask = invocationOnMock.getArgument(0);
+            updatePinnedTimstampTask = () -> {
+                long currentTime = System.currentTimeMillis();
+                while (RemoteStorePinnedTimestampService.getPinnedTimestamps().v1() < currentTime) {
+                    updateTask.run();
+                }
+            };
             return null;
         }).then(subsequentInvocationsOnMock -> null);
 
@@ -190,11 +195,20 @@ public void testIndexDeletionWithNoPinnedTimestampNoRecentMdFiles() throws Excep
         addToTranslogAndListAndUpload(translog, ops, new Translog.Index("3", 3, primaryTerm.get(), new byte[] { 1 }));
         addToTranslogAndListAndUpload(translog, ops, new Translog.Index("4", 4, primaryTerm.get(), new byte[] { 1 }));
 
+        assertBusy(() -> {
+            assertEquals(5, blobStoreTransferService.listAll(getTranslogDirectory().add(METADATA_DIR)).size());
+            assertEquals(
+                12,
+                blobStoreTransferService.listAll(getTranslogDirectory().add(DATA_DIR).add(String.valueOf(primaryTerm.get()))).size()
+            );
+        });
+
         assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));
         updatePinnedTimstampTask.run();
         translog.trimUnreferencedReaders(true, false);
 
         assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));
+
         assertBusy(() -> {
             assertEquals(0, blobStoreTransferService.listAll(getTranslogDirectory().add(METADATA_DIR)).size());
             assertEquals(
@@ -250,6 +264,13 @@ public void testSimpleOperationsUpload() throws Exception {
         addToTranslogAndListAndUpload(translog, ops, new Translog.Index("5", 5, primaryTerm.get(), new byte[] { 1 }));
         addToTranslogAndListAndUpload(translog, ops, new Translog.Index("6", 6, primaryTerm.get(), new byte[] { 1 }));
 
+        assertBusy(() -> {
+            assertEquals(
+                16,
+                blobStoreTransferService.listAll(getTranslogDirectory().add(DATA_DIR).add(String.valueOf(primaryTerm.get()))).size()
+            );
+        });
+
         assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));
 
         RemoteStoreSettings.setPinnedTimestampsLookbackInterval(TimeValue.ZERO);
@@ -356,6 +377,11 @@ public void onResponse(List<BlobMetadata> blobMetadataList) {
                         );
                         when(remoteStorePinnedTimestampsBlobStore.getBlobPathForUpload(any())).thenReturn(new BlobPath());
 
+                        Set<String> dataFilesBeforeTrim = blobStoreTransferService.listAll(
+                            getTranslogDirectory().add(DATA_DIR).add(String.valueOf(primaryTerm.get()))
+                        );
+
+                        assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));
                         updatePinnedTimstampTask.run();
                         RemoteStoreSettings.setPinnedTimestampsLookbackInterval(TimeValue.ZERO);
                         translog.trimUnreferencedReaders();
@@ -366,11 +392,15 @@ public void onResponse(List<BlobMetadata> blobMetadataList) {
                             getTranslogDirectory().add(DATA_DIR).add(String.valueOf(primaryTerm.get()))
                         );
 
-                        // We check for number of pinned timestamp or +1 due to latest metadata.
-                        assertTrue(
-                            metadataFilesAfterTrim.size() == pinnedTimestamps.size()
-                                || metadataFilesAfterTrim.size() == pinnedTimestamps.size() + 1
-                        );
+                        // If non pinned generations are within, minRemoteGenReferenced - 1 - indexSettings().getRemoteTranslogExtraKeep()
+                        // we will not delete them
+                        if (dataFilesAfterTrim.equals(dataFilesBeforeTrim) == false) {
+                            // We check for number of pinned timestamp or +1 due to latest metadata.
+                            assertTrue(
+                                metadataFilesAfterTrim.size() == pinnedTimestamps.size()
+                                    || metadataFilesAfterTrim.size() == pinnedTimestamps.size() + 1
+                            );
+                        }
 
                         for (String md : pinnedTimestampMatchingMetadataFiles) {
                             assertTrue(metadataFilesAfterTrim.contains(md));