Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java (revision 1783200) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java (working copy) @@ -38,11 +38,12 @@ this.store = store; } - public Iterable getPossiblyDeletedDocs(final long lastModifiedTime) { + public Iterable getPossiblyDeletedDocs(final long prevLastModifiedTime, final long lastModifiedTime) { return filter(getSelectedDocuments(store, NodeDocument.DELETED_ONCE, 1), new Predicate() { @Override public boolean apply(NodeDocument input) { - return input.wasDeletedOnce() && !input.hasBeenModifiedSince(lastModifiedTime); + return input.wasDeletedOnce() && input.hasBeenModifiedSince(prevLastModifiedTime) + && !input.hasBeenModifiedSince(lastModifiedTime); } }); } Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java (revision 1783200) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java (working copy) @@ -80,11 +80,28 @@ private static final Set GC_TYPES = EnumSet.of( DEFAULT_LEAF, COMMIT_ROOT_ONLY); + /** + * Document id stored in settings collection that keeps info about version gc + */ + private static final String SETTINGS_COLLECTION_ID = "versionGC"; + + /** + * Property name to timestamp when last gc run happened + */ + private static final String SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP = "lastOldestTimeStamp"; + + /** + * Margin for lower bound to be used while collecting documents to be GC'ed + */ + private static final long OLDEST_TIMESTAMP_MARGIN = TimeUnit.MINUTES.toMillis(1); + VersionGarbageCollector(DocumentNodeStore nodeStore, VersionGCSupport gcSupport) { this.nodeStore = nodeStore; this.versionStore = gcSupport; this.ds = nodeStore.getDocumentStore(); + + createSettingDocIfNotExist(); } public VersionGCStats gc(long maxRevisionAge, TimeUnit unit) throws IOException { @@ -249,9 +266,11 @@ final long oldestRevTimeStamp = nodeStore.getClock().getTime() - maxRevisionAgeInMillis; final RevisionVector headRevision = nodeStore.getHeadRevision(); - log.info("Starting revision garbage collection. Revisions older than [{}] will be " + - "removed", Utils.timestampToString(oldestRevTimeStamp)); + final long lastOldestTimeStamp = getLastOldestTimeStamp(); + log.info("Starting revision garbage collection. Revisions older than [{}] and newer than [{}] will be removed", + Utils.timestampToString(oldestRevTimeStamp), Utils.timestampToString(lastOldestTimeStamp)); + //Check for any registered checkpoint which prevent the GC from running Revision checkpoint = nodeStore.getCheckpoints().getOldestRevisionToKeep(); if (checkpoint != null && checkpoint.getTimestamp() < oldestRevTimeStamp) { @@ -264,9 +283,11 @@ return phases.stats; } - collectDeletedDocuments(phases, headRevision, oldestRevTimeStamp); + collectDeletedDocuments(phases, headRevision, lastOldestTimeStamp, oldestRevTimeStamp); collectSplitDocuments(phases, oldestRevTimeStamp); + setLastOldestTimeStamp(oldestRevTimeStamp); + phases.close(); phases.stats.canceled = cancel.get(); log.info("Revision garbage collection finished in {}. {}", phases.elapsed, phases.stats); @@ -282,13 +303,14 @@ private void collectDeletedDocuments(GCPhases phases, RevisionVector headRevision, - long oldestRevTimeStamp) + final long lastOldestTimeStamp, + final long oldestRevTimeStamp) throws IOException { int docsTraversed = 0; DeletedDocsGC gc = new DeletedDocsGC(headRevision, cancel); try { if (phases.start(GCPhase.COLLECTING)) { - Iterable itr = versionStore.getPossiblyDeletedDocs(oldestRevTimeStamp); + Iterable itr = versionStore.getPossiblyDeletedDocs(lastOldestTimeStamp, oldestRevTimeStamp); try { for (NodeDocument doc : itr) { // continue with GC? @@ -342,6 +364,25 @@ } } + private long getLastOldestTimeStamp() { + Document versionGCDoc = ds.find(Collection.SETTINGS, SETTINGS_COLLECTION_ID, 0); + return (Long) versionGCDoc.get(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP) - OLDEST_TIMESTAMP_MARGIN; + } + + private void setLastOldestTimeStamp(long lastGCRunTime) { + UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, false); + updateOp.set(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, lastGCRunTime); + ds.createOrUpdate(Collection.SETTINGS, updateOp); + } + + private void createSettingDocIfNotExist() { + if (ds.find(Collection.SETTINGS, SETTINGS_COLLECTION_ID) == null) { + UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, true); + updateOp.set(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, OLDEST_TIMESTAMP_MARGIN); + ds.createOrUpdate(Collection.SETTINGS, updateOp); + } + } + /** * A helper class to remove document for deleted nodes. */ Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java (revision 1783200) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java (working copy) @@ -75,12 +75,11 @@ } @Override - public CloseableIterable getPossiblyDeletedDocs(final long lastModifiedTime) { + public CloseableIterable getPossiblyDeletedDocs(final long prevLastModifiedTime, final long lastModifiedTime) { //_deletedOnce == true && _modified < lastModifiedTime - DBObject query = - start(NodeDocument.DELETED_ONCE).is(Boolean.TRUE) - .put(NodeDocument.MODIFIED_IN_SECS).lessThan(NodeDocument.getModifiedInSecs(lastModifiedTime)) - .get(); + DBObject query = start(NodeDocument.DELETED_ONCE).is(Boolean.TRUE).put(NodeDocument.MODIFIED_IN_SECS) + .greaterThan(NodeDocument.getModifiedInSecs(prevLastModifiedTime)) + .lessThan(NodeDocument.getModifiedInSecs(lastModifiedTime)).get(); DBCursor cursor = getNodeCollection().find(query).setReadPreference(ReadPreference.secondaryPreferred()); cursor.batchSize(batchSize); if (!disableIndexHint) { Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBVersionGCSupport.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBVersionGCSupport.java (revision 1783200) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBVersionGCSupport.java (working copy) @@ -47,10 +47,11 @@ } @Override - public Iterable getPossiblyDeletedDocs(final long lastModifiedTime) { + public Iterable getPossiblyDeletedDocs(final long prevLastModifiedTime, final long lastModifiedTime) { List conditions = new ArrayList(); conditions.add(new QueryCondition(NodeDocument.DELETED_ONCE, "=", 1)); conditions.add(new QueryCondition(NodeDocument.MODIFIED_IN_SECS, "<", NodeDocument.getModifiedInSecs(lastModifiedTime))); + conditions.add(new QueryCondition(NodeDocument.MODIFIED_IN_SECS, ">", NodeDocument.getModifiedInSecs(prevLastModifiedTime))); return getIterator(RDBDocumentStore.EMPTY_KEY_PATTERN, conditions); } Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java (revision 1783200) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java (working copy) @@ -30,6 +30,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import static com.google.common.collect.Iterables.filter; @@ -86,6 +87,8 @@ private Clock clock; + private DocumentMK.Builder documentMKBuilder; + private DocumentNodeStore store; private VersionGarbageCollector gc; @@ -121,12 +124,9 @@ clock = new Clock.Virtual(); clock.waitUntil(System.currentTimeMillis()); Revision.setClock(clock); - store = new DocumentMK.Builder() - .clock(clock) - .setLeaseCheck(false) - .setDocumentStore(fixture.createDocumentStore()) - .setAsyncDelay(0) - .getNodeStore(); + documentMKBuilder = new DocumentMK.Builder().clock(clock).setLeaseCheck(false) + .setDocumentStore(fixture.createDocumentStore()).setAsyncDelay(0); + store = documentMKBuilder.getNodeStore(); gc = store.getVersionGarbageCollector(); } @@ -453,8 +453,8 @@ final BlockingQueue docs = Queues.newSynchronousQueue(); VersionGCSupport gcSupport = new VersionGCSupport(store.getDocumentStore()) { @Override - public Iterable getPossiblyDeletedDocs(long lastModifiedTime) { - return filter(super.getPossiblyDeletedDocs(lastModifiedTime), + public Iterable getPossiblyDeletedDocs(long prevLastModifiedTime, long lastModifiedTime) { + return filter(super.getPossiblyDeletedDocs(prevLastModifiedTime, lastModifiedTime), new Predicate() { @Override public boolean apply(NodeDocument input) { @@ -622,10 +622,10 @@ final AtomicReference gcRef = Atomics.newReference(); VersionGCSupport gcSupport = new VersionGCSupport(store.getDocumentStore()) { @Override - public Iterable getPossiblyDeletedDocs(long lastModifiedTime) { + public Iterable getPossiblyDeletedDocs(long prevLastModifiedTime, long lastModifiedTime) { // cancel as soon as it runs gcRef.get().cancel(); - return super.getPossiblyDeletedDocs(lastModifiedTime); + return super.getPossiblyDeletedDocs(prevLastModifiedTime, lastModifiedTime); } }; gcRef.set(new VersionGarbageCollector(store, gcSupport)); @@ -655,12 +655,12 @@ final AtomicReference gcRef = Atomics.newReference(); VersionGCSupport gcSupport = new VersionGCSupport(store.getDocumentStore()) { @Override - public Iterable getPossiblyDeletedDocs(final long lastModifiedTime) { + public Iterable getPossiblyDeletedDocs(final long prevLastModifiedTime, final long lastModifiedTime) { return new Iterable() { @Override public Iterator iterator() { return new AbstractIterator() { - private Iterator it = candidates(lastModifiedTime); + private Iterator it = candidates(prevLastModifiedTime, lastModifiedTime); @Override protected NodeDocument computeNext() { if (it.hasNext()) { @@ -675,8 +675,8 @@ }; } - private Iterator candidates(long lastModifiedTime) { - return super.getPossiblyDeletedDocs(lastModifiedTime).iterator(); + private Iterator candidates(long prevLastModifiedTime, long lastModifiedTime) { + return super.getPossiblyDeletedDocs(prevLastModifiedTime, lastModifiedTime).iterator(); } }; gcRef.set(new VersionGarbageCollector(store, gcSupport)); @@ -688,6 +688,59 @@ assertEquals(0, stats.splitDocGCCount); } + // OAK-3070 + @Test + public void lowerBoundOfModifiedDocs() throws Exception { + Revision.setClock(clock); + final VersionGCSupport fixtureGCSupport = documentMKBuilder.createVersionGCSupport(); + final AtomicInteger docCounter = new AtomicInteger(); + VersionGCSupport nonReportingGcSupport = new VersionGCSupport(store.getDocumentStore()) { + @Override + public Iterable getPossiblyDeletedDocs(final long prevLastModifiedTime, long lastModifiedTime) { + return filter(fixtureGCSupport.getPossiblyDeletedDocs(prevLastModifiedTime, lastModifiedTime), + new Predicate() { + @Override + public boolean apply(NodeDocument input) { + docCounter.incrementAndGet(); + return false;// don't report any doc to be + // GC'able + } + }); + } + }; + final VersionGarbageCollector gc = new VersionGarbageCollector(store, nonReportingGcSupport); + final long maxAgeHours = 1; + final long clockDelta = HOURS.toMillis(maxAgeHours) + MINUTES.toMillis(5); + + // create and delete node + NodeBuilder builder = store.getRoot().builder(); + builder.child("foo1"); + merge(store, builder); + builder = store.getRoot().builder(); + builder.getChildNode("foo1").remove(); + merge(store, builder); + store.runBackgroundOperations(); + + clock.waitUntil(clock.getTime() + clockDelta); + gc.gc(maxAgeHours, HOURS); + assertEquals("Not all deletable docs got reported on first run", 1, docCounter.get()); + + docCounter.set(0); + // create and delete another node + builder = store.getRoot().builder(); + builder.child("foo2"); + merge(store, builder); + builder = store.getRoot().builder(); + builder.getChildNode("foo2").remove(); + merge(store, builder); + store.runBackgroundOperations(); + + // wait another hour and GC in last 1 hour + clock.waitUntil(clock.getTime() + clockDelta); + gc.gc(maxAgeHours, HOURS); + assertEquals(1, docCounter.get()); + } + private void createTestNode(String name) throws CommitFailedException { DocumentStore ds = store.getDocumentStore(); NodeBuilder builder = store.getRoot().builder();