OAK-12280: Provide a way to reconcile the offline GC result back

oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/MarkSweepGarbageCollector.java

@@ -39,7 +39,6 @@
 import java.sql.Timestamp;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
@@ -154,6 +153,9 @@ public String apply(@Nullable String input) {
 
     private Clock clock;
 
+    @Nullable
+    private File lastGcSummaryFile;
+
     /**
      * Creates an instance of MarkSweepGarbageCollector
      *
@@ -331,6 +333,7 @@ public OperationsStatsMBean getConsistencyOperationStats() {
      */
     protected void markAndSweep(boolean markOnly, boolean forceBlobRetrieve) throws Exception {
         statsCollector.start();
+        lastGcSummaryFile = null;
         boolean threw = true;
         GarbageCollectorFileState fs = new GarbageCollectorFileState(root);
         Stopwatch sw = Stopwatch.createStarted();
@@ -545,6 +548,20 @@ protected long sweep(GarbageCollectorFileState fs, long markStart, boolean force
         if (checkConsistencyAfterGc) {
             BlobCollectionType.get(blobStore).checkConsistencyAfterGC(blobStore, fs, consistencyStats);
         }
+
+        // Copy the gc summary file out of the gcworkdir so it outlives handleRemoves.
+        // The persistent copy is exposed via getLastGcSummaryFile() for callers that
+        // need to act on the sweep result after collectGarbage() returns.
+        File gcSummary = new File(root, fs.getGarbage().getName());
+        if (fs.getGarbage().exists() && fs.getGarbage().length() > 0) {
+            try {
+                copyFile(fs.getGarbage(), gcSummary);
+                lastGcSummaryFile = gcSummary;
+            } catch (IOException e) {
+                LOG.warn("Could not persist gc summary to [{}]", gcSummary, e);
+            }
+        }
+
         BlobCollectionType.get(blobStore).handleRemoves(blobStore, fs.getGarbage(), fs.getMarkedRefs());
 
         if(count != deleted) {
@@ -809,6 +826,16 @@ public void setClock(Clock clock) {
         this.clock = clock;
     }
 
+    /**
+     * Returns the gc summary file written during the most recent sweep, or {@code null} if no
+     * sweep has run yet or the last sweep deleted nothing. The file persists beyond the lifetime
+     * of the internal working directory and can be used by callers to act on the GC result.
+     */
+    @Nullable
+    public File getLastGcSummaryFile() {
+        return lastGcSummaryFile;
+    }
+
     /**
      * BlobIdRetriever class to retrieve all blob ids.
      */
@@ -1067,13 +1094,19 @@ long sweepInternal(GarbageCollectableBlobStore blobStore, List<String> ids,
             LOG.trace("Blob ids to be deleted {}", ids);
             for (String id : ids) {
                 try {
-                    long deleted = blobStore.countDeleteChunks(new ArrayList<>(Arrays.asList(id)), maxModified);
-                    if (deleted != 1) {
-                        LOG.debug("Blob [{}] not deleted", id);
-                    } else {
+                    long deleted = blobStore.countDeleteChunk(id, maxModified);
+                    if (deleted == 1) {
                         exceptionQueue.add(id);
                         totalDeleted += 1;
+                    } else if (deleted < 0) {
+                        LOG.info("Blob [{}] no longer exists in data store; removing from tracker state", id);
+                        exceptionQueue.add(id);
+                    } else {
+                        LOG.debug("Blob [{}] not deleted", id);
                     }
+                } catch (DataStoreException e) {
+                   LOG.info("Blob [{}] not found in data store; removing from tracker state", id);
+                    exceptionQueue.add(id);
                 } catch (Exception e) {
                     LOG.warn("Error occurred while deleting blob with id [{}]", id, e);
                 }

oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreBlobStore.java

@@ -174,7 +174,7 @@ public DataRecord getRecordIfStored(DataIdentifier identifier) throws DataStoreE
                     delegate.getRecordIfStored(identifier);
 
             long elapsed = System.nanoTime() - start;
-            stats.getRecordIfStoredCalled(elapsed, TimeUnit.NANOSECONDS, rec.getLength());
+            stats.getRecordIfStoredCalled(elapsed, TimeUnit.NANOSECONDS, rec != null ? rec.getLength() : 0);
             stats.getRecordIfStoredCompleted(identifier.toString());
 
             return rec;
@@ -509,42 +509,54 @@ public boolean deleteChunks(List<String> chunkIds, long maxLastModifiedTime) thr
         return (chunkIds.size() == countDeleteChunks(chunkIds, maxLastModifiedTime));
     }
 
+    @Override
+    public long countDeleteChunk(String chunkId, long maxLastModifiedTime) throws Exception {
+        if (!(delegate instanceof MultiDataStoreAware)) {
+            return 0;
+        }
+        long start = System.nanoTime();
+        String blobId = extractBlobId(chunkId);
+        DataIdentifier identifier = new DataIdentifier(blobId);
+        try {
+            DataRecord dataRecord = getRecordIfStored(identifier);
+            if (dataRecord == null) {
+                log.info("Blob [{}] not found in data store", blobId);
+                return -1;
+            }
+            boolean eligible = (maxLastModifiedTime <= 0) || dataRecord.getLastModified() <= maxLastModifiedTime;
+            log.trace("Deleting blob [{}] with last modified date [{}] : [{}]", blobId,
+                    dataRecord.getLastModified(), eligible);
+            if (eligible) {
+                ((MultiDataStoreAware) delegate).deleteRecord(identifier);
+            }
+            stats.deleted(blobId, System.nanoTime() - start, TimeUnit.NANOSECONDS);
+            stats.deleteCompleted(blobId);
+            return eligible ? 1 : 0;
+        } catch (Exception e) {
+            stats.deleteFailed();
+            throw e;
+        }
+    }
+
     @Override
     public long countDeleteChunks(List<String> chunkIds, long maxLastModifiedTime) throws Exception {
-        int count = 0;
+        long count = 0;
         if (delegate instanceof MultiDataStoreAware) {
-            try {
-                List<String> deleted = new ArrayList<>(512);
-                for (String chunkId : chunkIds) {
-                    long start = System.nanoTime();
-
-                    String blobId = extractBlobId(chunkId);
-                    DataIdentifier identifier = new DataIdentifier(blobId);
-                    DataRecord dataRecord = getRecordForId(identifier);
-                    boolean success = (maxLastModifiedTime <= 0)
-                            || dataRecord.getLastModified() <= maxLastModifiedTime;
-                    log.trace("Deleting blob [{}] with last modified date [{}] : [{}]", blobId,
-                            dataRecord.getLastModified(), success);
-                    if (success) {
-                        ((MultiDataStoreAware) delegate).deleteRecord(identifier);
-                        deleted.add(blobId);
-                        count++;
-                        if (count % 512 == 0) {
-                            log.info("Deleted blobs {}", deleted);
-                            deleted.clear();
-                        }
+            List<String> deleted = new ArrayList<>(512);
+            for (String chunkId : chunkIds) {
+                long result = countDeleteChunk(chunkId, maxLastModifiedTime);
+                if (result == 1) {
+                    deleted.add(extractBlobId(chunkId));
+                    count++;
+                    if (count % 512 == 0) {
+                        log.info("Deleted blobs {}", deleted);
+                        deleted.clear();
                     }
-
-                    stats.deleted(blobId, System.nanoTime() - start, TimeUnit.NANOSECONDS);
-                    stats.deleteCompleted(blobId);
-                }
-                if (!deleted.isEmpty()) {
-                    log.info("Deleted blobs {}", deleted);
                 }
+                // result == -1 means ghost blob — skip silently; caller cleans tracker state
             }
-            catch (Exception e) {
-                stats.deleteFailed();
-                throw e;
+            if (!deleted.isEmpty()) {
+                log.info("Deleted blobs {}", deleted);
             }
         }
         return count;

oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreMarkSweepGarbageCollectorTest.java

@@ -45,7 +45,10 @@
 
 import static org.apache.jackrabbit.oak.plugins.blob.SharedDataStore.Type.SHARED;
 import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -139,6 +142,27 @@ private void setupSharedDataRecords(final String refRepoId, final String repoRep
     when(blobStore.getAllMetadataRecords(SharedDataStoreUtils.SharedStoreRecordType.MARKED_START_MARKER.getType())).thenReturn(refs);
   }
 
+  @Test
+  public void sweepShouldNotFailWhenGhostBlobCountDeleteChunkReturnsMinusOne() throws Exception {
+    setupSharedDataRecords("REPO1", "REPO1");
+    when(blobStore.getAllChunkIds(0L)).thenReturn(List.of("ghost-blob-id").iterator());
+    when(blobStore.countDeleteChunk(anyString(), anyLong())).thenReturn(-1L);
+    collector.markAndSweep(false, true);
+    assertEquals(0L, collector.getOperationStats().numDeleted());
+    assertEquals(0L, collector.getOperationStats().getFailureCount());
+  }
+
+  @Test
+  public void sweepShouldNotFailWhenGhostBlobCountDeleteChunkThrowsDataStoreException() throws Exception {
+    setupSharedDataRecords("REPO1", "REPO1");
+    when(blobStore.getAllChunkIds(0L)).thenReturn(List.of("ghost-blob-id").iterator());
+    when(blobStore.countDeleteChunk(anyString(), anyLong()))
+        .thenThrow(new DataStoreException("Record does not exist"));
+    collector.markAndSweep(false, true);
+    assertEquals(0L, collector.getOperationStats().numDeleted());
+    assertEquals(0L, collector.getOperationStats().getFailureCount());
+  }
+
   private interface MockGarbageCollectableSharedDataStore extends GarbageCollectableBlobStore, SharedDataStore {
   }
 }

oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreBlobStoreTest.java

@@ -54,6 +54,8 @@
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 public class DataStoreBlobStoreTest extends AbstractBlobStoreTest {
@@ -245,6 +247,35 @@ public void testEmptyIdentifier() throws Exception {
     public void testGarbageCollection() throws Exception {
     }
 
+    @Test
+    public void testCountDeleteChunkReturnsMinusOneForGhostBlob() throws Exception {
+        DataIdentifier ghostId = new DataIdentifier("ghostblobid");
+
+        OakFileDataStore mockedDS = mock(OakFileDataStore.class);
+        when(mockedDS.getMinRecordLength()).thenReturn(0);
+        when(mockedDS.getRecordIfStored(ghostId)).thenReturn(null);
+        try (DataStoreBlobStore ds = new DataStoreBlobStore(mockedDS)) {
+            long result = ds.countDeleteChunk("ghostblobid", 0L);
+
+            assertEquals(-1, result);
+            // deleteRecord must NOT have been called for a ghost blob
+            verify(mockedDS, never()).deleteRecord(ghostId);
+        }
+    }
+
+
+    @Test
+    public void testGetRecordIfStoredReturnsNull() throws DataStoreException {
+        DataIdentifier missingId = new DataIdentifier("missingblob");
+
+        DataStore mockedDS = mock(DataStore.class);
+        when(mockedDS.getMinRecordLength()).thenReturn(0);
+        when(mockedDS.getRecordIfStored(missingId)).thenReturn(null);
+        try (DataStoreBlobStore ds = new DataStoreBlobStore(mockedDS)) {
+            assertNull(ds.getRecordIfStored(missingId));
+        }
+    }
+
     @After
     @Override
     public void tearDown() throws Exception {

oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/GarbageCollectableBlobStore.java

@@ -110,6 +110,32 @@ public interface GarbageCollectableBlobStore extends BlobStore {
      */
     long countDeleteChunks(List<String> chunkIds, long maxLastModifiedTime) throws Exception;
 
+    /**
+     * Deletes a single blob with the given id.
+     *
+     * <p>Return values:
+     * <ul>
+     *   <li>{@code 1}  &mdash; blob was found and successfully deleted</li>
+     *   <li>{@code 0}  &mdash; blob was found but skipped (too recent per {@code maxLastModifiedTime})</li>
+     *   <li>{@code -1} &mdash; blob was not found in the data store (ghost blob — e.g. already
+     *       removed by offline GC); the caller should remove the id from any tracker state to
+     *       avoid repeated warnings on future GC runs</li>
+     * </ul>
+     *
+     * <p>The default implementation delegates to {@link #countDeleteChunks(List, long)} with a
+     * single-element list.  Implementations that can distinguish ghost blobs from ordinary
+     * deletion failures (e.g. {@code DataStoreBlobStore}) should override this method.
+     *
+     * @param chunkId the chunk id to delete
+     * @param maxLastModifiedTime the max last modified time to consider for retrieval,
+     *            with the special value '0' meaning no filtering by time
+     * @return the deletion outcome: {@code 1} deleted, {@code 0} skipped, {@code -1} ghost blob
+     * @throws Exception the exception
+     */
+    default long countDeleteChunk(String chunkId, long maxLastModifiedTime) throws Exception {
+        return countDeleteChunks(List.of(chunkId), maxLastModifiedTime);
+    }
+
     /**
      * Resolve chunks stored in the blob store from the given Id.
      * This will not return any chunks stored in-line in the id.

oak-run/README.md

@@ -635,6 +635,7 @@ Maintenance commands for the DataStore:
             [--out-dir <output_path>] \
             [--work-dir <temporary_path>] \
             [--max-age <seconds>] \
+            [--repository-home <repository_home>] \
             [--verbose] \
             [--verboseRootPath] \
             [--useDirListing] \
@@ -683,7 +684,16 @@ The following options are available:
     --sweep-only-refs-past-retention - Sweep only if the earliest references from all repositories are past the retention period which is govered by the max-age parameter.
                                        Boolean (Optional). Defaults to False. Only applicable for --collect-garbage
     --check-consistency-gc    - Performs a consistency check immediately after the GC.        
-                                Boolean (Optional). Defaults to False. Only applicable for --collect-garbage                           
+                                 Boolean (Optional). Defaults to False. Only applicable for --collect-garbage
+    --repository-home         - Repository home of the running Oak instance, i.e. the parent of the
+                                'blobids' BlobIdTracker directory (the tracker lives at <repository-home>/blobids).
+                                Optional: when omitted it is derived from the segment store path (its parent),
+                                which matches a standard deployment. After --collect-garbage (non mark-only) the
+                                BlobIdTracker is reconciled automatically: blob IDs deleted by this run are removed
+                                from the local tracker files and the DataStore snapshot, so online GC no longer
+                                warns about (or fails to clean up) already-deleted blobs on subsequent runs.
+                                Only applicable to a segment NodeStore backed by a FileDataStore (SharedDataStore);
+                                pass this explicitly for a document NodeStore or a non-standard layout.
 Note:
 
 Note: When using --export-metrics the following additional jars have to be downloaded to support Prometheus Pushgatway