OAK-10921 : fixed race condition where fullGC database variables gets…

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<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> propValMap) {
+        final UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, true);
+        setUpdateOp(propValMap, updateOp);
         vgc.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp);
     }
 
-    private void setLongSetting(final Map<String, Long> propValMap) {
-        UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, true);
-        propValMap.forEach(updateOp::set);
-        vgc.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp);
+    private void setUpdateOp(final Map<String, Object> 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<String, Object> 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

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));
 

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;
@@ -1477,6 +1476,90 @@ private Iterator<NodeDocument> 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<VersionGarbageCollector> gcRef = Atomics.newReference();
+        final VersionGCSupport gcSupport = new VersionGCSupport(store1.getDocumentStore()) {
+
+            @Override
+            public Iterable<NodeDocument> getModifiedDocs(long fromModified, long toModified, int limit, @NotNull String fromId,
+                                                          final @NotNull Set<String> includePaths, final @NotNull Set<String> 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