This is an automated email from the ASF dual-hosted git repository.
daim pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new aa9c5698c2 OAK-12119 : offline compaction does not persist compacted
head into gc.log (#2779)
aa9c5698c2 is described below
commit aa9c5698c21214f6bc504bf535b9539cfa49cc40
Author: Rishabh Kumar <[email protected]>
AuthorDate: Thu Mar 5 16:28:59 2026 +0530
OAK-12119 : offline compaction does not persist compacted head into gc.log
(#2779)
* OAK-12119 : offline compaction does not persist compacted head into gc.log
* OAK-12119 : fix cleanup fallback path and rename tests to camelCase
* OAK-12119 : improve test coverage and fix static imports
* OAK-12119 : remove volatile from lastCompactionResult and fix test setup
---
.../segment/file/GarbageCollectionStrategy.java | 2 +
.../oak/segment/file/GarbageCollector.java | 28 +-
.../SynchronizedGarbageCollectionStrategy.java | 7 +
.../file/DefaultGarbageCollectionStrategyTest.java | 81 +++---
.../GarbageCollectorOfflineCompactionTest.java | 281 +++++++++++++++++++++
.../segment/file/OfflineCompactionGcLogTest.java | 2 -
.../SynchronizedGarbageCollectionStrategyTest.java | 93 +++++++
7 files changed, 459 insertions(+), 35 deletions(-)
diff --git
a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/GarbageCollectionStrategy.java
b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/GarbageCollectionStrategy.java
index eba6c6e824..5f757da972 100644
---
a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/GarbageCollectionStrategy.java
+++
b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/GarbageCollectionStrategy.java
@@ -100,4 +100,6 @@ interface GarbageCollectionStrategy {
List<String> cleanup(Context context) throws IOException;
+ List<String> cleanup(Context context, CompactionResult compactionResult)
throws IOException;
+
}
diff --git
a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/GarbageCollector.java
b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/GarbageCollector.java
index a6db420773..af80e3f6f5 100644
---
a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/GarbageCollector.java
+++
b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/GarbageCollector.java
@@ -42,6 +42,7 @@ import
org.apache.jackrabbit.oak.segment.spi.persistence.GCGeneration;
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 @@ class GarbageCollector {
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()));
}
diff --git
a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/SynchronizedGarbageCollectionStrategy.java
b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/SynchronizedGarbageCollectionStrategy.java
index f301b40c04..9341ce6861 100644
---
a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/SynchronizedGarbageCollectionStrategy.java
+++
b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/SynchronizedGarbageCollectionStrategy.java
@@ -74,4 +74,11 @@ class SynchronizedGarbageCollectionStrategy implements
GarbageCollectionStrategy
}
}
+ @Override
+ public List<String> cleanup(Context context, CompactionResult
compactionResult) throws IOException {
+ synchronized (lock) {
+ return strategy.cleanup(context, compactionResult);
+ }
+ }
+
}
diff --git
a/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/DefaultGarbageCollectionStrategyTest.java
b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/DefaultGarbageCollectionStrategyTest.java
index e0d4a2d7ae..20b92dba70 100644
---
a/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/DefaultGarbageCollectionStrategyTest.java
+++
b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/DefaultGarbageCollectionStrategyTest.java
@@ -23,48 +23,40 @@ import org.apache.jackrabbit.oak.segment.SegmentId;
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 @@ public class DefaultGarbageCollectionStrategyTest {
}
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 class DefaultGarbageCollectionStrategyTest {
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 class DefaultGarbageCollectionStrategyTest {
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));
}
}
diff --git
a/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/GarbageCollectorOfflineCompactionTest.java
b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/GarbageCollectorOfflineCompactionTest.java
new file mode 100644
index 0000000000..5629254606
--- /dev/null
+++
b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/GarbageCollectorOfflineCompactionTest.java
@@ -0,0 +1,281 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.jackrabbit.oak.segment.file;
+
+import org.apache.jackrabbit.oak.segment.RecordId;
+import org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions;
+import org.apache.jackrabbit.oak.segment.file.cancel.Canceller;
+import org.apache.jackrabbit.oak.segment.spi.persistence.GCGeneration;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
+
+import java.io.IOException;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * Verifies the {@code lastCompactionResult} lifecycle inside {@link
GarbageCollector}
+ * for the offline compaction path, covering all combinations of {@code
compactFull()}
+ * and {@code cleanup()} calls described in the scenario matrix:
+ *
+ * <pre>
+ * compact(ok) → cleanup journal written
+ * compact(ok) → compact(ok) → cleanup gen2 supersedes gen1
+ * compact(ok) → compact(abort) → cleanup abort does not clobber gen1
+ * compact(abort) → compact(ok) → cleanup journal written for gen2
+ * compact(abort) → cleanup journal NOT written
+ * cleanup → compact(ok) → cleanup journal written only for second
cleanup
+ * compact(ok) → cleanup → cleanup second cleanup is a no-op
+ * </pre>
+ *
+ * <p>The strategy is mocked so that specific {@link CompactionResult}
instances can be
+ * injected. {@link ArgumentCaptor} is then used to assert exactly which
result was
+ * forwarded to {@code strategy.cleanup()}, proving that the 2-arg
+ * {@code cleanup(Context, CompactionResult)} path (journal entry required) vs
the 1-arg
+ * {@code cleanup(Context)} path (no journal entry) is chosen correctly.
+ */
+public class GarbageCollectorOfflineCompactionTest {
+
+ private GarbageCollectionStrategy strategy;
+ private GarbageCollector collector;
+
+ @Before
+ public void setUp() {
+ strategy = Mockito.mock(GarbageCollectionStrategy.class);
+ collector = new GarbageCollector(
+ SegmentGCOptions.defaultGCOptions(),
+ Mockito.mock(GCListener.class),
+ null, // gcJournal — not needed; strategy is
mocked
+ new AtomicBoolean(true),
+ null, // fileReaper
+ null, // tarFiles
+ null, // tracker
+ null, // segmentReader
+ () -> null, // revisionsSupplier
+ null, // blobStore
+ null, // segmentCache
+ null, // segmentWriter
+ null, // stats
+ Canceller.newCanceller(),
+ () -> {}, // flusher
+ null // segmentWriterFactory
+ );
+ }
+
+ private CompactionResult succeeded(int gen) {
+ return CompactionResult.succeeded(
+ SegmentGCOptions.GCType.FULL,
+ GCGeneration.newGCGeneration(gen, gen, false),
+ SegmentGCOptions.defaultGCOptions(),
+ RecordId.NULL,
+ gen
+ );
+ }
+
+ private CompactionResult aborted(int gen) {
+ return CompactionResult.aborted(GCGeneration.newGCGeneration(gen, gen,
false), gen);
+ }
+
+ // -----------------------------------------------------------------------
+ // Scenario: compactTail(ok) → cleanup
+ // The compactTail path stores lastCompactionResult identically to
+ // compactFull; the succeeded result must reach the 2-arg cleanup.
+ // -----------------------------------------------------------------------
+
+ @Test
+ public void testCompactTailOkCleanupJournalWritten() throws IOException {
+ CompactionResult result = succeeded(1);
+
Mockito.when(strategy.compactTail(Mockito.any(GarbageCollectionStrategy.Context.class)))
+ .thenReturn(result);
+
+ collector.compactTail(strategy);
+ collector.cleanup(strategy);
+
+ ArgumentCaptor<CompactionResult> captor =
ArgumentCaptor.forClass(CompactionResult.class);
+ Mockito.verify(strategy).cleanup(
+ Mockito.any(GarbageCollectionStrategy.Context.class),
captor.capture());
+ Assert.assertSame(result, captor.getValue());
+ Assert.assertTrue(captor.getValue().requiresGCJournalEntry());
+ Mockito.verify(strategy, Mockito.never())
+ .cleanup(Mockito.any(GarbageCollectionStrategy.Context.class));
+ }
+
+ // -----------------------------------------------------------------------
+ // Scenario: compact(ok) → cleanup
+ // The succeeded result must be passed to the 2-arg cleanup.
+ // -----------------------------------------------------------------------
+
+ @Test
+ public void testCompactOkCleanupJournalWritten() throws IOException {
+ CompactionResult result = succeeded(1);
+
Mockito.when(strategy.compactFull(Mockito.any(GarbageCollectionStrategy.Context.class)))
+ .thenReturn(result);
+
+ collector.compactFull(strategy);
+ collector.cleanup(strategy);
+
+ ArgumentCaptor<CompactionResult> captor =
ArgumentCaptor.forClass(CompactionResult.class);
+ Mockito.verify(strategy).cleanup(
+ Mockito.any(GarbageCollectionStrategy.Context.class),
captor.capture());
+ Assert.assertSame(result, captor.getValue());
+ Assert.assertTrue(captor.getValue().requiresGCJournalEntry());
+ Mockito.verify(strategy, Mockito.never())
+ .cleanup(Mockito.any(GarbageCollectionStrategy.Context.class));
+ }
+
+ // -----------------------------------------------------------------------
+ // Scenario: compact(ok) → compact(ok) → cleanup
+ // The second succeeded result supersedes the first.
+ // -----------------------------------------------------------------------
+
+ @Test
+ public void testCompactOkCompactOkCleanupGen2Supersedes() throws
IOException {
+ CompactionResult gen1 = succeeded(1);
+ CompactionResult gen2 = succeeded(2);
+
Mockito.when(strategy.compactFull(Mockito.any(GarbageCollectionStrategy.Context.class)))
+ .thenReturn(gen1)
+ .thenReturn(gen2);
+
+ collector.compactFull(strategy);
+ collector.compactFull(strategy);
+ collector.cleanup(strategy);
+
+ ArgumentCaptor<CompactionResult> captor =
ArgumentCaptor.forClass(CompactionResult.class);
+ Mockito.verify(strategy).cleanup(
+ Mockito.any(GarbageCollectionStrategy.Context.class),
captor.capture());
+ Assert.assertSame("gen2 must supersede gen1", gen2, captor.getValue());
+ }
+
+ // -----------------------------------------------------------------------
+ // Scenario: compact(ok) → compact(abort) → cleanup
+ // An aborted result must not overwrite the previous succeeded result.
+ // -----------------------------------------------------------------------
+
+ @Test
+ public void testCompactOkCompactAbortCleanupAbortDoesNotClobber() throws
IOException {
+ CompactionResult gen1 = succeeded(1);
+
Mockito.when(strategy.compactFull(Mockito.any(GarbageCollectionStrategy.Context.class)))
+ .thenReturn(gen1)
+ .thenReturn(aborted(1));
+
+ collector.compactFull(strategy);
+ collector.compactFull(strategy);
+ collector.cleanup(strategy);
+
+ ArgumentCaptor<CompactionResult> captor =
ArgumentCaptor.forClass(CompactionResult.class);
+ Mockito.verify(strategy).cleanup(
+ Mockito.any(GarbageCollectionStrategy.Context.class),
captor.capture());
+ Assert.assertSame("abort must not clobber the gen1 succeeded result",
gen1, captor.getValue());
+ }
+
+ // -----------------------------------------------------------------------
+ // Scenario: compact(abort) → compact(ok) → cleanup
+ // The initial abort must not prevent a subsequent succeeded result from
+ // being used in cleanup.
+ // -----------------------------------------------------------------------
+
+ @Test
+ public void testCompactAbortCompactOkCleanupJournalWritten() throws
IOException {
+ CompactionResult gen2 = succeeded(2);
+
Mockito.when(strategy.compactFull(Mockito.any(GarbageCollectionStrategy.Context.class)))
+ .thenReturn(aborted(0))
+ .thenReturn(gen2);
+
+ collector.compactFull(strategy);
+ collector.compactFull(strategy);
+ collector.cleanup(strategy);
+
+ ArgumentCaptor<CompactionResult> captor =
ArgumentCaptor.forClass(CompactionResult.class);
+ Mockito.verify(strategy).cleanup(
+ Mockito.any(GarbageCollectionStrategy.Context.class),
captor.capture());
+ Assert.assertSame(gen2, captor.getValue());
+ Assert.assertTrue(captor.getValue().requiresGCJournalEntry());
+ }
+
+ // -----------------------------------------------------------------------
+ // Scenario: compact(abort) → cleanup
+ // No succeeded result is available; cleanup must use the 1-arg (skipped)
+ // path — no journal entry.
+ // -----------------------------------------------------------------------
+
+ @Test
+ public void testCompactAbortCleanupNoJournalEntry() throws IOException {
+
Mockito.when(strategy.compactFull(Mockito.any(GarbageCollectionStrategy.Context.class)))
+ .thenReturn(aborted(0));
+
+ collector.compactFull(strategy);
+ collector.cleanup(strategy);
+
+ Mockito.verify(strategy)
+ .cleanup(Mockito.any(GarbageCollectionStrategy.Context.class));
+ Mockito.verify(strategy, Mockito.never())
+ .cleanup(Mockito.any(GarbageCollectionStrategy.Context.class),
+ Mockito.any(CompactionResult.class));
+ }
+
+ // -----------------------------------------------------------------------
+ // Scenario: cleanup → compact(ok) → cleanup
+ // The first (pre-compaction) cleanup must not write a journal entry
+ // only the second cleanup (after a succeeded compaction) must.
+ // -----------------------------------------------------------------------
+
+ @Test
+ public void testCleanupCompactOkCleanupJournalWrittenOnlyForSecond()
throws IOException {
+ CompactionResult result = succeeded(1);
+
Mockito.when(strategy.compactFull(Mockito.any(GarbageCollectionStrategy.Context.class)))
+ .thenReturn(result);
+
+ collector.cleanup(strategy); // no preceding compact — 1-arg
path
+ collector.compactFull(strategy);
+ collector.cleanup(strategy); // succeeded result available —
2-arg path
+
+ Mockito.verify(strategy, Mockito.times(1))
+ .cleanup(Mockito.any(GarbageCollectionStrategy.Context.class));
+ ArgumentCaptor<CompactionResult> captor =
ArgumentCaptor.forClass(CompactionResult.class);
+ Mockito.verify(strategy, Mockito.times(1))
+ .cleanup(Mockito.any(GarbageCollectionStrategy.Context.class),
captor.capture());
+ Assert.assertSame(result, captor.getValue());
+ }
+
+ // -----------------------------------------------------------------------
+ // Scenario: compact(ok) → cleanup → cleanup
+ // The first cleanup consumes lastCompactionResult; the second must fall
+ // back to the 1-arg (skipped) path — no duplicate journal entry.
+ // -----------------------------------------------------------------------
+
+ @Test
+ public void testCompactOkCleanupCleanupSecondCleanupIsNoop() throws
IOException {
+ CompactionResult result = succeeded(1);
+
Mockito.when(strategy.compactFull(Mockito.any(GarbageCollectionStrategy.Context.class)))
+ .thenReturn(result);
+
+ collector.compactFull(strategy);
+ collector.cleanup(strategy); // 2-arg path; clears
lastCompactionResult
+ collector.cleanup(strategy); // 1-arg path; lastCompactionResult is
null
+
+ Mockito.verify(strategy, Mockito.times(1))
+ .cleanup(Mockito.any(GarbageCollectionStrategy.Context.class),
+ Mockito.any(CompactionResult.class));
+ Mockito.verify(strategy, Mockito.times(1))
+ .cleanup(Mockito.any(GarbageCollectionStrategy.Context.class));
+ }
+
+}
diff --git
a/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/OfflineCompactionGcLogTest.java
b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/OfflineCompactionGcLogTest.java
index a344df7419..3f13bc11bc 100644
---
a/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/OfflineCompactionGcLogTest.java
+++
b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/OfflineCompactionGcLogTest.java
@@ -24,7 +24,6 @@ import
org.apache.jackrabbit.oak.segment.file.GCJournal.GCJournalEntry;
import org.apache.jackrabbit.oak.segment.file.tar.TarPersistence;
import org.apache.jackrabbit.oak.segment.spi.persistence.GCGeneration;
import org.junit.Assert;
-import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
@@ -43,7 +42,6 @@ import static
org.apache.jackrabbit.oak.segment.file.FileStoreBuilder.fileStoreB
* and pass once the fix that retains the {@code CompactionResult} across the
two calls
* is applied.
*/
-@Ignore
public class OfflineCompactionGcLogTest {
@Rule
diff --git
a/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/SynchronizedGarbageCollectionStrategyTest.java
b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/SynchronizedGarbageCollectionStrategyTest.java
new file mode 100644
index 0000000000..26ce1ae50d
--- /dev/null
+++
b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/SynchronizedGarbageCollectionStrategyTest.java
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.segment.file;
+
+import org.apache.jackrabbit.oak.segment.RecordId;
+import org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions;
+import org.apache.jackrabbit.oak.segment.spi.persistence.GCGeneration;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.io.IOException;
+import java.util.Collections;
+
+/**
+ * Verifies that {@link SynchronizedGarbageCollectionStrategy} correctly
+ * delegates all method calls to the wrapped {@link GarbageCollectionStrategy}.
+ */
+public class SynchronizedGarbageCollectionStrategyTest {
+
+ private GarbageCollectionStrategy delegate;
+ private SynchronizedGarbageCollectionStrategy strategy;
+
+ @Before
+ public void setUp() {
+ delegate = Mockito.mock(GarbageCollectionStrategy.class);
+ strategy = new SynchronizedGarbageCollectionStrategy(delegate);
+ }
+
+ @Test
+ public void testCleanupWithCompactionResultDelegatesToWrappedStrategy()
throws IOException {
+ GarbageCollectionStrategy.Context context =
Mockito.mock(GarbageCollectionStrategy.Context.class);
+ CompactionResult compactionResult = CompactionResult.succeeded(
+ SegmentGCOptions.GCType.FULL,
+ GCGeneration.NULL,
+ SegmentGCOptions.defaultGCOptions(),
+ RecordId.NULL,
+ 0);
+ Mockito.when(delegate.cleanup(context,
compactionResult)).thenReturn(Collections.emptyList());
+
+ strategy.cleanup(context, compactionResult);
+
+ Mockito.verify(delegate).cleanup(context, compactionResult);
+ }
+
+ @Test
+ public void testCleanupDelegatesToWrappedStrategy() throws IOException {
+ GarbageCollectionStrategy.Context context =
Mockito.mock(GarbageCollectionStrategy.Context.class);
+
Mockito.when(delegate.cleanup(context)).thenReturn(Collections.emptyList());
+
+ strategy.cleanup(context);
+
+ Mockito.verify(delegate).cleanup(context);
+ }
+
+ @Test
+ public void testCompactFullDelegatesToWrappedStrategy() throws IOException
{
+ GarbageCollectionStrategy.Context context =
Mockito.mock(GarbageCollectionStrategy.Context.class);
+ CompactionResult expected =
CompactionResult.aborted(GCGeneration.NULL, 0);
+ Mockito.when(delegate.compactFull(context)).thenReturn(expected);
+
+ CompactionResult actual = strategy.compactFull(context);
+
+ Mockito.verify(delegate).compactFull(context);
+ org.junit.Assert.assertSame(expected, actual);
+ }
+
+ @Test
+ public void testCompactTailDelegatesToWrappedStrategy() throws IOException
{
+ GarbageCollectionStrategy.Context context =
Mockito.mock(GarbageCollectionStrategy.Context.class);
+ CompactionResult expected =
CompactionResult.aborted(GCGeneration.NULL, 0);
+ Mockito.when(delegate.compactTail(context)).thenReturn(expected);
+
+ CompactionResult actual = strategy.compactTail(context);
+
+ Mockito.verify(delegate).compactTail(context);
+ org.junit.Assert.assertSame(expected, actual);
+ }
+}
