From 53b5e90aa60cebc7d2910b863ab1147ccff8f72e Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 10 May 2024 18:55:12 -0500 Subject: [PATCH] [JENKINS-21605] Further reduce number of recorded upstream causes (#9248) * 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 --- core/src/main/java/hudson/model/Cause.java | 18 ++++++++----- .../src/test/java/hudson/model/CauseTest.java | 25 ++++++++----------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/hudson/model/Cause.java b/core/src/main/java/hudson/model/Cause.java index 6990dd2b39e0..668587ffc8fc 100644 --- a/core/src/main/java/hudson/model/Cause.java +++ b/core/src/main/java/hudson/model/Cause.java @@ -182,6 +182,10 @@ public UpstreamCause(Run up) { upstreamCauses = new ArrayList<>(); Set 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)); } } @@ -239,14 +243,16 @@ public int hashCode() { } UpstreamCause uc = (UpstreamCause) c; List 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); } diff --git a/test/src/test/java/hudson/model/CauseTest.java b/test/src/test/java/hudson/model/CauseTest.java index 0d4caf54f61b..2484a03c52ed 100644 --- a/test/src/test/java/hudson/model/CauseTest.java +++ b/test/src/test/java/hudson/model/CauseTest.java @@ -63,9 +63,9 @@ public class CauseTest { FreeStyleProject b = j.createFreeStyleProject("b"); FreeStyleBuild early = null; FreeStyleBuild last = null; - List> 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; } @@ -74,9 +74,6 @@ public class CauseTest { assertTrue("keeps full history:\n" + buildXml, buildXml.contains("1")); buildXml = new XmlFile(Run.XSTREAM, new File(last.getRootDir(), "build.xml")).asString(); assertFalse("too big:\n" + buildXml, buildXml.contains("1")); - for (QueueTaskFuture future : futures) { - j.assertBuildStatusSuccess(j.waitForCompletion(future.waitForStart())); - } } @Issue("JENKINS-15747") @@ -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 next1 = recordFuture(a.scheduleBuild2(0, cause), futures); - recordFuture(a.scheduleBuild2(0, cause), futures); - cause = new Cause.UpstreamCause(next1.get()); - QueueTaskFuture next2 = recordFuture(b.scheduleBuild2(0, cause), futures); - recordFuture(b.scheduleBuild2(0, cause), futures); - cause = new Cause.UpstreamCause(next2.get()); - QueueTaskFuture 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(" 100); //j.interactiveBreak();