This is an automated email from the ASF dual-hosted git repository.

daim pushed a commit to branch OAK-10199
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git

commit 69944e749431a4dc5846e0aaedb046268bb47a61
Author: Rishabh Kumar <d...@adobe.com>
AuthorDate: Tue Jun 27 20:53:53 2023 +0530

    OAK-10199 : added unit cases to handle concurrent prop update and escaped 
properties update
---
 .../plugins/document/VersionGCRecommendations.java |   4 +-
 .../plugins/document/VersionGarbageCollector.java  |  31 ++-
 .../oak/plugins/document/VersionGCInitTest.java    |   4 +-
 .../document/VersionGarbageCollectorIT.java        | 217 +++++++++++++++++----
 4 files changed, 202 insertions(+), 54 deletions(-)

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 ac0bcc03e3..c80399f005 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
@@ -26,12 +26,12 @@ import 
org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.Versio
 import org.apache.jackrabbit.oak.plugins.document.util.TimeInterval;
 import org.apache.jackrabbit.oak.spi.gc.GCMonitor;
 import org.apache.jackrabbit.oak.stats.Clock;
-import org.jetbrains.annotations.NotNull;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import static java.lang.Long.MAX_VALUE;
 import static java.util.Map.of;
+import static java.util.concurrent.TimeUnit.SECONDS;
 import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.MIN_ID_VALUE;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.NULL;
 import static 
