diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java index 0cf919fba66..370f3b1485a 100644 --- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java +++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java @@ -179,6 +179,9 @@ public class VersionGCRecommendations { () -> oldestModifiedDocTimeStamp.set(0L)); oldestModifiedDocId = MIN_ID_VALUE; log.info("fullGCTimestamp found: {}", timestampToString(oldestModifiedDocTimeStamp.get())); + // initialize the fullGC database variables i.e. fullGCTimestamp and fullGCId + setVGCSetting(of(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP, oldestModifiedDocTimeStamp.get(), + SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP, oldestModifiedDocId)); } else { oldestModifiedDocTimeStamp.set(fullGCTimestamp); } @@ -266,13 +269,15 @@ public void evaluate(VersionGCStats stats) { long nextDuration = Math.max(precisionMs, scope.getDurationMs() / 2); gcmon.info("Limit {} documents exceeded, reducing next collection interval to {} seconds", this.maxCollect, TimeUnit.MILLISECONDS.toSeconds(nextDuration)); - setLongSetting(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP, nextDuration); + setVGCSetting(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP, nextDuration); stats.needRepeat = true; } else if (!stats.canceled && !stats.ignoredGCDueToCheckPoint && !isFullGCDryRun) { // success, we would not expect to encounter revisions older than this in the future - setLongSetting(of(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, scope.toMs, - SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp)); - setStringSetting(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP, stats.oldestModifiedDocId); + setVGCSetting(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, scope.toMs); + updateVGCSetting(new HashMap<>() {{ + put(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp); + put(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP, stats.oldestModifiedDocId); + }}); int count = stats.deletedDocGCCount - stats.deletedLeafDocGCCount; double usedFraction; @@ -289,7 +294,7 @@ public void evaluate(VersionGCStats stats) { long nextDuration = (long) Math.ceil(suggestedIntervalMs * 1.5); log.debug("successful run using {}% of limit, raising recommended interval to {} seconds", Math.round(usedFraction * 1000) / 10.0, TimeUnit.MILLISECONDS.toSeconds(nextDuration)); - setLongSetting(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP, nextDuration); + setVGCSetting(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP, nextDuration); } else { log.debug("not increasing limit: collected {} documents ({}% >= {}% limit)", count, usedFraction, allowedFraction); @@ -304,11 +309,11 @@ public void evaluate(VersionGCStats stats) { if (fullGCEnabled && !stats.canceled && !stats.ignoredFullGCDueToCheckPoint) { // success, we would not expect to encounter revisions older than this in the future if (isFullGCDryRun) { - setLongSetting(SETTINGS_COLLECTION_FULL_GC_DRY_RUN_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp); - setStringSetting(SETTINGS_COLLECTION_FULL_GC_DRY_RUN_DOCUMENT_ID_PROP, stats.oldestModifiedDocId); + setVGCSetting(of(SETTINGS_COLLECTION_FULL_GC_DRY_RUN_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp, + SETTINGS_COLLECTION_FULL_GC_DRY_RUN_DOCUMENT_ID_PROP, stats.oldestModifiedDocId)); } else { - setLongSetting(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp); - setStringSetting(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP, stats.oldestModifiedDocId); + updateVGCSetting(of(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp, + SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP, stats.oldestModifiedDocId)); } final long scopeEnd = scopeFullGC.toMs; @@ -345,20 +350,31 @@ private Map getVGCSettings() { return settings; } - private void setLongSetting(String propName, long val) { - setLongSetting(of(propName, val)); + private void setVGCSetting(final String propName, final Object val) { + setVGCSetting(new HashMap<>() {{ + put(propName, val); + }}); } - private void setStringSetting(String propName, String val) { - UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, true); - updateOp.set(propName, val); + private void setVGCSetting(final Map propValMap) { + final UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, true); + setUpdateOp(propValMap, updateOp); vgc.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp); } - private void setLongSetting(final Map propValMap) { - UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, true); - propValMap.forEach(updateOp::set); - vgc.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp); + private void setUpdateOp(final Map propValMap, final UpdateOp updateOp) { + propValMap.forEach((k, v) -> { + if (v instanceof Number) updateOp.set(k, ((Number) v).longValue()); + if (v instanceof String) updateOp.set(k, (String) v); + if (v instanceof Boolean) updateOp.set(k, (Boolean) v); + }); + } + + private void updateVGCSetting(final Map propValMap) { + final UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, false); + setUpdateOp(propValMap, updateOp); + propValMap.forEach((k, v) -> updateOp.contains(k, true)); + vgc.getDocumentStore().findAndUpdate(Collection.SETTINGS, updateOp); } @NotNull diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCInitTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCInitTest.java index 2e21b927ca6..14833269e97 100644 --- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCInitTest.java +++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCInitTest.java @@ -63,7 +63,7 @@ public void lazyInitialize() throws Exception { vgc = store.find(SETTINGS, SETTINGS_COLLECTION_ID); assertNotNull(vgc); assertEquals(0L, vgc.get(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP)); - assertNull(vgc.get(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP)); + assertEquals(MIN_ID_VALUE, vgc.get(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP)); } @Test diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java index 857813a503a..ecc7ac0fd8d 100644 --- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java +++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java @@ -189,7 +189,9 @@ public void cancelMustNotUpdateLastOldestTimeStamp() throws Exception { // ensure a canceled GC doesn't update that versionGC SETTINGS entry Document statusAfter = store.find(Collection.SETTINGS, "versionGC"); if (statusBefore == null) { - assertNull(statusAfter); + // fullGC values had been added, so it won't be null + assertNotNull(statusAfter); + assertNull(statusAfter.get(lastOldestTimeStampProp)); } else { assertNotNull(statusAfter); assertEquals( diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java index 58c1f0ef160..ac994ea4301 100644 --- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java +++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java @@ -94,7 +94,6 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import static org.junit.Assume.assumeThat; import static org.junit.Assume.assumeTrue; import org.apache.jackrabbit.guava.common.base.Function; @@ -1478,6 +1477,89 @@ private Iterator candidates(long fromModified, long toModified, in // OAK-10199 END + // OAK-10921 + @Test + public void resetGCFromOakRunWhileRunning() throws Exception { + doResetGCFromOakRunWhileRunning(false); + } + + @Test + public void resetFullGCFromOakRunWhileRunning() throws Exception { + doResetGCFromOakRunWhileRunning(true); + } + + private void doResetGCFromOakRunWhileRunning(final boolean resetFullGC) throws Exception { + // if we reset fullGC from any external source while GC is running, + // it should not update the fullGC variables. + + //1. Create nodes with properties + NodeBuilder b1 = store1.getRoot().builder(); + + // Add property to node & save + for (int i = 0; i < 5; i++) { + b1.child("z"+i).setProperty("prop", "foo", STRING); + } + store1.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY); + store1.runBackgroundOperations(); + + // enable the full gc flag + writeField(gc, "fullGCEnabled", true, true); + long maxAge = 1; //hours + long delta = MINUTES.toMillis(10); + //1. Go past GC age and check no GC done as nothing deleted + clock.waitUntil(getCurrentTimestamp() + maxAge); + VersionGCStats stats = gc(gc, maxAge, HOURS); + assertStatsCountsZero(stats); + + //Remove property + NodeBuilder b2 = store1.getRoot().builder(); + for (int i = 0; i < 5; i++) { + b2.getChildNode("z"+i).removeProperty("prop"); + } + store1.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY); + store1.runBackgroundOperations(); + + final AtomicReference gcRef = Atomics.newReference(); + final VersionGCSupport gcSupport = new VersionGCSupport(store1.getDocumentStore()) { + + @Override + public Iterable getModifiedDocs(long fromModified, long toModified, int limit, @NotNull String fromId, + final @NotNull Set includePaths, final @NotNull Set excludePaths) { + // reset fullGC variables externally while GC is running + if (resetFullGC) { + gcRef.get().resetFullGC(); + } else { + gcRef.get().reset(); + } + return super.getModifiedDocs(fromModified, toModified, limit, fromId, includePaths, excludePaths); + } + }; + + gcRef.set(new VersionGarbageCollector(store1, gcSupport, true, false, false, 3)); + + //3. Check that deleted property does get collected post maxAge + clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta); + + Document document = store1.getDocumentStore().find(SETTINGS, SETTINGS_COLLECTION_ID); + assert document != null; + assertNotNull(document.get(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP)); + assertNotNull(document.get(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP)); + + stats = gcRef.get().gc(maxAge*2, HOURS); + + document = store1.getDocumentStore().find(SETTINGS, SETTINGS_COLLECTION_ID); + assertEquals(5, stats.updatedFullGCDocsCount); + assertEquals(5, stats.deletedPropsCount); + assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); + + // 4. verify that fullGC variables are not updated after resetting them + assert document != null; + assertNull(document.get(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP)); + assertNull(document.get(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP)); + } + + // OAK-10921 END + @SuppressWarnings("unchecked") /** * Test when a revision on a parent is becoming garbage on a property