Skip to content

Commit f5bec27

Browse files
lewisbirkstimja
andauthored
fix: don't cache blocks when part of an active node (#1255)
Co-authored-by: Tim Jacomb <timjacomb1@gmail.com>
1 parent d450b6e commit f5bec27

3 files changed

Lines changed: 32 additions & 9 deletions

File tree

src/main/java/io/jenkins/plugins/pipelinegraphview/treescanner/NodeRelationship.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,16 @@ public FlowNode getEnd() {
136136
}
137137

138138
/**
139-
* Same as {@link #getStatus(WorkflowRun)} but threads {@code activeNodeIds} through to
140-
* the step-node leaf. Block cases defer to {@link #getStatus(WorkflowRun)} so subclass
141-
* overrides (e.g. {@link ParallelBlockRelationship}) apply.
139+
* Same as {@link #getStatus(WorkflowRun)} but uses {@code activeNodeIds} for liveness
140+
* checks instead of {@link FlowNode#isActive()}.
141+
*
142+
* <ul>
143+
* <li>Step nodes ({@code start == end}): liveness read from the set.</li>
144+
* <li>Block nodes ({@code start != end}): if {@code start} is in the active set the
145+
* block is still running (RUNNING/UNKNOWN); otherwise delegates to
146+
* {@link #getStatus(WorkflowRun)} so subclass overrides (e.g.
147+
* {@link ParallelBlockRelationship}) apply.</li>
148+
* </ul>
142149
*/
143150
public @NonNull NodeRunStatus getStatus(WorkflowRun run, @CheckForNull Set<String> activeNodeIds) {
144151
if (activeNodeIds == null) {
@@ -156,9 +163,17 @@ public FlowNode getEnd() {
156163
return new NodeRunStatus(GenericStatus.fromResult(warningAction.getResult()));
157164
}
158165
}
166+
// Step node: start and end are the same FlowNode — use the activeNodeIds set.
159167
if (this.start.getId().equals(this.end.getId())) {
160168
return new NodeRunStatus(this.start, activeNodeIds);
161169
}
170+
// Block node: if the start is in the active set the block is still running.
171+
// computeChunkStatus2 / getStatus(run) use FlowNode#isActive() / isCurrentHead()
172+
// which can disagree with the snapshot's activeNodeIds during live execution.
173+
// Honour the snapshot's view here so status stays consistent within a single read.
174+
if (activeNodeIds.contains(this.start.getId())) {
175+
return new NodeRunStatus(BlueRun.BlueRunResult.UNKNOWN, BlueRun.BlueRunState.RUNNING);
176+
}
162177
return getStatus(run);
163178
}
164179
}

src/main/java/io/jenkins/plugins/pipelinegraphview/treescanner/PipelineNodeTreeScanner.java

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -557,15 +557,23 @@ private void assignParent(@NonNull FlowNodeWrapper wrappedNode, @CheckForNull Fl
557557
FlowNode start = relationship.getStart();
558558
FlowNode end = relationship.getEnd();
559559
// Timing and status for a closed block are stable once the BlockEndNode
560-
// exists; memoise so subsequent requests don't re-walk actions per block.
561-
BlockResolutionCache cache = (start != end && end instanceof BlockEndNode<?>)
562-
? LiveGraphRegistry.get().blockResolutionCache(execution)
563-
: null;
560+
// exists AND the block is no longer active; memoise so subsequent requests
561+
// don't re-walk actions per block.
562+
// We must not cache while the start node is still active: the status can
563+
// still change (e.g. IN_PROGRESS → SUCCESS), and an incorrectly cached
564+
// value would be returned for the remainder of the run.
565+
boolean blockClosed = start != end
566+
&& end instanceof BlockEndNode<?>
567+
&& (activeNodeIds == null || !activeNodeIds.contains(start.getId()));
568+
BlockResolutionCache cache =
569+
blockClosed ? LiveGraphRegistry.get().blockResolutionCache(execution) : null;
564570
if (cache != null) {
565571
timing = cache.getOrComputeTiming(
566572
start.getId(), end.getId(), () -> relationship.getTimingInfo(this.run));
573+
// Use the activeNodeIds-aware overload so the set is consulted even for
574+
// cached entries — the cache supplier captures activeNodeIds by reference.
567575
status = cache.getOrComputeStatus(
568-
start.getId(), end.getId(), () -> relationship.getStatus(this.run));
576+
start.getId(), end.getId(), () -> relationship.getStatus(this.run, activeNodeIds));
569577
} else {
570578
timing = relationship.getTimingInfo(this.run);
571579
status = relationship.getStatus(this.run, activeNodeIds);

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/PipelineGraphViewCache.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
*/
3838
public class PipelineGraphViewCache {
3939

40-
public static final int SCHEMA_VERSION = 1;
40+
public static final int SCHEMA_VERSION = 2;
4141

4242
public static final String TREE_FILE_NAME = "pipeline-graph-view-tree.v" + SCHEMA_VERSION + ".json";
4343
public static final String ALL_STEPS_FILE_NAME = "pipeline-graph-view-allsteps.v" + SCHEMA_VERSION + ".json";

0 commit comments

Comments
 (0)