OAK-12119 : offline compaction does not persist compacted head into gc.log

oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/GarbageCollectionStrategy.java

@@ -100,4 +100,6 @@ interface Context {
 
     List<String> cleanup(Context context) throws IOException;
 
+    List<String> cleanup(Context context, CompactionResult compactionResult) throws IOException;
+
 }

oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/GarbageCollector.java

@@ -42,6 +42,7 @@
 import org.apache.jackrabbit.oak.segment.file.tar.TarFiles;
 import org.apache.jackrabbit.oak.spi.blob.BlobStore;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 class GarbageCollector {
 
@@ -112,6 +113,15 @@ class GarbageCollector {
      */
     private SegmentGCOptions.GCType lastCompactionType = FULL;
 
+    /**
+     * Result of the last compaction, or {@code null} if no compaction has been
+     * performed since this store was opened or since the last cleanup. Used by
+     * standalone cleanup (e.g. offline compaction via oak-run) to persist the
+     * GC journal entry.
+     */
+    @Nullable
+    private CompactionResult lastCompactionResult;
+
     private volatile boolean cancelRequested;
 
     GarbageCollector(
@@ -292,16 +302,30 @@ synchronized void runTail(GarbageCollectionStrategy strategy) throws IOException
 
     synchronized CompactionResult compactFull(GarbageCollectionStrategy strategy) throws IOException {
         cancelRequested = false;
-        return strategy.compactFull(newGarbageCollectionContext(GC_COUNT.get()));
+        CompactionResult result = strategy.compactFull(newGarbageCollectionContext(GC_COUNT.get()));
+        if (result.requiresGCJournalEntry()) {
+            lastCompactionResult = result;
+        }
+        return result;
     }
 
     synchronized CompactionResult compactTail(GarbageCollectionStrategy strategy) throws IOException {
         cancelRequested = false;
-        return strategy.compactTail(newGarbageCollectionContext(GC_COUNT.get()));
+        CompactionResult result = strategy.compactTail(newGarbageCollectionContext(GC_COUNT.get()));
+        if (result.requiresGCJournalEntry()) {
+            lastCompactionResult = result;
+        }
+        return result;
     }
 
     synchronized List<String> cleanup(GarbageCollectionStrategy strategy) throws IOException {
         cancelRequested = false;
+        CompactionResult compactionResult = lastCompactionResult;
+        if (compactionResult != null) {
+            List<String> result = strategy.cleanup(newGarbageCollectionContext(GC_COUNT.get()), compactionResult);
+            lastCompactionResult = null;
+            return result;
+        }
         return strategy.cleanup(newGarbageCollectionContext(GC_COUNT.get()));
     }
 

oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/SynchronizedGarbageCollectionStrategy.java

@@ -74,4 +74,11 @@ public List<String> cleanup(Context context) throws IOException {
         }
     }
 
+    @Override
+    public List<String> cleanup(Context context, CompactionResult compactionResult) throws IOException {
+        synchronized (lock) {
+            return strategy.cleanup(context, compactionResult);
+        }
+    }
+
 }

oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/DefaultGarbageCollectionStrategyTest.java

@@ -23,48 +23,40 @@
 import org.apache.jackrabbit.oak.segment.SegmentTracker;
 import org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions;
 import org.apache.jackrabbit.oak.segment.file.tar.CleanupContext;
-import org.apache.jackrabbit.oak.segment.spi.persistence.GCGeneration;
 import org.apache.jackrabbit.oak.segment.file.tar.TarFiles;
 import org.apache.jackrabbit.oak.segment.memory.MemoryStore;
+import org.apache.jackrabbit.oak.segment.spi.persistence.GCGeneration;
 import org.junit.Test;
 import org.mockito.Mockito;
 import org.mockito.verification.VerificationMode;
 
 import java.io.IOException;
 
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyLong;
-import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
 public class DefaultGarbageCollectionStrategyTest {
     private final GCJournal journal;
 
     public DefaultGarbageCollectionStrategyTest() {
         journal = Mockito.mock(GCJournal.class);
-        when(journal.read()).thenReturn(Mockito.mock(GCJournal.GCJournalEntry.class));
+        Mockito.when(journal.read()).thenReturn(Mockito.mock(GCJournal.GCJournalEntry.class));
     }
 
     private GarbageCollectionStrategy.Context getMockedGCContext(MemoryStore store) throws IOException {
         GarbageCollectionStrategy.Context mockedContext = Mockito.mock(GarbageCollectionStrategy.Context.class);
 
-        when(mockedContext.getGCListener()).thenReturn(Mockito.mock(GCListener.class));
-        when(mockedContext.getTarFiles()).thenReturn(Mockito.mock(TarFiles.class));
-        when(mockedContext.getSegmentCache()).thenReturn(Mockito.mock(SegmentCache.class));
-        when(mockedContext.getFileStoreStats()).thenReturn(Mockito.mock(FileStoreStats.class));
+        Mockito.when(mockedContext.getGCListener()).thenReturn(Mockito.mock(GCListener.class));
+        Mockito.when(mockedContext.getTarFiles()).thenReturn(Mockito.mock(TarFiles.class));
+        Mockito.when(mockedContext.getSegmentCache()).thenReturn(Mockito.mock(SegmentCache.class));
+        Mockito.when(mockedContext.getFileStoreStats()).thenReturn(Mockito.mock(FileStoreStats.class));
 
         SegmentTracker tracker = new SegmentTracker((msb, lsb) -> new SegmentId(store, msb, lsb));
-        when(mockedContext.getSegmentTracker()).thenReturn(tracker);
-        when(mockedContext.getCompactionMonitor()).thenReturn(GCNodeWriteMonitor.EMPTY);
-        when(mockedContext.getRevisions()).thenReturn(store.getRevisions());
-        when(mockedContext.getGCJournal()).thenReturn(journal);
+        Mockito.when(mockedContext.getSegmentTracker()).thenReturn(tracker);
+        Mockito.when(mockedContext.getCompactionMonitor()).thenReturn(GCNodeWriteMonitor.EMPTY);
+        Mockito.when(mockedContext.getRevisions()).thenReturn(store.getRevisions());
+        Mockito.when(mockedContext.getGCJournal()).thenReturn(journal);
 
         TarFiles mockedTarFiles = Mockito.mock(TarFiles.class);
-        when(mockedContext.getTarFiles()).thenReturn(mockedTarFiles);
-        when(mockedTarFiles.cleanup(any(CleanupContext.class)))
+        Mockito.when(mockedContext.getTarFiles()).thenReturn(mockedTarFiles);
+        Mockito.when(mockedTarFiles.cleanup(Mockito.any(CleanupContext.class)))
                 .thenReturn(Mockito.mock(TarFiles.CleanupResult.class));
 
         return mockedContext;
@@ -77,12 +69,12 @@ private void runCleanup(CompactionResult result) throws IOException {
     }
 
     private void verifyGCJournalPersistence(VerificationMode mode) {
-        verify(journal, mode).persist(
-                anyLong(),
-                anyLong(),
-                any(GCGeneration.class),
-                anyLong(),
-                anyString());
+        Mockito.verify(journal, mode).persist(
+                Mockito.anyLong(),
+                Mockito.anyLong(),
+                Mockito.any(GCGeneration.class),
+                Mockito.anyLong(),
+                Mockito.anyString());
     }
 
     @Test
@@ -94,14 +86,14 @@ public void successfulCompactionPersistsToJournal() throws Exception {
                 RecordId.NULL,
                 0);
         runCleanup(result);
-        verifyGCJournalPersistence(times(1));
+        verifyGCJournalPersistence(Mockito.times(1));
     }
 
     @Test
     public void partialCompactionDoesNotPersistToJournal() throws Exception {
         CompactionResult result = CompactionResult.partiallySucceeded(GCGeneration.NULL, RecordId.NULL, 0);
         runCleanup(result);
-        verifyGCJournalPersistence(never());
+        verifyGCJournalPersistence(Mockito.never());
     }
 
     @Test
@@ -113,18 +105,45 @@ public void skippedCompactionDoesNotPersistToJournal() throws Exception {
                 RecordId.NULL,
                 0);
         runCleanup(result);
-        verifyGCJournalPersistence(never());
+        verifyGCJournalPersistence(Mockito.never());
     }
 
     @Test
     public void nonApplicableCompactionDoesNotPersistToJournal() throws Exception {
         runCleanup(CompactionResult.notApplicable(0));
-        verifyGCJournalPersistence(never());
+        verifyGCJournalPersistence(Mockito.never());
     }
 
     @Test
     public void abortedCompactionDoesNotPersistToJournal() throws Exception {
         runCleanup(CompactionResult.aborted(GCGeneration.NULL, 0));
-        verifyGCJournalPersistence(never());
+        verifyGCJournalPersistence(Mockito.never());
+    }
+
+    @Test
+    public void offlineCompactionAfterSuccessfulTailCompactionPersistsToJournal() throws Exception {
+        CompactionResult result = CompactionResult.succeeded(
+                SegmentGCOptions.GCType.TAIL,
+                GCGeneration.NULL,
+                SegmentGCOptions.defaultGCOptions(),
+                RecordId.NULL,
+                0);
+        runCleanup(result);
+        verifyGCJournalPersistence(Mockito.times(1));
+    }
+
+    @Test
+    public void abortedRetryDoesNotOverwritePriorSucceededResultForJournalPersistence() throws Exception {
+        // Simulates: compactFull -> aborted, compactFull -> succeeded, cleanup.
+        // GarbageCollector stores only the succeeded result (isSuccess() gate),
+        // so cleanup is ultimately called with the succeeded result.
+        runCleanup(CompactionResult.aborted(GCGeneration.NULL, 0));
+        runCleanup(CompactionResult.succeeded(
+                SegmentGCOptions.GCType.FULL,
+                GCGeneration.NULL,
+                SegmentGCOptions.defaultGCOptions(),
+                RecordId.NULL,
+                0));
+        verifyGCJournalPersistence(Mockito.times(1));
     }
 }