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..779bcead2e8 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,16 @@ 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); + + final Map updateVGCMap = new HashMap<>(); + updateVGCMap.put(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp); + updateVGCMap.put(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP, stats.oldestModifiedDocId); + updateVGCSetting(updateVGCMap); int count = stats.deletedDocGCCount - stats.deletedLeafDocGCCount; double usedFraction; @@ -289,7 +295,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 +310,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 +351,51 @@ private Map getVGCSettings() { return settings; } - private void setLongSetting(String propName, long val) { - setLongSetting(of(propName, val)); + /** + * Set the VGC settings with the given property and value. + * If property is not present, it will add the property to versionGC document with given value. + * + * @param propName the property name + * @param val the value + * @see VersionGCRecommendations#setVGCSetting(Map) + */ + private void setVGCSetting(final String propName, final Object val) { + final Map vgcMap = new HashMap<>(); + vgcMap.put(propName, val); + setVGCSetting(vgcMap); } - private void setStringSetting(String propName, String val) { - UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, true); - updateOp.set(propName, val); + /** + * Set the VGC settings with the given properties and values. + * If properties are not present, it will add the properties to versionGC document with given values + * . + * @param propValMap the properties and values to set + */ + 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); + }); + } + + /** + * Update the VGC settings with the given properties and values. + * Properties are only updated if they already exists in the versionGC document. + * + * @param propValMap the properties and values to update + */ + 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..47afbf2b7e5 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 @@ -62,7 +62,9 @@ public void lazyInitialize() throws Exception { vgc = store.find(SETTINGS, SETTINGS_COLLECTION_ID); assertNotNull(vgc); - assertEquals(0L, vgc.get(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP)); + + // fullGC values shouldn't have been updated without fullGC enabled + assertNull(vgc.get(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP)); assertNull(vgc.get(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP)); } @@ -124,9 +126,6 @@ public void lazyInitializeWithFullGCDryRun() throws Exception { vgc = store.find(SETTINGS, SETTINGS_COLLECTION_ID); assertNotNull(vgc); // fullGC values shouldn't have been updated in dryRun mode - System.out.println(stats.oldestModifiedDocId); - System.out.println(stats.oldestModifiedDocTimeStamp); - System.out.println(vgc); assertNull(vgc.get(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP)); assertNull(vgc.get(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP)); 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 84f84d439cb..fcc0c869f02 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 @@ -95,7 +95,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.cache.Cache; @@ -1611,6 +1610,90 @@ private Iterator candidates(long fromModified, long toModified, in // OAK-10199 END + // OAK-10921 + @Test + public void resetGCFromOakRunWhileRunning() throws Exception { + // if we reset fullGC from any external source while GC is running, + // it should not update the fullGC variables. + resetFullGCExternally(false); + } + + @Test + public void resetFullGCFromOakRunWhileRunning() throws Exception { + // if we reset fullGC from any external source while GC is running, + // it should not update the fullGC variables. + resetFullGCExternally(true); + } + + private void resetFullGCExternally(final boolean resetFullGCOnly) throws Exception { + //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 (resetFullGCOnly) { + 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