Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/MarkSweepGarbageCollector.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/MarkSweepGarbageCollector.java (revision 1583235) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/MarkSweepGarbageCollector.java (working copy) @@ -99,7 +99,7 @@ private final boolean debugMode = Boolean.getBoolean("debugModeGC") || LOG.isDebugEnabled(); /** Flag to indicate the state of the gc **/ - private String state; + private String state = NOT_RUNNING; /** * Gets the max last modified interval considered for garbage collection. @@ -174,7 +174,7 @@ * @param maxLastModifiedInterval * @throws IOException Signals that an I/O exception has occurred. */ - public void init( + public MarkSweepGarbageCollector( BlobReferenceRetriever marker, GarbageCollectableBlobStore blobStore, String root, @@ -183,12 +183,14 @@ int maxSweeperThreads, long maxLastModifiedInterval) throws IOException { + this.blobStore = blobStore; + this.marker = marker; this.batchCount = batchCount; this.root = root; this.runConcurrently = runBackendConcurrently; this.numSweepers = maxSweeperThreads; this.maxLastModifiedInterval = maxLastModifiedInterval; - init(marker, blobStore); + fs = new GarbageCollectorFileState(root); } /** @@ -198,7 +200,9 @@ * @param blobStore * @throws IOException Signals that an I/O exception has occurred. */ - public void init(BlobReferenceRetriever marker, GarbageCollectableBlobStore blobStore) + public MarkSweepGarbageCollector( + BlobReferenceRetriever marker, + GarbageCollectableBlobStore blobStore) throws IOException { Preconditions.checkState(!Strings.isNullOrEmpty(root)); @@ -357,7 +361,9 @@ executorService.shutdown(); executorService.awaitTermination(100, TimeUnit.MINUTES); } catch (InterruptedException e) { - e.printStackTrace(); + LOG.error("Exception while waiting for termination of the executor service", e); + LOG.error("Immediately shutdown"); + executorService.shutdownNow(); } count -= exceptionQueue.size(); @@ -372,7 +378,7 @@ IOUtils.closeQuietly(writer); } - LOG.debug("Blobs deleted count - " + count); + LOG.info("Blobs deleted count - " + count); LOG.debug("Ending sweep phase of the garbage collector"); } finally { fs.complete(); @@ -424,6 +430,7 @@ @Override public void run() { try { + LOG.debug("Deleting blobs : " + ids); boolean deleted = blobStore.deleteChunks(ids, (maxLastModifiedInterval > 0 ? System.currentTimeMillis() @@ -432,7 +439,7 @@ exceptionQueue.addAll(ids); } } catch (Exception e) { - e.printStackTrace(); + LOG.error("Error in deleting blobs - " + ids, e); exceptionQueue.addAll(ids); } } @@ -539,7 +546,7 @@ fs.sort(fs.getAvailableRefs()); LOG.debug("Ending retrieving all blobs : " + blobsCount); } catch (Exception e) { - e.printStackTrace(); + LOG.error("Error retrieving available blob ids", e); } finally { IOUtils.closeQuietly(bufferWriter); } Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (revision 1583235) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (working copy) @@ -1615,10 +1615,11 @@ private MarkSweepGarbageCollector createBlobGarbageCollector(BlobStore blobStore) { MarkSweepGarbageCollector blobGC = null; if(blobStore instanceof GarbageCollectableBlobStore){ - blobGC = new MarkSweepGarbageCollector(); + blobGC = null; try { - blobGC.init(new DocumentBlobReferenceRetriever(this), - (GarbageCollectableBlobStore) blobStore); + blobGC = new MarkSweepGarbageCollector( + new DocumentBlobReferenceRetriever(this), + (GarbageCollectableBlobStore) blobStore); } catch (IOException e) { throw new RuntimeException("Error occurred while initializing " + "the MarkSweepGarbageCollector",e); Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeStoreService.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeStoreService.java (revision 1583235) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeStoreService.java (working copy) @@ -174,8 +174,8 @@ RevisionGCMBean.TYPE, "Segment node store revision garbage collection"); if (blobStore instanceof GarbageCollectableBlobStore) { - MarkSweepGarbageCollector gc = new MarkSweepGarbageCollector(); - gc.init(new SegmentBlobReferenceRetriever(store.getTracker()), + MarkSweepGarbageCollector gc = new MarkSweepGarbageCollector( + new SegmentBlobReferenceRetriever(store.getTracker()), (GarbageCollectableBlobStore) blobStore); blobGCRegistration = registerMBean(whiteboard, BlobGCMBean.class, new BlobGC(gc, executor), BlobGCMBean.TYPE, "Segment node store blob garbage collection"); Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AbstractMongoConnectionTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AbstractMongoConnectionTest.java (revision 1583235) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AbstractMongoConnectionTest.java (working copy) @@ -18,6 +18,7 @@ import org.apache.jackrabbit.mk.api.MicroKernel; import org.apache.jackrabbit.oak.plugins.document.util.MongoConnection; +import org.apache.jackrabbit.oak.stats.Clock; import org.junit.After; import org.junit.Assume; import org.junit.Before; @@ -42,9 +43,13 @@ public void setUpConnection() throws Exception { mongoConnection = MongoUtils.getConnection(); MongoUtils.dropCollections(mongoConnection.getDB()); - mk = new DocumentMK.Builder().setMongoDB(mongoConnection.getDB()).open(); + mk = new DocumentMK.Builder().clock(getTestClock()).setMongoDB(mongoConnection.getDB()).open(); } + protected Clock getTestClock() throws InterruptedException { + return Clock.SIMPLE; + } + @After public void tearDownConnection() throws Exception { mk.dispose(); @@ -59,5 +64,6 @@ protected MicroKernel getMicroKernel() { return mk; } + } Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/blob/ds/MongoDataStoreBlobGCTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/blob/ds/MongoDataStoreBlobGCTest.java (revision 1583235) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/blob/ds/MongoDataStoreBlobGCTest.java (working copy) @@ -30,7 +30,7 @@ /** * Test for MongoMK GC with {@link DataStoreBlobStore} - * + * */ public class MongoDataStoreBlobGCTest extends MongoBlobGCTest { @BeforeClass @@ -47,7 +47,7 @@ public void setUpConnection() throws Exception { mongoConnection = MongoUtils.getConnection(); MongoUtils.dropCollections(mongoConnection.getDB()); - mk = new DocumentMK.Builder().setMongoDB(mongoConnection.getDB()) + mk = new DocumentMK.Builder().clock(getTestClock()).setMongoDB(mongoConnection.getDB()) .setBlobStore(DataStoreUtils.getBlobStore()).open(); } Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/MongoBlobGCTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/MongoBlobGCTest.java (revision 1583235) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/MongoBlobGCTest.java (working copy) @@ -19,6 +19,7 @@ import static org.junit.Assert.assertTrue; import java.io.ByteArrayInputStream; +import java.io.IOException; import java.io.InputStream; import java.util.HashSet; import java.util.Iterator; @@ -25,7 +26,10 @@ import java.util.List; import java.util.Random; import java.util.Set; +import java.util.concurrent.TimeUnit; +import junit.framework.Assert; + import com.google.common.collect.Lists; import com.google.common.collect.Sets; import com.mongodb.BasicDBObject; @@ -33,10 +37,12 @@ import org.apache.jackrabbit.oak.api.Blob; import org.apache.jackrabbit.oak.plugins.blob.MarkSweepGarbageCollector; +import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.VersionGCStats; import org.apache.jackrabbit.oak.spi.blob.GarbageCollectableBlobStore; import org.apache.jackrabbit.oak.spi.commit.CommitInfo; import org.apache.jackrabbit.oak.spi.commit.EmptyHook; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; +import org.apache.jackrabbit.oak.stats.Clock; import org.junit.Test; /** @@ -43,8 +49,9 @@ * Tests for MongoMK GC */ public class MongoBlobGCTest extends AbstractMongoConnectionTest { + private Clock clock; - public HashSet setUp() throws Exception { + public HashSet setUp(boolean deleteDirect) throws Exception { HashSet set = new HashSet(); DocumentNodeStore s = mk.getNodeStore(); @@ -74,14 +81,29 @@ } s.merge(a, EmptyHook.INSTANCE, CommitInfo.EMPTY); - for (int id : processed) { - delete("c" + id); + if (deleteDirect) { + for (int id : processed) { + deleteFromMongo("c" + id); + } + } else { + a = s.getRoot().builder(); + for (int id : processed) { + a.child("c" + id).remove(); + s.merge(a, EmptyHook.INSTANCE, CommitInfo.EMPTY); + } + long maxAge = 10; // hours + // 1. Go past GC age and check no GC done as nothing deleted + clock.waitUntil(clock.getTime() + TimeUnit.MINUTES.toMillis(maxAge)); + + VersionGarbageCollector vGC = s.getVersionGarbageCollector(); + VersionGCStats stats = vGC.gc(0, TimeUnit.MILLISECONDS); + Assert.assertEquals(processed.size(), stats.deletedDocGCCount); } return set; } - private void delete(String nodeId) { + private void deleteFromMongo(String nodeId) { DBCollection coll = mongoConnection.getDB().getCollection("nodes"); BasicDBObject blobNodeObj = new BasicDBObject(); blobNodeObj.put("_id", "1:/" + nodeId); @@ -89,12 +111,21 @@ } @Test - public void gc() throws Exception { - HashSet set = setUp(); + public void gcDirectMongoDelete() throws Exception { + HashSet set = setUp(true); + gc(set); + } + @Test + public void gcVersionDelete() throws Exception { + HashSet set = setUp(false); + gc(set); + } + + private void gc(HashSet set) throws IOException, Exception { DocumentNodeStore store = mk.getNodeStore(); - MarkSweepGarbageCollector gc = new MarkSweepGarbageCollector(); - gc.init(new DocumentBlobReferenceRetriever(store), + MarkSweepGarbageCollector gc = new MarkSweepGarbageCollector( + new DocumentBlobReferenceRetriever(store), (GarbageCollectableBlobStore) store.getBlobStore(), "./target", 2048, true, 2, 0); gc.collectGarbage(); @@ -121,4 +152,11 @@ r.nextBytes(data); return new ByteArrayInputStream(data); } + + @Override + protected Clock getTestClock() throws InterruptedException { + clock = new Clock.Virtual(); + clock.waitUntil(Revision.getCurrentTimestamp()); + return clock; + } } Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/SegmentDataStoreBlobGCTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/SegmentDataStoreBlobGCTest.java (revision 1583235) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/SegmentDataStoreBlobGCTest.java (working copy) @@ -121,9 +121,9 @@ public void gc() throws Exception { HashSet set = setUp(); - MarkSweepGarbageCollector gc = new MarkSweepGarbageCollector(); - gc.init(new SegmentBlobReferenceRetriever(store.getTracker()), - (GarbageCollectableBlobStore) store.getBlobStore(), "./target", 2048, true, 2, 0); + MarkSweepGarbageCollector gc = new MarkSweepGarbageCollector( + new SegmentBlobReferenceRetriever(store.getTracker()), + (GarbageCollectableBlobStore) store.getBlobStore(), "./target", 2048, true, 2, 0); gc.collectGarbage(); Set existing = iterate();