This is an automated email from the ASF dual-hosted git repository. daim pushed a commit to branch DetailedGC/OAK-10199 in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit bed62672b010747376146975f43f3adf334b6a67 Author: Rishabh Kumar <[email protected]> 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/VersionGarbageCollector.java | 7 + .../oak/plugins/document/VersionGCInitTest.java | 5 + .../document/VersionGarbageCollectorIT.java | 144 +++++++++++++++++++++ 3 files changed, 156 insertions(+) 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 08b05aea91..cca73d8e4b 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 @@ -786,6 +786,13 @@ public class VersionGarbageCollector { fromModifiedMs = fromModifiedMs + SECONDS.toMillis(5); foundDoc = true; // to run while loop again } + + // 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); } 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 561e7426a9..e333ee765e 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 @@ -83,9 +83,14 @@ public class VersionGCInitTest { vgc = store.find(SETTINGS, SETTINGS_COLLECTION_ID); assertNotNull(vgc); +<<<<<<< HEAD assertEquals(stats.oldestModifiedDocTimeStamp, vgc.get(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP)); assertEquals(stats.oldestModifiedDocId, vgc.get(SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_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)); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) } @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 42d2a93d84..44de2312c0 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 @@ -35,9 +35,13 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +<<<<<<< HEAD import static java.util.List.of; import static java.util.Objects.requireNonNull; import static java.util.concurrent.TimeUnit.MILLISECONDS; +======= +import static java.util.Objects.requireNonNull; +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) import static java.util.stream.Collectors.toList; import static java.util.stream.StreamSupport.stream; import static org.apache.commons.lang3.reflect.FieldUtils.writeField; @@ -50,6 +54,7 @@ import static org.apache.jackrabbit.oak.api.Type.NAME; import static org.apache.jackrabbit.oak.api.Type.STRING; import static org.apache.jackrabbit.oak.api.Type.STRINGS; import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES; +<<<<<<< HEAD import static org.apache.jackrabbit.oak.plugins.document.Collection.SETTINGS; import static org.apache.jackrabbit.oak.plugins.document.DetailGCHelper.assertBranchRevisionNotRemovedFromAllDocuments; import static org.apache.jackrabbit.oak.plugins.document.DetailGCHelper.assertBranchRevisionRemovedFromAllDocuments; @@ -57,12 +62,17 @@ import static org.apache.jackrabbit.oak.plugins.document.DetailGCHelper.enableDe import static org.apache.jackrabbit.oak.plugins.document.DetailGCHelper.enableDetailGCDryRun; import static org.apache.jackrabbit.oak.plugins.document.DetailGCHelper.mergedBranchCommit; import static org.apache.jackrabbit.oak.plugins.document.DetailGCHelper.unmergedBranchCommit; +======= +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) 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; +<<<<<<< HEAD import static org.apache.jackrabbit.oak.plugins.document.Revision.getCurrentTimestamp; +======= +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) 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.SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_PROP; @@ -441,9 +451,15 @@ public class VersionGarbageCollectorIT { long maxAge = 1; //hours long delta = MINUTES.toMillis(10); //1. Go past GC age and check no GC done as nothing deleted +<<<<<<< HEAD clock.waitUntil(getCurrentTimestamp() + maxAge); VersionGCStats stats = gc(gc, maxAge, HOURS); assertEquals(0, stats.deletedPropsCount); +======= + clock.waitUntil(Revision.getCurrentTimestamp() + maxAge); + VersionGCStats stats = gc.gc(maxAge, HOURS); + assertEquals(0, stats.deletedPropsGCCount); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) assertEquals(0, stats.updatedDetailedGCDocsCount); //Remove property @@ -456,17 +472,29 @@ public class VersionGarbageCollectorIT { //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); +<<<<<<< HEAD stats = gc(gc, maxAge*2, HOURS); assertEquals(0, stats.deletedPropsCount); +======= + stats = gc.gc(maxAge*2, HOURS); + assertEquals(0, stats.deletedPropsGCCount); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) assertEquals(0, stats.updatedDetailedGCDocsCount); //3. Check that deleted property does get collected post maxAge clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta); +<<<<<<< HEAD stats = gc(gc, maxAge*2, HOURS); assertEquals(1, stats.deletedPropsCount); assertEquals(1, stats.updatedDetailedGCDocsCount); assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); +======= + stats = gc.gc(maxAge*2, HOURS); + assertEquals(1, stats.deletedPropsGCCount); + assertEquals(1, stats.updatedDetailedGCDocsCount); + assertEquals("1:/z", stats.oldestModifiedDocId); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) //4. Check that a revived property (deleted and created again) does not get gc NodeBuilder b3 = store1.getRoot().builder(); @@ -478,8 +506,13 @@ public class VersionGarbageCollectorIT { store1.merge(b4, EmptyHook.INSTANCE, CommitInfo.EMPTY); clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta); +<<<<<<< HEAD stats = gc(gc, maxAge*2, HOURS); assertEquals(0, stats.deletedPropsCount); +======= + stats = gc.gc(maxAge*2, HOURS); + assertEquals(0, stats.deletedPropsGCCount); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) assertEquals(0, stats.updatedDetailedGCDocsCount); assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); } @@ -516,10 +549,17 @@ public class VersionGarbageCollectorIT { //3. Check that deleted property does get collected post maxAge clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta); +<<<<<<< HEAD VersionGCStats stats = gc(gc, maxAge*2, HOURS); assertEquals(50_000, stats.deletedPropsCount); assertEquals(5_000, stats.updatedDetailedGCDocsCount); assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); +======= + VersionGCStats stats = gc.gc(maxAge*2, HOURS); + assertEquals(50_000, stats.deletedPropsGCCount); + assertEquals(5_000, stats.updatedDetailedGCDocsCount); + assertNotEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) } @Test @@ -539,7 +579,11 @@ public class VersionGarbageCollectorIT { // enable the detailed gc flag writeField(gc, "detailedGCEnabled", true, true); long maxAge = 1; //hours +<<<<<<< HEAD long delta = MINUTES.toMillis(20); +======= + long delta = TimeUnit.MINUTES.toMillis(20); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) //Remove property NodeBuilder b2 = store1.getRoot().builder(); @@ -552,20 +596,40 @@ public class VersionGarbageCollectorIT { } store1.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY); // increase the clock to create new revision for next batch +<<<<<<< HEAD clock.waitUntil(getCurrentTimestamp() + (k * 5)); +======= + clock.waitUntil(Revision.getCurrentTimestamp() + (k * 5)); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) } store1.runBackgroundOperations(); +<<<<<<< HEAD //3. Check that deleted property does get collected post maxAge clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta); VersionGCStats stats = gc(gc, maxAge, HOURS); assertEquals(50_000, stats.deletedPropsCount); +======= + + //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); + VersionGCStats 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, HOURS); + assertEquals(50_000, stats.deletedPropsGCCount); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) assertEquals(5_000, stats.updatedDetailedGCDocsCount); assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); } @Test +<<<<<<< HEAD public void testGC_WithNoDeletedProps_And_MoreThan_10_000_DocWithDifferentRevision() throws Exception { //1. Create nodes with properties NodeBuilder b1 = store1.getRoot().builder(); @@ -607,6 +671,8 @@ public class VersionGarbageCollectorIT { } @Test +======= +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) public void testGCDeletedPropsAlreadyGCed() throws Exception { //1. Create nodes with properties NodeBuilder b1 = store1.getRoot().builder(); @@ -636,10 +702,17 @@ public class VersionGarbageCollectorIT { //2. Check that deleted property does get collected post maxAge clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta); +<<<<<<< HEAD stats = gc(gc, maxAge*2, HOURS); assertEquals(10, stats.deletedPropsCount); assertEquals(10, stats.updatedDetailedGCDocsCount); assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); +======= + stats = gc.gc(maxAge*2, HOURS); + assertEquals(10, stats.deletedPropsGCCount); + assertEquals(10, stats.updatedDetailedGCDocsCount); + assertNotEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) //3. now reCreate those properties again NodeBuilder b3 = store1.getRoot().builder(); @@ -661,17 +734,25 @@ public class VersionGarbageCollectorIT { //4. Check that deleted property does get collected again // increment the clock again by more than 2 hours + delta clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta); +<<<<<<< HEAD stats = gc(gc, maxAge*2, HOURS); assertEquals(10, stats.deletedPropsCount); +======= + stats = gc.gc(maxAge*2, HOURS); + assertEquals(10, stats.deletedPropsGCCount); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) assertEquals(10, stats.updatedDetailedGCDocsCount); assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); } @Test public void testGCDeletedPropsAfterSystemCrash() throws Exception { +<<<<<<< HEAD if (store1 != null) { store1.dispose(); } +======= +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) final FailingDocumentStore fds = new FailingDocumentStore(fixture.createDocumentStore(), 42) { @Override public void dispose() {} @@ -732,8 +813,13 @@ public class VersionGarbageCollectorIT { //4. Check that deleted property does get collected again // increment the clock again by more than 2 hours + delta clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta); +<<<<<<< HEAD VersionGCStats stats = gc(gc, maxAge*2, HOURS); assertEquals(10, stats.deletedPropsCount); +======= + VersionGCStats stats = gc.gc(maxAge*2, HOURS); + assertEquals(10, stats.deletedPropsGCCount); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) assertEquals(10, stats.updatedDetailedGCDocsCount); assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); } @@ -754,9 +840,15 @@ public class VersionGarbageCollectorIT { long maxAge = 1; //hours long delta = MINUTES.toMillis(10); //1. Go past GC age and check no GC done as nothing deleted +<<<<<<< HEAD clock.waitUntil(getCurrentTimestamp() + maxAge); VersionGCStats stats = gc(gc, maxAge, HOURS); assertEquals(0, stats.deletedPropsCount); +======= + clock.waitUntil(Revision.getCurrentTimestamp() + maxAge); + VersionGCStats stats = gc.gc(maxAge, HOURS); + assertEquals(0, stats.deletedPropsGCCount); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) assertEquals(0, stats.updatedDetailedGCDocsCount); //Remove property @@ -771,8 +863,13 @@ public class VersionGarbageCollectorIT { //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); +<<<<<<< HEAD stats = gc(gc, maxAge*2, HOURS); assertEquals(0, stats.deletedPropsCount); +======= + stats = gc.gc(maxAge*2, HOURS); + assertEquals(0, stats.deletedPropsGCCount); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) assertEquals(0, stats.updatedDetailedGCDocsCount); //3. Check that deleted property does get collected post maxAge @@ -789,12 +886,18 @@ public class VersionGarbageCollectorIT { store1.merge(b4, EmptyHook.INSTANCE, CommitInfo.EMPTY); clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta); +<<<<<<< HEAD stats = gc(gc, maxAge*2, HOURS); assertEquals(0, stats.deletedPropsCount); +======= + stats = gc.gc(maxAge*2, HOURS); + assertEquals(0, stats.deletedPropsGCCount); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) assertEquals(0, stats.updatedDetailedGCDocsCount); } @Test +<<<<<<< HEAD public void testGCDeletedLongPathProps() throws Exception { //1. Create nodes with properties NodeBuilder b1 = store1.getRoot().builder(); @@ -963,6 +1066,8 @@ public class VersionGarbageCollectorIT { } @Test +======= +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) public void testGCDeletedPropsWhenModifiedConcurrently() throws Exception { //1. Create nodes with properties NodeBuilder b1 = store1.getRoot().builder(); @@ -976,11 +1081,19 @@ public class VersionGarbageCollectorIT { // enable the detailed gc flag writeField(gc, "detailedGCEnabled", true, true); long maxAge = 1; //hours +<<<<<<< HEAD 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); assertEquals(0, stats.deletedPropsCount); +======= + 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); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) assertEquals(0, stats.updatedDetailedGCDocsCount); //Remove property @@ -994,10 +1107,17 @@ public class VersionGarbageCollectorIT { //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); +<<<<<<< HEAD stats = gc(gc, maxAge*2, HOURS); assertEquals(0, stats.deletedPropsCount); assertEquals(0, stats.updatedDetailedGCDocsCount); assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); // as GC hadn't run +======= + stats = gc.gc(maxAge*2, HOURS); + assertEquals(0, stats.deletedPropsGCCount); + assertEquals(0, stats.updatedDetailedGCDocsCount); + assertNull(stats.oldestModifiedDocId); // as GC hadn't run +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) //3. Check that deleted property does get collected post maxAge clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta); @@ -1021,11 +1141,19 @@ public class VersionGarbageCollectorIT { } }; +<<<<<<< HEAD VersionGarbageCollector gc = new VersionGarbageCollector(store1, gcSupport, true, false); stats = gc.gc(maxAge*2, HOURS); assertEquals(0, stats.updatedDetailedGCDocsCount); assertEquals(0, stats.deletedPropsCount); assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); +======= + VersionGarbageCollector gc = new VersionGarbageCollector(store1, gcSupport, true); + stats = gc.gc(maxAge*2, HOURS); + assertEquals(0, stats.updatedDetailedGCDocsCount); + assertEquals(0, stats.deletedPropsGCCount); + assertNotEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) } @Test @@ -1045,11 +1173,19 @@ public class VersionGarbageCollectorIT { // enable the detailed gc flag writeField(gc, "detailedGCEnabled", true, true); long maxAge = 1; //hours +<<<<<<< HEAD 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); assertEquals(0, stats.deletedPropsCount); +======= + 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); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) assertEquals(0, stats.updatedDetailedGCDocsCount); //Remove property @@ -1087,14 +1223,22 @@ public class VersionGarbageCollectorIT { } }; +<<<<<<< HEAD gcRef.set(new VersionGarbageCollector(store1, gcSupport, true, false)); +======= + gcRef.set(new VersionGarbageCollector(store1, gcSupport, true)); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) //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); +<<<<<<< HEAD assertEquals(0, stats.deletedPropsCount); +======= + assertEquals(0, stats.deletedPropsGCCount); +>>>>>>> d3b73cd921 (OAK-10199 : added unit cases to handle concurrent prop update and escaped properties update) assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId); }
