stop() does not prevent upcoming steps to be executed anymore

[andre-bugay]

It seems like Spring Batch 6 cannot stop a Job anymore.
After calling stop(), all steps are executed and later the job is marked as FAILED.

In Spring Batch 5 the flow was:
STARTED -> STOPPING -> mark step executions as terminateOnly -> STOPPED

In Spring Batch 6 it is:
STARTED -> STOPPING -> STOPPED -> FAILED

If I am right then the root cause of this change is the following new line in

spring-batch/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobOperator.java

Line 348 in c8a0528

jobExecution.setEndTime(LocalDateTime.now());

Directly afterwards the
jobRepository.update(jobExecution);
checks

spring-batch/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java

Line 139 in c8a0528

if (jobExecution.getStatus() == BatchStatus.STOPPING && jobExecution.getEndTime() != null) {

This will always be false as the endTime was set just before.
The jobState will be set from STOPPING to STOPPED directly.

Consequence
Inside SimpleJobRepository#update(StepExecution) -> checkForInterruption(stepExecution) the check in

spring-batch/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java

Lines 186 to 188 in c8a0528

	if (jobExecution.isStopping()) {
	logger.info("Parent JobExecution is stopped, so passing message on to StepExecution");
	stepExecution.setTerminateOnly();

will never be true and the steps are not marked for terminateOnly.

Is this intended and how can I prevent running the unstarted steps once the job is stopped?

[KILL9-NO-MERCY]

Hi, I also have reviewed the root cause of this issue. I'd like to share my findings for your reference.

As you pointed out, the commit e5fbc2a introduced:

jobExecution.setEndTime(LocalDateTime.now());

This change causes the following logic within SimpleJobRepository to execute:

if (jobExecution.getStatus() == BatchStatus.STOPPING && jobExecution.getEndTime() != null) {
    if (logger.isInfoEnabled()) {
       logger.info("Upgrading job execution status from STOPPING to STOPPED since it has already ended.");
    }
    jobExecution.upgradeStatus(BatchStatus.STOPPED);
}

Looking at the history, this seems to be an intentional change (though I am uncertain of the exact reasoning for setting the status to STOPPED at this specific point). The solution will depend on whether Spring Batch Team decide to retain or revert this code modification.

Scenario 1: Retaining jobExecution.setEndTime(LocalDateTime.now()) (Current Batch 6 Behavior)

If we must keep jobExecution.setEndTime(LocalDateTime.now()), then the following is the issue:

Unless you are creating and using a custom Step implementation, and instead use the provided TaskletStep or the newly added ChunkOrientedStep (in Batch 6), the following logic in SimpleJobOperator at Line #374 is executed:

stoppableStep.stop(stepExecution);
// default void stop(StepExecution stepExecution) {
//     stepExecution.setTerminateOnly();
//     stepExecution.setStatus(BatchStatus.STOPPED);
//     stepExecution.setExitStatus(ExitStatus.STOPPED);
//     stepExecution.setEndTime(LocalDateTime.now());
// }

This sets the StepExecution to terminateOnly, and then at Line #375 of SimpleJobOperator, it is persisted to the database (metadata repository).

The core problem is that the StepExecution object being updated by the SimpleJobOperator.stop() call is not the same object instance currently being used by the actively executing Step thread. Therefore, to make interruption work, logic should be added to the executing Step (in both TaskletStep and ChunkOrientedStep) to fetch the latest status of the StepExecution from the metadata repository before every chunk transaction commit & after the ItemStream.update() call (or at a similar boundary, based on the historical TaskletStep logic).

Scenario 2: Reverting the Code Added in e5fbc2a (Returning to Legacy Behavior)

If Spring Batch Team choose to revert the code added in e5fbc2a, the logic you mentioned will appropriately interrupt the Step. However, only TaskletStep will be correctly interrupted.

If you look at TaskletStep.doExecute(), it calls getJobRepository().update(stepExecution); right before every transaction commit (after Tasklet.execute() completes - around line #464). This update triggers the logic you cited:

private void checkForInterruption(StepExecution stepExecution) {
    JobExecution jobExecution = stepExecution.getJobExecution();
    jobExecutionDao.synchronizeStatus(jobExecution); // <--- Reads the updated JobExecution status from DB
    if (jobExecution.isStopping()) {
       logger.info("Parent JobExecution is stopped, so passing message on to StepExecution");
       stepExecution.setTerminateOnly(); // <--- Sets terminateOnly
    }
}

This allows the running Step to read the latest JobExecution status modified by JobOperator and set terminateOnly.

The issue is that ChunkOrientedStep does not have this same logic. It only calls JobRepository.updateExecutionContext().

Therefore, if Spring Batch Team proceed with Scenario 2, a call to getJobRepository().update(stepExecution); must also be added to the ChunkOrientedStep implementation to ensure proper interruption.

I hope this analysis is helpful for your ongoing work!

[fmbenhassine]

@andre-bugay @KILL9-NO-MERCY Thank you for raising this issue and for taking the time to analyse the root cause! Indeed, stopping a job seems to be broken, even though the graceful shutdown sample was working as expected when I introduced it in d4a7dfd. There seems to be a commit after that that broke the stop feature.. What bothers me is that that sample involves a manual step (sending the interruption signal to the process) which makes it difficult to detect regressions automatically on CI.

I will check that and plan the fix for the upcoming 6.0.1.