Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OAK-11166 -- Remove asserts for flaky tests in VersionGarbageCollecto… #1759

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
+121 −78
Open

OAK-11166 -- Remove asserts for flaky tests in VersionGarbageCollecto… #1759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0df2c5d
OAK-11166 -- Remove asserts for flaky tests in VersionGarbageCollecto…
Oct 4, 2024
5f6ef8e
OAK-11166 -- Add condition for different fixtures in BranchCommitGCTest
Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
190 changes: 121 additions & 69 deletions ...document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchCommitGCTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;

import java.io.IOException;
import java.util.LinkedList;
Expand All @@ -65,10 +64,14 @@
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@RunWith(Parameterized.class)
public class BranchCommitGCTest {

private static final Logger LOG = LoggerFactory.getLogger(BranchCommitGCTest.class);

@Rule
public DocumentMKBuilderProvider builderProvider = new DocumentMKBuilderProvider();
private Clock clock;
Expand All @@ -82,6 +85,19 @@ public static java.util.Collection<Object[]> params() throws IOException {
for (Object[] fixture : AbstractDocumentStoreTest.fixtures()) {
DocumentStoreFixture f = (DocumentStoreFixture)fixture[0];
for (FullGCMode gcType : FullGCMode.values()) {
if (gcType == FullGCMode.ORPHANS_EMPTYPROPS_BETWEEN_CHECKPOINTS_NO_UNMERGED_BC
|| gcType == FullGCMode.ORPHANS_EMPTYPROPS_BETWEEN_CHECKPOINTS_WITH_UNMERGED_BC) {
// temporarily skip due to flakyness
continue;
}
if (f.getName().equals("Memory") || f.getName().startsWith("RDB")) {
// then only run NONE and EMPTY_PROPS, cause we are rolling EMPTY_PROPS first
if (gcType != FullGCMode.NONE && gcType != FullGCMode.EMPTYPROPS
&& gcType != FullGCMode.GAP_ORPHANS_EMPTYPROPS) {
// temporarily skip due to slowness
continue;
}
}
params.add(new Object[] {f, gcType});
}
}
Expand Down Expand Up @@ -161,7 +177,6 @@ public void unmergedAddChildren() throws Exception {

@Test
public void unmergedAddThenMergedAddAndRemoveChildren() throws Exception {
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS);
RevisionVector br = unmergedBranchCommit(b -> {
b.child("a");
b.child("b");
Expand Down Expand Up @@ -198,7 +213,9 @@ public void unmergedAddThenMergedAddAndRemoveChildren() throws Exception {
if (collisionsBeforeGC == 1) {
// expects a collision to have happened - which was cleaned up - hence a _bc (but not the _revision I guess)
// the collisions cleaned up - with the 1 collision and a _bc
assertStatsCountsEqual(stats,
VersionGarbageCollector.VersionGCStats finalStats = stats;
assertEventually(() -> {
assertStatsCountsEqual(finalStats,
new GCCounts(FullGCMode.NONE,
2, 0, 0, 0, 0, 0, 0),
gapOrphOnly(2, 0, 0, 0, 0, 0, 0),
Expand All @@ -210,20 +227,24 @@ public void unmergedAddThenMergedAddAndRemoveChildren() throws Exception {
betweenChkp(2, 0, 0, 0, 0, 0, 0),
unmergedBcs(2, 0, 1, 0, 2, 1, 1),
btwnChkpUBC(2, 0, 1, 0, 2, 1, 1));
} else {
}, 10000);
} else {
// in this case classic collision cleanup already took care of everything, nothing left
assertStatsCountsEqual(stats,
new GCCounts(FullGCMode.NONE,
2, 0, 0, 0, 0, 0, 0),
gapOrphOnly(2, 0, 0, 0, 0, 0, 0),
empPropOnly(2, 0, 0, 0, 0, 0, 0),
gapOrphProp(2, 0, 0, 0, 0, 0, 0),
allOrphProp(2, 0, 0, 0, 0, 0, 0),
keepOneFull(2, 1, 0, 0, 0, 0, 0),
keepOneUser(2, 1, 0, 0, 0, 0, 0),
betweenChkp(2, 0, 0, 0, 0, 0, 0),
unmergedBcs(2, 0, 0, 0, 0, 0, 0),
btwnChkpUBC(2, 0, 0, 0, 0, 0, 0));
VersionGarbageCollector.VersionGCStats finalStats1 = stats;
assertEventually(() -> {
assertStatsCountsEqual(finalStats1,
new GCCounts(FullGCMode.NONE,
2, 0, 0, 0, 0, 0, 0),
gapOrphOnly(2, 0, 0, 0, 0, 0, 0),
empPropOnly(2, 0, 0, 0, 0, 0, 0),
gapOrphProp(2, 0, 0, 0, 0, 0, 0),
allOrphProp(2, 0, 0, 0, 0, 0, 0),
keepOneFull(2, 1, 0, 0, 0, 0, 0),
keepOneUser(2, 1, 0, 0, 0, 0, 0),
betweenChkp(2, 0, 0, 0, 0, 0, 0),
unmergedBcs(2, 0, 0, 0, 0, 0, 0),
btwnChkpUBC(2, 0, 0, 0, 0, 0, 0));
}, 10000);
}

assertNotExists("1:/a");
Expand All @@ -246,9 +267,6 @@ private int countCollisionsOnRoot() {

@Test
public void testDeletedPropsAndUnmergedBC() throws Exception {
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS);
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_BETWEEN_CHECKPOINTS_WITH_UNMERGED_BC);

// create a node with property.
NodeBuilder nb = store.getRoot().builder();
nb.child("bar").setProperty("prop", "value");
Expand Down Expand Up @@ -280,16 +298,18 @@ public void testDeletedPropsAndUnmergedBC() throws Exception {
VersionGarbageCollector.VersionGCStats stats = gc.gc(1, HOURS);

// 6 deleted props: 0:/[_collisions], 1:/foo[p, a], 1:/bar[_bc,prop,_revisions]
assertStatsCountsEqual(stats,
gapOrphOnly(),
empPropOnly(0, 3, 0, 0, 0, 0, 2),
gapOrphProp(0, 3, 0, 0, 0, 0, 2),
allOrphProp(0, 3, 0, 0, 0, 0, 2),
keepOneFull(0, 3, 2, 1,17, 0, 3),
keepOneUser(0, 3, 0, 1, 0, 0, 2),
betweenChkp(0, 3, 0, 0, 1, 0, 3),
unmergedBcs(0, 3, 2, 1,15, 4, 3),
btwnChkpUBC(0, 3, 2, 1,16, 4, 3));
assertEventually(() -> {
assertStatsCountsEqual(stats,
gapOrphOnly(),
empPropOnly(0, 3, 0, 0, 0, 0, 2),
gapOrphProp(0, 3, 0, 0, 0, 0, 2),
allOrphProp(0, 3, 0, 0, 0, 0, 2),
keepOneFull(0, 3, 2, 1, 17, 0, 3),
keepOneUser(0, 3, 0, 1, 0, 0, 2),
betweenChkp(0, 3, 0, 0, 1, 0, 3),
unmergedBcs(0, 3, 2, 1, 15, 4, 3),
btwnChkpUBC(0, 3, 2, 1, 16, 4, 3));
}, 10000);
if (!isModeOneOf(FullGCMode.NONE, FullGCMode.GAP_ORPHANS)) {
assertBranchRevisionRemovedFromAllDocuments(store, br1);
assertBranchRevisionRemovedFromAllDocuments(store, br2);
Expand All @@ -300,8 +320,6 @@ public void testDeletedPropsAndUnmergedBC() throws Exception {

@Test
public void unmergedAddTwoChildren() throws Exception {
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS);
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_UNMERGED_BC);
RevisionVector br1 = unmergedBranchCommit(b -> {
b.child("a");
b.child("b");
Expand All @@ -322,17 +340,19 @@ public void unmergedAddTwoChildren() throws Exception {
// clean everything older than one hour
VersionGarbageCollector.VersionGCStats stats = gc.gc(1, HOURS);

assertStatsCountsEqual(stats,
gapOrphOnly(),
empPropOnly(),
gapOrphProp(),
allOrphProp(2, 0, 0, 0, 0, 0, 2),
keepOneFull(2, 0, 2, 0, 2, 0, 3),
keepOneUser(2, 0, 0, 0, 0, 0, 2),
betweenChkp(2, 0, 0, 0, 0, 0, 2),
unmergedBcs(2, 0, 2, 0, 2, 2, 3),
btwnChkpUBC(2, 0, 2, 0, 2, 2, 3));

VersionGarbageCollector.VersionGCStats finalStats = stats;
assertEventually(() -> {
assertStatsCountsEqual(finalStats,
gapOrphOnly(),
empPropOnly(),
gapOrphProp(),
allOrphProp(2, 0, 0, 0, 0, 0, 2),
keepOneFull(2, 0, 2, 0, 2, 0, 3),
keepOneUser(2, 0, 0, 0, 0, 0, 2),
betweenChkp(2, 0, 0, 0, 0, 0, 2),
unmergedBcs(2, 0, 2, 0, 2, 2, 3),
btwnChkpUBC(2, 0, 2, 0, 2, 2, 3));
}, 10000);
if (!isModeOneOf(FullGCMode.NONE, FullGCMode.GAP_ORPHANS, FullGCMode.GAP_ORPHANS_EMPTYPROPS, FullGCMode.EMPTYPROPS)) {
assertNotExists("1:/a");
assertNotExists("1:/b");
Expand All @@ -350,7 +370,6 @@ public void unmergedAddTwoChildren() throws Exception {

@Test
public void unmergedAddsThenMergedAddsChildren() throws Exception {
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_BETWEEN_CHECKPOINTS_WITH_UNMERGED_BC);
RevisionVector br1 = unmergedBranchCommit(b -> {
b.child("a");
b.child("b");
Expand Down Expand Up @@ -378,16 +397,19 @@ public void unmergedAddsThenMergedAddsChildren() throws Exception {
// clean everything older than one hour
VersionGarbageCollector.VersionGCStats stats = gc.gc(1, HOURS);

assertStatsCountsEqual(stats,
gapOrphOnly(),
empPropOnly(),
gapOrphProp(),
allOrphProp(),
keepOneFull(0, 0, 1, 4,12, 0, 3),
keepOneUser(0, 0, 0, 4, 0, 0, 2),
betweenChkp(),
unmergedBcs(0, 0, 1, 0,16, 2, 3),
btwnChkpUBC(0, 0, 1, 0,16, 2, 3));
VersionGarbageCollector.VersionGCStats finalStats = stats;
assertEventually(() -> {
assertStatsCountsEqual(finalStats,
gapOrphOnly(),
empPropOnly(),
gapOrphProp(),
allOrphProp(),
keepOneFull(0, 0, 1, 4, 12, 0, 3),
keepOneUser(0, 0, 0, 4, 0, 0, 2),
betweenChkp(),
unmergedBcs(0, 0, 1, 0, 16, 2, 3),
btwnChkpUBC(0, 0, 1, 0, 16, 2, 3));
}, 10000);
assertExists("1:/a");
assertExists("1:/b");

Expand All @@ -403,8 +425,6 @@ public void unmergedAddsThenMergedAddsChildren() throws Exception {

@Test
public void unmergedAddsThenMergedAddThenUnmergedRemovesChildren() throws Exception {
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_UNMERGED_BC); // see OAK-10852
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS); // see OAK-10852
RevisionVector br1 = unmergedBranchCommit(b -> {
b.child("a");
b.child("b");
Expand Down Expand Up @@ -443,16 +463,19 @@ public void unmergedAddsThenMergedAddThenUnmergedRemovesChildren() throws Except
// clean everything older than one hour
VersionGarbageCollector.VersionGCStats stats = gc.gc(1, HOURS);

assertStatsCountsEqual(stats,
empPropOnly(0, 0, 0, 0, 0, 0, 0),
gapOrphOnly(0, 0, 0, 0, 0, 0, 0),
gapOrphProp(0, 0, 0, 0, 0, 0, 0),
allOrphProp(0, 0, 0, 0, 0, 0, 0),
keepOneFull(0, 0, 1, 8,24, 0, 3),
keepOneUser(0, 0, 0, 8, 0, 0, 2),
betweenChkp(),
unmergedBcs(0, 0, 1, 0,32, 4, 3),
btwnChkpUBC(0, 0, 1, 0,32, 4, 3));
VersionGarbageCollector.VersionGCStats finalStats = stats;
assertEventually(() -> {
assertStatsCountsEqual(finalStats,
empPropOnly(0, 0, 0, 0, 0, 0, 0),
gapOrphOnly(0, 0, 0, 0, 0, 0, 0),
gapOrphProp(0, 0, 0, 0, 0, 0, 0),
allOrphProp(0, 0, 0, 0, 0, 0, 0),
keepOneFull(0, 0, 1, 8,24, 0, 3),
keepOneUser(0, 0, 0, 8, 0, 0, 2),
betweenChkp(),
unmergedBcs(0, 0, 1, 0,32, 4, 3),
btwnChkpUBC(0, 0, 1, 0,32, 4, 3));
}, 10000);
assertExists("1:/a");
assertExists("1:/b");

Expand Down Expand Up @@ -569,8 +592,6 @@ public void unmergedAddProperty() throws Exception {

@Test
public void unmergedRemoveChild() throws Exception {
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS);
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_BETWEEN_CHECKPOINTS_WITH_UNMERGED_BC);
mergedBranchCommit(b -> {
b.child("foo");
b.child("bar");
Expand All @@ -595,7 +616,8 @@ public void unmergedRemoveChild() throws Exception {
// clean everything older than one hour
stats = gc.gc(1, HOURS);

assertStatsCountsEqual(stats,
assertEventually(() -> {
assertStatsCountsEqual(stats,
gapOrphOnly(0, 0, 0, 0, 0, 0, 0),
empPropOnly(0, 0, 0, 0, 0, 0, 0),
gapOrphProp(0, 0, 0, 0, 0, 0, 0),
Expand All @@ -605,7 +627,7 @@ public void unmergedRemoveChild() throws Exception {
betweenChkp(),
unmergedBcs(0, 0, 1, 0,50,10, 2),
btwnChkpUBC(0, 0, 1, 0,50,10, 2));

}, 10000);
doc = store.getDocumentStore().find(Collection.NODES, "1:/foo");
Long finalModified = doc.getModified();

Expand Down Expand Up @@ -790,4 +812,34 @@ private void assertNoEmptyProperties() {
}
}

public static void assertEventually(Runnable r, long timeoutMillis) {
final long start = System.currentTimeMillis();
long lastAttempt = 0;
int attempts = 0;

while (true) {
try {
attempts++;
LOG.info("assertEventually attempt count:{}", attempts);
lastAttempt = System.currentTimeMillis();
r.run();
return;
} catch (Throwable e) {
long elapsedTime = lastAttempt - start;
LOG.trace("assertEventually attempt {} failed because of {}", attempts, e.getMessage());
if (elapsedTime >= timeoutMillis) {
String msg = String.format("Condition not satisfied after %1.2f seconds and %d attempts",
elapsedTime / 1000d, attempts);
throw new AssertionError(msg, e);
}
try {
Thread.sleep(100);
} catch (InterruptedException ex) {
ex.printStackTrace();
}

}
}
}

}
9 changes: 0 additions & 9 deletions ...t/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1975,8 +1975,6 @@ public void testUnmergedBCRootCleanup() throws Exception {
// OAK-8646
@Test
public void testDeletedPropsAndUnmergedBCWithoutCollision() throws Exception {
// OAK-10974:
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS);
// create a node with property.
NodeBuilder nb = store1.getRoot().builder();
nb.child("bar").setProperty("prop", "value");
Expand Down Expand Up @@ -2023,8 +2021,6 @@ public void testDeletedPropsAndUnmergedBCWithoutCollision() throws Exception {

@Test
public void testDeletedPropsAndUnmergedBCWithCollision() throws Exception {
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS);
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_UNMERGED_BC);
// create a node with property.
NodeBuilder nb = store1.getRoot().builder();
nb.child("bar").setProperty("prop", "value");
Expand Down Expand Up @@ -2290,9 +2286,6 @@ public void testBundlingAndLatestSplit() throws Exception {

@Test
public void testBundledPropUnmergedBCGC() throws Exception {
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS);
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_UNMERGED_BC);

//0. Initialize bundling configs
final NodeBuilder builder = store1.getRoot().builder();
new InitialContent().initialize(builder);
Expand Down Expand Up @@ -2659,8 +2652,6 @@ public void testGCDeletedPropsWithDryRunMode() throws Exception {

@Test
public void testDeletedPropsAndUnmergedBCWithCollisionWithDryRunMode() throws Exception {
// OAK-10869:
assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS);
// create a node with property.
NodeBuilder nb = store1.getRoot().builder();
nb.child("bar").setProperty("prop", "value");
Expand Down