org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_PROP;
@@ -124,7 +124,7 @@ public class VersionGCRecommendations {
             if (doc == NULL) {
                 oldestModifiedDocTimeStamp = 0L;
             } else {
-                oldestModifiedDocTimeStamp = doc.getModified() == null ? 0L : 
doc.getModified() - 1;
+                oldestModifiedDocTimeStamp = doc.getModified() == null ? 0L : 
SECONDS.toMillis(doc.getModified()) - 1L;
             }
             oldestModifiedDocId = MIN_ID_VALUE;
             log.info("detailedGCTimestamp found: {}", 
timestampToString(oldestModifiedDocTimeStamp));
diff --git 
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
 
b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
index 307315d5a8..eb25184f62 100644
--- 
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
+++ 
b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
@@ -24,6 +24,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.EnumSet;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -61,6 +62,7 @@ import static java.util.Collections.emptySet;
 import static java.util.Objects.requireNonNull;
 import static java.util.Optional.ofNullable;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
 import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toSet;
 import static 
org.apache.jackrabbit.guava.common.base.StandardSystemProperty.LINE_SEPARATOR;
@@ -629,7 +631,6 @@ public class VersionGarbageCollector {
                         // set foundDoc to false to allow exiting the while 
loop
                         foundDoc = false;
                         Iterable<NodeDocument> itr = 
versionStore.getModifiedDocs(fromModified, toModified, 1000, fromId);
-                        // set includeFromId to false for subsequent queries
                         try {
                             for (NodeDocument doc : itr) {
                                 foundDoc = true;
@@ -656,7 +657,7 @@ public class VersionGarbageCollector {
                                 if (modified == null) {
                                     monitor.warn("collectDetailGarbage : 
document has no _modified property : {}",
                                             doc.getId());
-                                } else if (modified < 
oldestModifiedDocTimeStamp) {
+                                } else if (SECONDS.toMillis(modified) < 
oldestModifiedDocTimeStamp) {
                                     monitor.warn(
                                             "collectDetailGarbage : document 
has older _modified than query boundary : {} (from: {}, to: {})",
                                             modified, fromModified, 
toModified);
@@ -668,13 +669,13 @@ public class VersionGarbageCollector {
                                 phases.stop(GCPhase.DETAILED_GC_CLEANUP);
                             }
                             if (lastDoc != null) {
-                                fromModified = 
ofNullable(lastDoc.getModified()).orElse(oldestModifiedDocTimeStamp);
+                                fromModified = lastDoc.getModified() == null ? 
oldestModifiedDocTimeStamp : SECONDS.toMillis(lastDoc.getModified());
                                 fromId = lastDoc.getId();
                             }
                         } finally {
                             Utils.closeIfCloseable(itr);
                             phases.stats.oldestModifiedDocTimeStamp = 
fromModified;
-                            if (fromModified > oldestModifiedDocTimeStamp) {
+                            if (fromModified > (oldestModifiedDocTimeStamp + 
1)) {
                                 // we have moved ahead, now we can reset 
oldestModifiedId to min value
                                 phases.stats.oldestModifiedDocId = 
MIN_ID_VALUE;
                             } else {
@@ -683,6 +684,13 @@ public class VersionGarbageCollector {
                                 phases.stats.oldestModifiedDocId = fromId;
                             }
                         }
+
+                        // if we are already at last document of current 
timeStamp,
+                        // we need to reset fromId and check again
+                        if (!foundDoc && !Objects.equals(fromId, 
MIN_ID_VALUE)) {
+                            fromId = MIN_ID_VALUE;
+                            foundDoc = true; // to run while loop again
+                        }
                     }
                     phases.stop(GCPhase.DETAILED_GC);
                 }
@@ -785,6 +793,8 @@ public class VersionGarbageCollector {
         private final AtomicBoolean cancel;
         private final Stopwatch timer;
         private final List<UpdateOp> updateOpList;
+
+        private final Map<String, Integer> deletedPropsCountMap;
         private int garbageDocsCount;
 
         public DetailedGC(@NotNull RevisionVector headRevision, @NotNull 
GCMonitor monitor, @NotNull AtomicBoolean cancel) {
@@ -792,6 +802,7 @@ public class VersionGarbageCollector {
             this.monitor = monitor;
             this.cancel = cancel;
             this.updateOpList = new ArrayList<>();
+            this.deletedPropsCountMap = new HashMap<>();
             this.timer = Stopwatch.createUnstarted();
         }
 
@@ -800,6 +811,8 @@ public class VersionGarbageCollector {
             monitor.info("Collecting Detailed Garbage for doc [{}]", 
doc.getId());
 
             final UpdateOp op = new UpdateOp(requireNonNull(doc.getId()), 
false);
+            op.equals(MODIFIED_IN_SECS, doc.getModified());
+
             collectDeletedProperties(doc, phases, op);
             collectUnmergedBranchCommitDocument(doc, phases, op);
             collectOldRevisions(doc, phases, op);
@@ -837,6 +850,7 @@ public class VersionGarbageCollector {
                         .map(DocumentNodeState::getPropertyNames)
                         .map(p -> 
p.stream().map(Utils::escapePropertyName).collect(toSet()))
                         .orElse(emptySet());
+
                 final int deletedPropsGCCount = properties.stream()
                         .filter(p -> !retainPropSet.contains(p))
                         .mapToInt(x -> {
@@ -844,8 +858,8 @@ public class VersionGarbageCollector {
                             return 1;})
                         .sum();
 
+                deletedPropsCountMap.put(doc.getId(), deletedPropsGCCount);
 
-                phases.stats.deletedPropsGCCount += deletedPropsGCCount;
                 if (log.isDebugEnabled()) {
                     log.debug("Collected {} deleted properties for document 
{}", deletedPropsGCCount, doc.getId());
                 }
@@ -896,9 +910,12 @@ public class VersionGarbageCollector {
 
             timer.reset().start();
             try {
-                updatedDocs = (int) ds.findAndUpdate(NODES, 
updateOpList).stream().filter(Objects::nonNull).count();
+                List<NodeDocument> oldDocs = ds.findAndUpdate(NODES, 
updateOpList);
+                int deletedProps = 
oldDocs.stream().filter(Objects::nonNull).mapToInt(d -> 
deletedPropsCountMap.getOrDefault(d.getId(), 0)).sum();
+                updatedDocs = (int) 
oldDocs.stream().filter(Objects::nonNull).count();
                 stats.updatedDetailedGCDocsCount += updatedDocs;
-                log.info("Updated [{}] documents", updatedDocs);
+                stats.deletedPropsGCCount += deletedProps;
+                log.info("Updated [{}] documents, deleted [{}] properties", 
updatedDocs, deletedProps);
                 // now reset delete metadata
                 updateOpList.clear();
                 garbageDocsCount = 0;
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 4db64c942f..738c1109ad 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
@@ -78,8 +78,8 @@ public class VersionGCInitTest {
 
         vgc = store.find(SETTINGS, "versionGC");
         assertNotNull(vgc);
-        assertEquals(40L, 
vgc.get(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP));
-        assertEquals(MIN_ID_VALUE, 
vgc.get(SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_PROP));
+        assertEquals(40_000L, 
vgc.get(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP));
+        assertEquals("1:/node", 
vgc.get(SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_PROP));
     }
 
     @Test
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 ca00648244..6ba8dd6f81 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
@@ -34,6 +34,9 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
+import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.StreamSupport.stream;
 import static org.apache.commons.lang3.reflect.FieldUtils.writeField;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.filter;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.size;
@@ -41,9 +44,12 @@ import static java.util.concurrent.TimeUnit.HOURS;
 import static java.util.concurrent.TimeUnit.MINUTES;
 import static org.apache.jackrabbit.oak.api.Type.STRING;
 import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.MIN_ID_VALUE;
 import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.NUM_REVS_THRESHOLD;
 import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.PREV_SPLIT_FACTOR;
 import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.setModified;
+import static org.apache.jackrabbit.oak.plugins.document.Revision.newRevision;
 import static org.apache.jackrabbit.oak.plugins.document.TestUtils.NO_BINARY;
 import static 
org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.VersionGCStats;
 import static org.junit.Assert.assertEquals;
@@ -260,6 +266,7 @@ public class VersionGarbageCollectorIT {
         clock.waitUntil(Revision.getCurrentTimestamp() + maxAge);
         VersionGCStats stats = gc.gc(maxAge, HOURS);
         assertEquals(0, stats.deletedPropsGCCount);
+        assertEquals(0, stats.updatedDetailedGCDocsCount);
 
         //Remove property
         NodeBuilder b2 = store.getRoot().builder();
@@ -273,12 +280,15 @@ public class VersionGarbageCollectorIT {
         clock.waitUntil(clock.getTime() + delta);
         stats = gc.gc(maxAge*2, HOURS);
         assertEquals(0, stats.deletedPropsGCCount);
+        assertEquals(0, stats.updatedDetailedGCDocsCount);
 
         //3. Check that deleted property does get collected post maxAge
         clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);
 
         stats = gc.gc(maxAge*2, HOURS);
         assertEquals(1, stats.deletedPropsGCCount);
+        assertEquals(1, stats.updatedDetailedGCDocsCount);
+        assertEquals("1:/z", stats.oldestModifiedDocId);
 
         //4. Check that a revived property (deleted and created again) does 
not get gc
         NodeBuilder b3 = store.getRoot().builder();
@@ -292,11 +302,12 @@ public class VersionGarbageCollectorIT {
         clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);
         stats = gc.gc(maxAge*2, HOURS);
         assertEquals(0, stats.deletedPropsGCCount);
+        assertEquals(0, stats.updatedDetailedGCDocsCount);
+        assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId);
     }
 
-    // Test when we have more than 1000 deleted properties
     @Test
-    public void testGCDeletedProps_1() throws Exception {
+    public void testGCDeletedProps_MoreThan_1000_WithSameRevision() throws 
Exception {
         //1. Create nodes with properties
         NodeBuilder b1 = store.getRoot().builder();
 
@@ -312,10 +323,6 @@ public class VersionGarbageCollectorIT {
         writeField(gc, "detailedGCEnabled", true, true);
         long maxAge = 1; //hours
         long delta = TimeUnit.MINUTES.toMillis(10);
-        //1. Go past GC age and check no GC done as nothing deleted
-        clock.waitUntil(Revision.getCurrentTimestamp() + maxAge);
-        VersionGCStats stats = gc.gc(maxAge, HOURS);
-        assertEquals(0, stats.deletedPropsGCCount);
 
         //Remove property
         NodeBuilder b2 = store.getRoot().builder();
@@ -328,77 +335,60 @@ public class VersionGarbageCollectorIT {
 
         store.runBackgroundOperations();
 
-        //2. Check that a deleted property is not collected before maxAge
-        //Clock cannot move back (it moved forward in #1) so double the maxAge
-        clock.waitUntil(clock.getTime() + delta);
-        stats = gc.gc(maxAge*2, HOURS);
-        assertEquals(0, stats.deletedPropsGCCount);
-
         //3. Check that deleted property does get collected post maxAge
         clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);
 
-        stats = gc.gc(maxAge*2, HOURS);
+        VersionGCStats stats = gc.gc(maxAge*2, HOURS);
         assertEquals(50_000, stats.deletedPropsGCCount);
-
+        assertEquals(5_000, stats.updatedDetailedGCDocsCount);
+        assertNotEquals(MIN_ID_VALUE, stats.oldestModifiedDocId);
     }
 
-    // Test when we have more than 1000 deleted properties with different 
revisions
     @Test
-    public void testGCDeletedProps_2() throws Exception {
+    public void testGCDeletedProps_MoreThan_1000_WithDifferentRevision() 
throws Exception {
         //1. Create nodes with properties
-        NodeBuilder b1 = null;
+        NodeBuilder b1 = store.getRoot().builder();
         for (int k = 0; k < 50; k ++) {
-            b1 = store.getRoot().builder();
             // Add property to node & save
             for (int i = 0; i < 100; i++) {
                 for (int j = 0; j < 10; j++) {
                     b1.child(k + "z" + i).setProperty("prop" + j, "foo", 
STRING);
                 }
             }
-            store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
-            // increase the clock to create new revision for next batch
-            clock.waitUntil(Revision.getCurrentTimestamp() + (k * 5));
         }
+        store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
 
         // enable the detailed gc flag
         writeField(gc, "detailedGCEnabled", true, true);
         long maxAge = 1; //hours
-        long delta = TimeUnit.MINUTES.toMillis(10);
-        //1. Go past GC age and check no GC done as nothing deleted
-        clock.waitUntil(Revision.getCurrentTimestamp() + maxAge);
-        VersionGCStats stats = gc.gc(maxAge, HOURS);
-        assertEquals(0, stats.deletedPropsGCCount);
+        long delta = TimeUnit.MINUTES.toMillis(20);
 
         //Remove property
         NodeBuilder b2 = store.getRoot().builder();
         for (int k = 0; k < 50; k ++) {
+            b2 = store.getRoot().builder();
             for (int i = 0; i < 100; i++) {
                 for (int j = 0; j < 10; j++) {
                     b2.getChildNode(k + "z" + i).removeProperty("prop" + j);
                 }
             }
+            store.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+            // increase the clock to create new revision for next batch
+            clock.waitUntil(Revision.getCurrentTimestamp() + (k * 5));
         }
-        store.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
 
         store.runBackgroundOperations();
-
-        //2. Check that a deleted property is not collected before maxAge
-        //Clock cannot move back (it moved forward in #1) so double the maxAge
-        clock.waitUntil(clock.getTime() + delta);
-        stats = gc.gc(maxAge*2, HOURS);
-        assertEquals(0, stats.deletedPropsGCCount);
-
         //3. Check that deleted property does get collected post maxAge
         clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);
 
-        stats = gc.gc(maxAge*2, HOURS);
+        VersionGCStats stats = gc.gc(maxAge, HOURS);
         assertEquals(50_000, stats.deletedPropsGCCount);
-
+        assertEquals(5_000, stats.updatedDetailedGCDocsCount);
+        assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId);
     }
 
-    // Test where we modify the already GCed nodes
     @Test
-    public void testGCDeletedProps_3() throws Exception {
+    public void testGCDeletedPropsAlreadyGCed() throws Exception {
         //1. Create nodes with properties
         NodeBuilder b1 = store.getRoot().builder();
         // Add property to node & save
@@ -429,6 +419,8 @@ public class VersionGarbageCollectorIT {
 
         stats = gc.gc(maxAge*2, HOURS);
         assertEquals(10, stats.deletedPropsGCCount);
+        assertEquals(10, stats.updatedDetailedGCDocsCount);
+        assertNotEquals(MIN_ID_VALUE, stats.oldestModifiedDocId);
 
         //3. now reCreate those properties again
         NodeBuilder b3 = store.getRoot().builder();
@@ -452,11 +444,12 @@ public class VersionGarbageCollectorIT {
         clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);
         stats = gc.gc(maxAge*2, HOURS);
         assertEquals(10, stats.deletedPropsGCCount);
+        assertEquals(10, stats.updatedDetailedGCDocsCount);
+        assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId);
     }
 
-    // Test properties are collected after system crash had happened
     @Test
-    public void testGCDeletedProps_4() throws Exception {
+    public void testGCDeletedPropsAfterSystemCrash() throws Exception {
         final FailingDocumentStore fds = new 
FailingDocumentStore(fixture.createDocumentStore(), 42) {
             @Override
             public void dispose() {}
@@ -519,12 +512,12 @@ public class VersionGarbageCollectorIT {
         clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);
         VersionGCStats stats = gc.gc(maxAge*2, HOURS);
         assertEquals(10, stats.deletedPropsGCCount);
-
+        assertEquals(10, stats.updatedDetailedGCDocsCount);
+        assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId);
     }
 
-    // Test when escaped properties are collected
     @Test
-    public void testGCDeletedProps_5() throws Exception {
+    public void testGCDeletedEscapeProps() throws Exception {
         //1. Create nodes with properties
         NodeBuilder b1 = store.getRoot().builder();
 
@@ -542,6 +535,7 @@ public class VersionGarbageCollectorIT {
         clock.waitUntil(Revision.getCurrentTimestamp() + maxAge);
         VersionGCStats stats = gc.gc(maxAge, HOURS);
         assertEquals(0, stats.deletedPropsGCCount);
+        assertEquals(0, stats.updatedDetailedGCDocsCount);
 
         //Remove property
         NodeBuilder b2 = store.getRoot().builder();
@@ -557,6 +551,7 @@ public class VersionGarbageCollectorIT {
         clock.waitUntil(clock.getTime() + delta);
         stats = gc.gc(maxAge*2, HOURS);
         assertEquals(0, stats.deletedPropsGCCount);
+        assertEquals(0, stats.updatedDetailedGCDocsCount);
 
         //3. Check that deleted property does get collected post maxAge
         clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);
@@ -574,7 +569,143 @@ public class VersionGarbageCollectorIT {
         clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);
         stats = gc.gc(maxAge*2, HOURS);
         assertEquals(0, stats.deletedPropsGCCount);
+        assertEquals(0, stats.updatedDetailedGCDocsCount);
+    }
+
+    @Test
+    public void testGCDeletedPropsWhenModifiedConcurrently() throws Exception {
+        //1. Create nodes with properties
+        NodeBuilder b1 = store.getRoot().builder();
+
+        // Add property to node & save
+        for (int i = 0; i < 10; i++) {
+            b1.child("x"+i).setProperty("test"+i, "t", STRING);
+        }
+        store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+        // enable the detailed gc flag
+        writeField(gc, "detailedGCEnabled", true, true);
+        long maxAge = 1; //hours
+        long delta = TimeUnit.MINUTES.toMillis(10);
+        //1. Go past GC age and check no GC done as nothing deleted
+        clock.waitUntil(Revision.getCurrentTimestamp() + maxAge);
+        VersionGCStats stats = gc.gc(maxAge, HOURS);
+        assertEquals(0, stats.deletedPropsGCCount);
+        assertEquals(0, stats.updatedDetailedGCDocsCount);
+
+        //Remove property
+        NodeBuilder b2 = store.getRoot().builder();
+        for (int i = 0; i < 10; i++) {
+            b2.getChildNode("x"+i).removeProperty("test"+i);
+        }
+        store.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        store.runBackgroundOperations();
+
+        //2. Check that a deleted property is not collected before maxAge
+        //Clock cannot move back (it moved forward in #1) so double the maxAge
+        clock.waitUntil(clock.getTime() + delta);
+        stats = gc.gc(maxAge*2, HOURS);
+        assertEquals(0, stats.deletedPropsGCCount);
+        assertEquals(0, stats.updatedDetailedGCDocsCount);
+        assertNull(stats.oldestModifiedDocId); // as GC hadn't run
+
+        //3. Check that deleted property does get collected post maxAge
+        clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);
+
+        VersionGCSupport gcSupport = new 
VersionGCSupport(store.getDocumentStore()) {
+
+            @Override
+            public Iterable<NodeDocument> getModifiedDocs(long fromModified, 
long toModified, int limit, @NotNull String fromId) {
+                Iterable<NodeDocument> modifiedDocs = 
super.getModifiedDocs(fromModified, toModified, limit, fromId);
+                List<NodeDocument> result = stream(modifiedDocs.spliterator(), 
false).collect(toList());
+                final Revision updateRev = newRevision(1);
+                store.getDocumentStore().findAndUpdate(NODES, 
stream(modifiedDocs.spliterator(), false)
+                        .map(doc -> {
+                            UpdateOp op = new 
UpdateOp(requireNonNull(doc.getId()), false);
+                            setModified(op, updateRev);
+                            return op;
+                        }).
+                        collect(toList())
+                );
+                return result;
+            }
+        };
+
+        VersionGarbageCollector gc = new VersionGarbageCollector(store, 
gcSupport, true);
+        stats = gc.gc(maxAge*2, HOURS);
+        assertEquals(0, stats.updatedDetailedGCDocsCount);
+        assertEquals(0, stats.deletedPropsGCCount);
+        assertNotEquals(MIN_ID_VALUE, stats.oldestModifiedDocId);
+    }
+
+    @Test
+    public void cancelDetailedGCAfterFirstBatch() throws Exception {
+        //1. Create nodes with properties
+        NodeBuilder b1 = store.getRoot().builder();
+
+        // Add property to node & save
+        for (int i = 0; i < 5_000; i++) {
+            for (int j = 0; j < 10; j++) {
+                b1.child("z"+i).setProperty("prop"+j, "foo", STRING);
+            }
+        }
+        store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        store.runBackgroundOperations();
+
+        // enable the detailed gc flag
+        writeField(gc, "detailedGCEnabled", true, true);
+        long maxAge = 1; //hours
+        long delta = TimeUnit.MINUTES.toMillis(10);
+        //1. Go past GC age and check no GC done as nothing deleted
+        clock.waitUntil(Revision.getCurrentTimestamp() + maxAge);
+        VersionGCStats stats = gc.gc(maxAge, HOURS);
+        assertEquals(0, stats.deletedPropsGCCount);
+        assertEquals(0, stats.updatedDetailedGCDocsCount);
 
+        //Remove property
+        NodeBuilder b2 = store.getRoot().builder();
+        for (int i = 0; i < 5_000; i++) {
+            for (int j = 0; j < 10; j++) {
+                b2.getChildNode("z"+i).removeProperty("prop"+j);
+            }
+        }
+        store.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        store.runBackgroundOperations();
+
+        final AtomicReference<VersionGarbageCollector> gcRef = 
Atomics.newReference();
+        final VersionGCSupport gcSupport = new 
VersionGCSupport(store.getDocumentStore()) {
+
+            @Override
+            public Iterable<NodeDocument> getModifiedDocs(long fromModified, 
long toModified, int limit, @NotNull String fromId) {
+                return () -> new AbstractIterator<>() {
+                    private final Iterator<NodeDocument> it = 
candidates(fromModified, toModified, limit, fromId);
+
+                    @Override
+                    protected NodeDocument computeNext() {
+                        if (it.hasNext()) {
+                            return it.next();
+                        }
+                        // cancel when we reach the end
+                        gcRef.get().cancel();
+                        return endOfData();
+                    }
+                };
+            }
+
+            private Iterator<NodeDocument> candidates(long fromModified, long 
toModified, int limit, @NotNull String fromId) {
+                return super.getModifiedDocs(fromModified, toModified, limit, 
fromId).iterator();
+            }
+        };
+
+        gcRef.set(new VersionGarbageCollector(store, gcSupport, true));
+
+        //3. Check that deleted property does get collected post maxAge
+        clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);
+        stats = gcRef.get().gc(maxAge*2, HOURS);
+        assertTrue(stats.canceled);
+        assertEquals(0, stats.updatedDetailedGCDocsCount);
+        assertEquals(0, stats.deletedPropsGCCount);
+        assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId);
     }
 
     // OAK-10199 END

Reply via email to