Unification of restart logic between decisions and steps [BATCH-2123]

Piotr Dyraga opened BATCH-2123 and commented

Currently, when failed job is being recovered, steps that were already executed and finished with a status of COMPLETED or ABANDONED are skipped.

Snippet from org.springframework.batch.core.job.SimpleStepHandler:

if ((stepStatus == BatchStatus.COMPLETED && step.isAllowStartIfComplete() == false)
        || stepStatus == BatchStatus.ABANDONED) {
    // step is complete, false should be returned, indicating that the
    // step should not be started
    logger.info("Step already complete or not restartable, so no action to execute: " + lastStepExecution);
    return false;
}

This behaviour does not apply to decisions - all JobExecutionDeciders are currently executed once again. It may lead to some unpredictable job execution results if decision is taken basis on data modified somehow by the next steps (it's not idempotent).

There are two possible solutions for that:

1. Allow developer to set allow-start-if-complete on decisions just like on steps
2. Do not execute decision once again if at least one step after it has been COMPLETED / ABANDONED

Here is some real-world scenario:
Flat file is being imported. In order to avoid duplicates, file checksum is being computed and stored in database. If file with the same checksum was already imported before, file is rejected as a duplicate. Example job implementation may look as follows:

<!--
  Check if file is a duplicate:
  - computes file checksum
  - checks in database if file with the given checksum has been imported before
-->
<batch:decision id="duplicateCheckDecision" decider="duplicateCheckDecider">
  <batch:next on="NOT_A_DUPLICATE" to="loadFileStep" />
  <batch:next on="IS_A_DUPLICATE" to="rejectFileStep" />
</batch:decision>  
 
<!--
  Loads items from file into the database.
  Moreover, listener saves file checksum into some DB table (duplicates prevention).
-->
<batch:step id="loadFileStep">
  <batch:tasklet>
    <batch:chunk reader="itemReader"  writer="itemWriter" commit-interval="10" />      
  </batch:tasklet>
  <batch:listeners>
    <batch:listener ref="storeFileChecksumInDBListener" />
  </batch:listeners>
  <batch:end on="COMPLETED" />
  <batch:fail on="FAILED" />
</batch:step>
   
<!--
  Mark file as rejected (it's a duplicate)
-->
<batch:step id="rejectFileStep" >   
  <batch:tasklet ref="rejectFileTasklet" />     
  <batch:end on="COMPLETED" />
  <batch:fail on="FAILED" />
</batch:step>

When the given file is imported for the first time, "duplicateFileCheck" decision cannot find its checksum in database and returns "NOT_A_DUPLICATE" - this way the next step to be executed is "loadFileStep". Now, let's assume that something goes wrong in the middle of "loadFileStep" execution and it is marked as FAILED. Because failure occurred in the middle of its execution, some items have been already loaded into the database (commit-interval="10").

If we try to restart this job later, "duplicateFileCheck" decision is invoked once again and this time it decides that file "IS_A_DUPLICATE" (file checksum was already registered by "loadFileStep" listener). This time, job flow goes to "rejectFileStep" and file is rejected as a duplicate. This leads to data inconsistency - some items have been loaded into database (during the first job run) and at the same time, some others were not loaded because file has been marked as a duplicate (during the second job run).

Of course, this is just a simple example that can be possibly redesigned somehow to alleviate the pain. The general issue however still remains - if decision is not idempotent it should not be executed once again.

Affects: 2.2.1

Start with org.springframework.batch.core.job.SimpleStepHandler and trace how restart decisions invoke JobExecutionDeciders; compare the existing COMPLETED/ABANDONED step rules with decision handling. The issue does not name tests or settle the two proposed behaviors, so first establish the chosen restart contract, then verify that a restarted job preserves the intended decision path without re-running it when the contract says to skip it.