Adding extra buffer before deleting older generations of translog

Signed-off-by: Gaurav Bafna <[email protected]>
opensearch-project · Oct 21, 2023 · dd548fb · dd548fb
1 parent 41a12e2
commit dd548fb
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 4 deletions.
diff --git a/server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java b/server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java
@@ -70,6 +70,7 @@ public class RemoteFsTranslog extends Translog {
 
     private static final int REMOTE_DELETION_PERMITS = 2;
     private static final int DOWNLOAD_RETRIES = 2;
+    private static long EXTRA_GEN_TO_KEEP = 100;
     public static final String TRANSLOG = "translog";
 
     // Semaphore used to allow only single remote generation to happen at a time
@@ -425,18 +426,20 @@ public void trimUnreferencedReaders() throws IOException {
         // cleans up remote translog files not referenced in latest uploaded metadata.
         // This enables us to restore translog from the metadata in case of failover or relocation.
         Set<Long> generationsToDelete = new HashSet<>();
-        for (long generation = minRemoteGenReferenced - 1; generation >= 0; generation--) {
+        long minGenToDelete = Long.max(minRemoteGenReferenced - 1 - EXTRA_GEN_TO_KEEP, 0);
+        for (long generation = minGenToDelete; generation >= 0; generation--) {
             if (fileTransferTracker.uploaded(Translog.getFilename(generation)) == false) {
                 break;
             }
             generationsToDelete.add(generation);
         }
+
+        translogTransferManager.deleteStaleTranslogMetadataFilesAsync(remoteGenerationDeletionPermits::release);
         if (generationsToDelete.isEmpty() == false) {
             deleteRemoteGeneration(generationsToDelete);
-            translogTransferManager.deleteStaleTranslogMetadataFilesAsync(remoteGenerationDeletionPermits::release);
             deleteStaleRemotePrimaryTerms();
         } else {
-            remoteGenerationDeletionPermits.release(REMOTE_DELETION_PERMITS);
+            remoteGenerationDeletionPermits.release();
         }
     }
 
@@ -544,4 +547,8 @@ public void onUploadFailed(TransferSnapshot transferSnapshot, Exception ex) thro
             }
         }
     }
+
+    static void setExtraGenToKeep(long extraGenToKeep) {
+        EXTRA_GEN_TO_KEEP = extraGenToKeep;
+    }
 }
diff --git a/server/src/test/java/org/opensearch/index/translog/RemoteFsTranslogTests.java b/server/src/test/java/org/opensearch/index/translog/RemoteFsTranslogTests.java
@@ -147,6 +147,7 @@ public void setUp() throws Exception {
         // if a previous test failed we clean up things here
         translogDir = createTempDir();
         translog = create(translogDir);
+        RemoteFsTranslog.setExtraGenToKeep(0);
     }
 
     @Override
@@ -623,10 +624,29 @@ public void testSimpleOperationsUpload() throws Exception {
         translog.setMinSeqNoToKeep(2);
 
         assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));
+
+        // this should not trim as tlog-2 files from remote as we are keeping 1 extra gen as well as metadata files
+        RemoteFsTranslog.setExtraGenToKeep(1);
+        translog.trimUnreferencedReaders();
+        assertEquals(
+            6,
+            blobStoreTransferService.listAll(getTranslogDirectory().add(DATA_DIR).add(String.valueOf(primaryTerm.get()))).size()
+        );
+
+        assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));
+        // this should now trim as tlog-2 files from remote, but not tlog-3 and tlog-4
+        RemoteFsTranslog.setExtraGenToKeep(0);
         translog.trimUnreferencedReaders();
         assertEquals(1, translog.readers.size());
         assertEquals(1, translog.stats().estimatedNumberOfOperations());
-        assertBusy(() -> assertEquals(4, translog.allUploaded().size()));
+        assertBusy(() -> {
+            assertEquals(4, translog.allUploaded().size());
+            assertEquals(
+                4,
+                blobStoreTransferService.listAll(getTranslogDirectory().add(DATA_DIR).add(String.valueOf(primaryTerm.get()))).size()
+            );
+        });
+
     }
 
     public void testMetadataFileDeletion() throws Exception {