Skip to content

Commit

Permalink
Merge pull request #354 from jglick/PlaceholderTask
Browse files Browse the repository at this point in the history
Simplified `PlaceholderTask`
  • Loading branch information
jglick authored Jan 30, 2024
2 parents 5337e0c + c52e84d commit 63864b7
Show file tree
Hide file tree
Showing 5 changed files with 293 additions and 158 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,10 @@ protected Executor tryResolve() throws Exception {
if (task instanceof ExecutorStepExecution.PlaceholderTask) {
ExecutorStepExecution.PlaceholderTask placeholder = (ExecutorStepExecution.PlaceholderTask)task;

// Non-null cookie means body has begun execution, i.e. we started using a node
// PlaceholderTask#getAssignedLabel is set to the Node name when execution starts
// Thus we're guaranteeing the execution began and the Node is now unknown.
// Theoretically it's safe to simply fail earlier when rehydrating any EphemeralNode... but we're being extra safe.
if (placeholder.getCookie() != null && Jenkins.get().getNode(placeholder.getAssignedLabel().getName()) == null ) {
if (placeholder.hasStarted() && Jenkins.get().getNode(placeholder.getAssignedLabel().getName()) == null ) {

Check warning on line 120 in src/main/java/org/jenkinsci/plugins/workflow/support/pickles/ExecutorPickle.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 120 is not covered by tests
if (System.nanoTime() > endTimeNanos) {
Queue.getInstance().cancel(item);
owner.getListener().getLogger().printf("Killed %s after waiting for %s because we assume unknown agent %s is never going to appear%n",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public final class ExecutorStepDynamicContext implements Serializable {

private static final long serialVersionUID = 1;

@NonNull ExecutorStepExecution.PlaceholderTask task;
final @NonNull ExecutorStepExecution.PlaceholderTask task;
final @NonNull String node;
final @NonNull String path;
final int depth;
Expand Down Expand Up @@ -97,8 +97,6 @@ void resume(StepContext context) throws Exception {
if (item == null) {
throw new IllegalStateException("queue refused " + task);
}
// Try to avoid having distinct a instance of PlaceholderTask here compared to any previously-scheduled task.
task = (ExecutorStepExecution.PlaceholderTask)item.task;
LOGGER.fine(() -> (result.isCreated() ? "scheduled " : " using already-scheduled ") + item + " for " + path + " on " + node);
TaskListener listener = context.get(TaskListener.class);
if (!node.isEmpty()) { // unlikely to be any delay for built-in node anyway
Expand Down
Loading

0 comments on commit 63864b7

Please sign in to comment.