Skip to content

Commit

Permalink
[JENKINS-21605] Further reduce number of recorded upstream causes (#9248
Browse files Browse the repository at this point in the history
)

* Fix non-determinism in nested cause tests

For deeplyNestedCauses, quickly triggering a jobs with no quiet period
runs the possibility that a job is triggered while it is still in the
queue. Triggering and waiting for the jobs to execute synchronously
avoids this and also simplifies the code.

The broadlyNesstedCauses test suffers from the same problem, but is
additionally not structured in the way the original issue describes to
reproduce the issue. Using the quiet period argument to scheduleBuild2,
we can reliably produce a graph where each build is triggered by the
other two jobs. An additional final job outside of the loop cuts off the
1 second quiet period from the last loop's iteration so that the test is
never waiting for the quiet period of any jobs even though half of the
schedule operations specify one.

* Re-work nested cause trimming algorithm
  • Loading branch information
cottsay authored May 10, 2024
1 parent 57d6464 commit 53b5e90
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 20 deletions.
18 changes: 12 additions & 6 deletions core/src/main/java/hudson/model/Cause.java
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,10 @@ public UpstreamCause(Run<?, ?> up) {
upstreamCauses = new ArrayList<>();
Set<String> traversed = new HashSet<>();
for (Cause c : up.getCauses()) {
if (traversed.size() >= MAX_LEAF) {
upstreamCauses.add(new DeeplyNestedUpstreamCause());
break;
}
upstreamCauses.add(trim(c, MAX_DEPTH, traversed));
}
}
Expand Down Expand Up @@ -239,14 +243,16 @@ public int hashCode() {
}
UpstreamCause uc = (UpstreamCause) c;
List<Cause> cs = new ArrayList<>();
if (depth > 0) {
if (traversed.add(uc.upstreamUrl + uc.upstreamBuild)) {
for (Cause c2 : uc.upstreamCauses) {
cs.add(trim(c2, depth - 1, traversed));
if (traversed.add(uc.upstreamUrl + uc.upstreamBuild)) {
for (Cause c2 : uc.upstreamCauses) {
if (depth <= 0 || traversed.size() >= MAX_LEAF) {
cs.add(new DeeplyNestedUpstreamCause());
break;
}
cs.add(trim(c2, depth - 1, traversed));
}
} else if (traversed.size() < MAX_LEAF) {
cs.add(new DeeplyNestedUpstreamCause());
} else {
traversed.add(uc.upstreamUrl + uc.upstreamBuild + '#' + traversed.size());
}
return new UpstreamCause(uc.upstreamProject, uc.upstreamBuild, uc.upstreamUrl, cs);
}
Expand Down
25 changes: 11 additions & 14 deletions test/src/test/java/hudson/model/CauseTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ public class CauseTest {
FreeStyleProject b = j.createFreeStyleProject("b");
FreeStyleBuild early = null;
FreeStyleBuild last = null;
List<QueueTaskFuture<FreeStyleBuild>> futures = new ArrayList<>();
for (int i = 1; i <= 15; i++) {
last = recordFuture(b.scheduleBuild2(0, new Cause.UpstreamCause(recordFuture(a.scheduleBuild2(0, last == null ? null : new Cause.UpstreamCause(last)), futures).get())), futures).get();
last = j.waitForCompletion(a.scheduleBuild2(0, last == null ? null : new Cause.UpstreamCause(last)).get());
last = j.waitForCompletion(b.scheduleBuild2(0, new Cause.UpstreamCause(last)).get());
if (i == 5) {
early = last;
}
Expand All @@ -74,9 +74,6 @@ public class CauseTest {
assertTrue("keeps full history:\n" + buildXml, buildXml.contains("<upstreamBuild>1</upstreamBuild>"));
buildXml = new XmlFile(Run.XSTREAM, new File(last.getRootDir(), "build.xml")).asString();
assertFalse("too big:\n" + buildXml, buildXml.contains("<upstreamBuild>1</upstreamBuild>"));
for (QueueTaskFuture<FreeStyleBuild> future : futures) {
j.assertBuildStatusSuccess(j.waitForCompletion(future.waitForStart()));
}
}

@Issue("JENKINS-15747")
Expand All @@ -88,16 +85,16 @@ public class CauseTest {
Run<?, ?> last = null;
for (int i = 1; i <= 10; i++) {
Cause cause = last == null ? null : new Cause.UpstreamCause(last);
QueueTaskFuture<FreeStyleBuild> next1 = recordFuture(a.scheduleBuild2(0, cause), futures);
recordFuture(a.scheduleBuild2(0, cause), futures);
cause = new Cause.UpstreamCause(next1.get());
QueueTaskFuture<FreeStyleBuild> next2 = recordFuture(b.scheduleBuild2(0, cause), futures);
recordFuture(b.scheduleBuild2(0, cause), futures);
cause = new Cause.UpstreamCause(next2.get());
QueueTaskFuture<FreeStyleBuild> next3 = recordFuture(c.scheduleBuild2(0, cause), futures);
recordFuture(c.scheduleBuild2(0, cause), futures);
last = next3.get();
last = j.waitForCompletion(a.scheduleBuild2(0, cause).get());
recordFuture(b.scheduleBuild2(1, cause), futures);
cause = new Cause.UpstreamCause(last);
last = j.waitForCompletion(b.scheduleBuild2(0, cause).get());
recordFuture(c.scheduleBuild2(1, cause), futures);
cause = new Cause.UpstreamCause(last);
last = j.waitForCompletion(c.scheduleBuild2(0, cause).get());
recordFuture(a.scheduleBuild2(1, cause), futures);
}
last = j.waitForCompletion(a.scheduleBuild2(0, new Cause.UpstreamCause(last)).get());
int count = new XmlFile(Run.XSTREAM, new File(last.getRootDir(), "build.xml")).asString().split(Pattern.quote("<hudson.model.Cause_-UpstreamCause")).length;
assertFalse("too big at " + count, count > 100);
//j.interactiveBreak();
Expand Down

0 comments on commit 53b5e90

Please sign in to comment.