Skip to content

Define termination milestones for suspending & saving #301

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

Merged
jglick merged 1 commit into from
Aug 16, 2023
Merged

Define termination milestones for suspending & saving #301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions src/main/java/org/jenkinsci/plugins/workflow/flow/FlowExecution.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,10 @@ public Iterable<BlockStartNode> iterateEnclosingBlocks(@NonNull FlowNode node) {
return getInternalGraphLookup().iterateEnclosingBlocks(node);
}

/** Perform shutdown-specific logic -- should be invoked by the {@link FlowExecutionOwner#notifyShutdown()} method
* in response to {@link FlowExecutionList#saveAll()} */
/**
* @deprecated No longer used.
*/
@Deprecated
protected void notifyShutdown() {
// Default is no-op
}
Expand Down
19 changes: 18 additions & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import hudson.ExtensionList;
import hudson.XmlFile;
import hudson.init.InitMilestone;
import hudson.init.TermMilestone;
import hudson.init.Terminator;
import hudson.model.Computer;
import hudson.model.listeners.ItemListener;
Expand Down Expand Up @@ -38,6 +39,7 @@
import java.util.logging.Logger;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.graphanalysis.LinearBlockHoppingScanner;
import org.jvnet.hudson.reactor.Milestone;

import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.Beta;
Expand All @@ -50,6 +52,19 @@
*/
@Extension
public class FlowExecutionList implements Iterable<FlowExecution> {

/**
* Milestone for {@link Terminator} between {@link TermMilestone#STARTED} and {@link #LIST_SAVED}.
* All running builds have been suspended and their {@link FlowExecutionOwner#getListener}s closed.
*/
public static final String EXECUTIONS_SUSPENDED = "FlowExecutionList.EXECUTIONS_SUSPENDED";

/**
* Milestone for {@link Terminator} between {@link #EXECUTIONS_SUSPENDED} and {@link TermMilestone#COMPLETED}.
* {@link FlowExecutionList} itself has been saved.
*/
public static final String LIST_SAVED = "FlowExecutionList.LIST_SAVED";

private final CopyOnWriteList<FlowExecutionOwner> runningTasks = new CopyOnWriteList<>();
private final SingleLaneExecutorService executor = new SingleLaneExecutorService(Timer.get());
private XmlFile configFile;
Expand Down Expand Up @@ -238,7 +253,9 @@ public void onFailure(@NonNull Throwable t) {
}

@Restricted(DoNotUse.class)
@Terminator public static void saveAll() throws InterruptedException {
@SuppressWarnings("deprecation")
@Terminator(requires = EXECUTIONS_SUSPENDED, attains = LIST_SAVED)
Copy link
Member

@dwnusbaum dwnusbaum Aug 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I just thought about: if you have workflow-api installed but not workflow-cps, is the reactor smart enough to see that nothing attains EXECUTIONS_SUSPENDED and still allow this to run? Or does it block (bad), or get skipped (fine since you won't have any executions anyway)?

Copy link
Member Author

@jglick jglick Aug 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the reactor smart enough to see that nothing attains EXECUTIONS_SUSPENDED and still allow this to run?

I wondered the same, but actually you can see from the fact that tests in this plugin pass that it is OK: it just considers EXECUTIONS_SUSPENDED to have attained immediately.

Copy link
Member

@dwnusbaum dwnusbaum Aug 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually you can see from the fact that tests in this plugin pass that it is OK

Yeah I suspected that was the case, but wanted to make sure.

Copy link
Member Author

@jglick jglick Aug 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Not the same scenario as you mention, but similar, since the workflow-cps in test scope of course predates the code that would attain this milestone.)

Copy link
Member Author

@jglick jglick Aug 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example:

diff --git src/test/java/org/jenkinsci/plugins/workflow/actions/ErrorActionTest.java src/test/java/org/jenkinsci/plugins/workflow/actions/ErrorActionTest.java
index 53d4dd1..9d99c10 100644
--- src/test/java/org/jenkinsci/plugins/workflow/actions/ErrorActionTest.java
+++ src/test/java/org/jenkinsci/plugins/workflow/actions/ErrorActionTest.java
@@ -79,6 +79,7 @@ public class ErrorActionTest {
     @Test
     public void simpleException() throws Throwable {
         rr.then(r -> {
+            System.setProperty("jenkins.model.Jenkins.termLogLevel", "INFO");
             final String EXPECTED = "For testing purpose";
             WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "p");
             job.setDefinition(new CpsFlowDefinition(String.format(

⇒

INFO	hudson.lifecycle.Lifecycle#onStatusUpdate: Stopping Jenkins
INFO	jenkins.model.Jenkins$16#onAttained: Attained FlowExecutionList.EXECUTIONS_SUSPENDED
INFO	jenkins.model.Jenkins$16#onAttained: Attained Started termination
INFO	jenkins.model.Jenkins$16#onTaskStarted: Started SCMEvent.closeExecutorService
INFO	jenkins.model.Jenkins$16#onTaskCompleted: Completed SCMEvent.closeExecutorService
INFO	jenkins.model.Jenkins$16#onTaskStarted: Started FlowExecutionList.saveAll
INFO	jenkins.model.Jenkins$16#onTaskCompleted: Completed FlowExecutionList.saveAll
INFO	jenkins.model.Jenkins$16#onAttained: Attained FlowExecutionList.LIST_SAVED
INFO	jenkins.model.Jenkins$16#onTaskStarted: Started NioChannelSelector.cleanUp
INFO	jenkins.model.Jenkins$16#onTaskCompleted: Completed NioChannelSelector.cleanUp
INFO	jenkins.model.Jenkins$16#onTaskStarted: Started CpsFlowExecution.suspendAll
INFO	jenkins.model.Jenkins$16#onTaskCompleted: Completed CpsFlowExecution.suspendAll
INFO	jenkins.model.Jenkins$16#onTaskStarted: Started DurableTaskStep.shutDownThreadPool
INFO	jenkins.model.Jenkins$16#onTaskCompleted: Completed DurableTaskStep.shutDownThreadPool
INFO	jenkins.model.Jenkins$16#onTaskStarted: Started IOHubProvider.cleanUp
INFO	jenkins.model.Jenkins$16#onTaskCompleted: Completed IOHubProvider.cleanUp
INFO	jenkins.model.Jenkins$16#onAttained: Attained Completed termination
INFO	hudson.lifecycle.Lifecycle#onStatusUpdate: Jenkins stopped

public static void saveAll() throws InterruptedException {
LOGGER.fine("ensuring all executions are saved");

for (FlowExecutionOwner owner : get().runningTasks.getView()) {
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionOwner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ public abstract class FlowExecutionOwner implements Serializable {
@NonNull
public abstract FlowExecution get() throws IOException;

/** Invoked in {@link FlowExecutionList#saveAll()} to notify that execution has been suspended */
/**
* @deprecated No longer used.
*/
@Deprecated
void notifyShutdown() {
FlowExecution exec = getOrNull();
if (exec != null) {
Expand Down