You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by st...@apache.org on 2024/04/02 10:10:30 UTC
(jackrabbit-oak) 01/01: OAK-10734 : keep _revisions entry for non bcs younger than sweep - added test case to showcase as parentWithGarbageGCChildIndependent
This is an automated email from the ASF dual-hosted git repository.
stefanegli pushed a commit to branch OAK-10734
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit 7a9b0df2eff3fd2407cc2bd1557be061c2b85658
Author: Stefan Egli <st...@apache.org>
AuthorDate: Tue Apr 2 12:10:17 2024 +0200
OAK-10734 : keep _revisions entry for non bcs younger than sweep - added test case to showcase as parentWithGarbageGCChildIndependent
---
.../plugins/document/VersionGarbageCollector.java | 2 +-
.../document/VersionGarbageCollectorIT.java | 87 ++++++++++++++++++++--
2 files changed, 81 insertions(+), 8 deletions(-)
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 b772258fbc..e0ef315ae8 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
@@ -1488,7 +1488,7 @@ public class VersionGarbageCollector {
}
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);
final boolean newerThanSweep = nodeStore.getSweepRevisions().isRevisionNewer(revision);
if (newerThanSweep) {
//TODO wondering if we should at all do any DGC on documents newer than sweep
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 f249d111b9..98d7fa1e84 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
@@ -46,6 +46,7 @@ import static java.util.Objects.requireNonNull;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.stream.Collectors.toList;
import static java.util.stream.StreamSupport.stream;
+import static org.apache.commons.lang3.reflect.FieldUtils.readField;
import static org.apache.commons.lang3.reflect.FieldUtils.writeField;
import static org.apache.commons.lang3.reflect.FieldUtils.writeStaticField;
import static org.apache.jackrabbit.guava.common.base.Strings.repeat;
@@ -96,6 +97,7 @@ import static org.junit.Assume.assumeTrue;
import org.apache.jackrabbit.guava.common.base.Function;
import org.apache.jackrabbit.guava.common.base.Predicate;
+import org.apache.jackrabbit.guava.common.cache.Cache;
import org.apache.jackrabbit.guava.common.collect.AbstractIterator;
import org.apache.jackrabbit.guava.common.collect.ImmutableList;
import org.apache.jackrabbit.guava.common.collect.Iterators;
@@ -559,7 +561,7 @@ public class VersionGarbageCollectorIT {
stats = gc(gc, maxAge*2, HOURS);
assertStatsCountsEqual(stats,
noOldPropGc(0, 0, 0, 0, 0, 0, 0),
- keepOneFull(0, 0, 0, 2, 1, 0, 1),
+ keepOneFull(0, 0, 0, 2, 0, 0, 1),
keepOneUser(0, 0, 0, 2, 0, 0, 1),
betweenChkp(0, 0, 0, 0, 0, 0, 0));
assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId);
@@ -1263,6 +1265,77 @@ public class VersionGarbageCollectorIT {
// 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");
+ store1.merge(nb, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ // now unlike usually, DONT do a store1.runBackgroundOperations() here
+ // this will leave the parent GC-able
+
+ // check state of 2:/parent/child.ck before GC
+ // invalidate a bunch of caches, in particular nodecache and commitvaluecache
+ store1.getNodeCache().invalidateAll();
+ store1.getNodeChildrenCache().invalidateAll();
+ CachingCommitValueResolver cvr = (CachingCommitValueResolver) readField(store1, "commitValueResolver", true);
+ Cache<Revision, String> commitValueCache = (Cache<Revision, String>) readField( cvr, "commitValueCache", true);
+ commitValueCache.invalidateAll();
+ PropertyState ckBefore = store1.getRoot().getChildNode("parent").getChildNode("child").getProperty("ck");
+ assertNotNull(ckBefore);
+
+ // now the GC
+ VersionGCStats stats = gc(gc, 1, HOURS);
+
+ // check state of 2:/parent/child.ck after GC - should match before GC state
+ // invalidate a bunch of caches, in particular nodecache and commitvaluecache
+ store1.getNodeCache().invalidateAll();
+ store1.getNodeChildrenCache().invalidateAll();
+ cvr = (CachingCommitValueResolver) readField(store1, "commitValueResolver", true);
+ commitValueCache = (Cache<Revision, String>) readField( cvr, "commitValueCache", true);
+ commitValueCache.invalidateAll();
+ PropertyState ckAfter = store1.getRoot().getChildNode("parent").getChildNode("child").getProperty("ck");
+ assertNotNull(ckAfter);
+ assertEquals(ckBefore.getValue(Type.STRING), ckAfter.getValue(Type.STRING));
+ assertStatsCountsEqual(stats,
+ noOldPropGc(0, 0, 0, 0, 0, 0, 0),
+ keepOneFull(0, 0, 0, 3, 0, 0, 2),
+ keepOneUser(0, 0, 0, 3, 0, 0, 2),
+ betweenChkp(0, 0, 0, 0, 0, 0, 0));
+ }
+
@Test
public void parentGCChildIndependent() throws Exception {
assumeTrue(fixture.hasSinglePersistence());
@@ -1410,7 +1483,7 @@ public class VersionGarbageCollectorIT {
// deletedPropRevsCount=1 : (nothing on /node1[a, _commitRoot), /[_revisions]
assertStatsCountsEqual(stats,
noOldPropGc(0, 0, 0, 0, 0, 0, 0),
- keepOneFull(0, 0, 0, 0, 0, 0, 0),
+ keepOneFull(0, 0, 1, 0, 1, 0, 1),
keepOneUser(0, 0, 0, 0, 0, 0, 0),
betweenChkp(0, 0, 1, 0, 2, 1, 1));
// checking for br1 revisino to have disappeared doesn't really make much sense,
@@ -1452,7 +1525,7 @@ public class VersionGarbageCollectorIT {
assertStatsCountsEqual(stats,
noOldPropGc(0, 3, 0, 0, 0, 0, 2),
- keepOneFull(0, 3, 2, 1, 9, 0, 3),
+ keepOneFull(0, 3, 1, 1, 9, 0, 3),
keepOneUser(0, 3, 0, 1, 0, 0, 2),
betweenChkp(0, 3, 1, 1, 8, 2, 3));
assertBranchRevisionRemovedFromAllDocuments(store1, br1);
@@ -1493,7 +1566,7 @@ public class VersionGarbageCollectorIT {
VersionGCStats stats = gc(gc, 1, HOURS);
assertStatsCountsEqual(stats,
noOldPropGc(0, 3, 0, 0, 0, 0, 2),
- keepOneFull(0, 3, 3, 1, 17, 0, 3),
+ keepOneFull(0, 3, 2, 1, 17, 0, 3),
keepOneUser(0, 3, 0, 1, 0, 0, 2),
betweenChkp(0, 3, 2, 1, 18, 4, 3));
assertBranchRevisionRemovedFromAllDocuments(store1, br1);
@@ -2044,7 +2117,7 @@ public class VersionGarbageCollectorIT {
// removeProperty(prop) plus 1 _commitRoot entry
assertStatsCountsEqual(stats,
noOldPropGc(0, 0, 0, 0, 0, 0, 0),
- keepOneFull(0, 0, 0, 2, 1, 0, 1),
+ keepOneFull(0, 0, 0, 2, 0, 0, 1),
keepOneUser(0, 0, 0, 2, 0, 0, 1),
betweenChkp(0, 0, 0, 0, 0, 0, 0));
assertDocumentsExist(of("/bar"));
@@ -2076,7 +2149,7 @@ public class VersionGarbageCollectorIT {
// 1 prop-rev removal : the late-write null
assertStatsCountsEqual(stats,
noOldPropGc(0, 0, 0, 0, 0, 0, 0),
- keepOneFull(0, 0, 1, 1, 0, 0, 1),
+ keepOneFull(0, 0, 0, 1, 0, 0, 1),
keepOneUser(0, 0, 0, 1, 0, 0, 1),
betweenChkp(0, 0, 0, 0, 0, 0, 0));
assertDocumentsExist(of("/bar"));
@@ -2110,7 +2183,7 @@ public class VersionGarbageCollectorIT {
// removes 1 prop-rev : the late-write null
assertStatsCountsEqual(stats,
noOldPropGc(0, 0, 0, 0, 0, 0, 0),
- keepOneFull(0, 0, 1, 1, 0, 0, 1),
+ keepOneFull(0, 0, 0, 1, 0, 0, 1),
keepOneUser(0, 0, 0, 1, 0, 0, 1),
betweenChkp(0, 0, 0, 0, 0, 0, 0));
assertDocumentsExist(of("/foo/bar/baz"));