Skip to content

Set local task exit status when time limit is exceeded #6592

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+33 −0
Open

Set local task exit status when time limit is exceeded #6592

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
1 change: 1 addition & 0 deletions modules/nextflow/src/main/groovy/nextflow/executor/local/LocalTaskHandler.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ class LocalTaskHandler extends TaskHandler implements FusionAwareTask {
*/
if( elapsedTimeMillis() > wallTimeMillis ) {
destroy()
task.exitStatus = process.exitValue()
Copy link
Member

@pditommaso pditommaso Nov 20, 2025

Choose a reason for hiding this comment

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

Would not make sense to use a predictable exit status (we event is known). Otherwise it can entirely depend on "destroy" implementation, and I'm not even sure that's going to a deterministic value.

Copy link
Member Author

@bentsherman bentsherman Nov 20, 2025

Choose a reason for hiding this comment

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

It's an interesting question. I think you could argue either way -- to make it consistent at Nextflow level, or make it consistent with the underlying OS, which the JVM implementation is likely to imitate

I figure that if a user has a pipeline with a mix of e.g. local and SLURM jobs, they would prefer to use the same error codes for both. That's why I lean towards deferring to the JVM

When I tested locally, it returned 143 which corresponds to SIGTERM. This is probably what most implementations do. Some might use 137 (SIGKILL) but that seems less likely since it's more aggressive

task.stdout = outputFile
task.stderr = errorFile
task.error = new ProcessException("Process exceeded running time limit (${task.config.getTime()})")
Expand Down
32 changes: 32 additions & 0 deletions modules/nextflow/src/test/groovy/nextflow/executor/local/LocalTaskHandlerTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@ import java.nio.file.Path

import nextflow.Global
import nextflow.container.DockerConfig
import nextflow.exception.ProcessException
import nextflow.file.http.XPath
import nextflow.processor.TaskBean
import nextflow.processor.TaskConfig
import nextflow.processor.TaskRun
import nextflow.processor.TaskStatus
import nextflow.util.Duration
import spock.lang.Specification
/**
*
Expand Down Expand Up @@ -83,4 +86,33 @@ class LocalTaskHandlerTest extends Specification {
cleanup:
builder?.redirectOutput()?.file()?.delete()
}

def 'should kill task when task exceeds time limit' () {
given:
def workDir = Path.of('/tmp/test-work-dir')
def task = Mock(TaskRun) {
getWorkDir() >> workDir
getConfig() >> Mock(TaskConfig) {
getTime() >> Duration.of(100)
}
}
and:
def handler = Spy(new LocalTaskHandler(task, Mock(LocalExecutor))) {
buildTaskWrapper() >> {}
elapsedTimeMillis() >> 200
}
handler.@process = Mock(Process) {
exitValue() >> 143 // Typical exit code for SIGTERM
}
handler.status = TaskStatus.RUNNING

when:
def completed = handler.checkIfCompleted()

then:
completed == true
1 * task.setExitStatus(143)
1 * task.setError(_ as ProcessException)
handler.status == TaskStatus.COMPLETED
}
}
Loading