rishabhdaim commented on code in PR #1393:
URL: https://github.com/apache/jackrabbit-oak/pull/1393#discussion_r1548075492


##########
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java:
##########
@@ -1263,6 +1265,77 @@ private Iterator<NodeDocument> candidates(long 
fromModified, long toModified, in
 
     // OAK-10199 END
 
+    @SuppressWarnings("unchecked")
+    /**
+     * Test when a revision on a parent is becoming garbage on a property
+     * but not on "_revision" as it is (potentially but in this case indeed)
+     * still required by a child (2:/parent/child "ck").
+     * Note that this is not involving and branch commits.
+     */
+    @Test
+    public void parentWithGarbageGCChildIndependent() throws Exception { // 
rename me
+        assumeTrue(fixture.hasSinglePersistence());
+        NodeBuilder nb = store1.getRoot().builder();
+        NodeBuilder parent = nb.child("parent");
+        parent.setProperty("pk", "pv");
+        parent.child("child").setProperty("ck", "cv");
+        store1.merge(nb, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        store1.runBackgroundOperations();
+        nb = store1.getRoot().builder();
+        nb.child("parent").setProperty("pk", "pv2");
+        nb.child("parent").child("child").setProperty("ck", "cv2");
+        store1.merge(nb, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        nb = store1.getRoot().builder();
+        nb.child("parent").setProperty("pk", "pv3");
+        store1.merge(nb, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        // not doing a bgops == sweep here to cause "cv2" revision
+        // not being considered as commited by sweep
+        // (the test goal is to cause that resolution to fail in case
+        // we would delete the parent's _revisions entry)
+
+        // enable the detailed gc flag
+        writeField(gc, "detailedGCEnabled", true, true);
+
+        // wait two hours
+        clock.waitUntil(clock.getTime() + HOURS.toMillis(2));
+
+        // now make a child change so that only parent (and root) gets GCed
+        nb = store1.getRoot().builder();
+        nb.child("parent").child("child").setProperty("ck", "cv2");

Review Comment:
   shouldn't this be set to a different value here?



##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java:
##########
@@ -1488,7 +1488,7 @@ private int collectUnusedInternalPropertyRevisions(final 
NodeDocument doc,
                 }
                 final boolean isRoot = 
doc.getId().equals(Utils.getIdFromPath(Path.ROOT));
                 // local bcs only considered for removal
-                final boolean isBC = 
!doc.getLocalBranchCommits().contains(revision);
+                final boolean isBC = 
doc.getLocalBranchCommits().contains(revision);

Review Comment:
   good catch.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to