|
Looking at JobExecution.status is probably the best way we have to do this - it is a framework enum, and really it was designed for exactly this kind of use case. So I am OK with that part - using BatchStatus as a message to reduce coupling is what it amounts to. ConditionalJob (or similar) will need to consume the same kind of message.
I'll probably form an opinion about the create/continuation logic when I start using it. If we decide to address the concerns discussed in the comments somehow I suppose those will be worth creating separate issues, so I'm resolving this one.
Robert,
I was doing a review of this today and I have a couple questions for you. In the SimpleJobLauncher, you implemented pause such that, if the status of the last execution is Paused, you just reuse it. This contrasts with how we handle restart, where we create a new JobExecution. I understand what you were trying to do, since it seems like pausing should allow for resuming. However, you lose information. If you wanted to know what time the execution was paused, or started, you don't really know. I suppose you could look at the last step to finish, and the first one to start, but even then if you resumed fairly quickly, there's no guarantee. If you look at how we restart a step, it's the same thing (Unless I'm mistaken), we create a new StepExecution, the only difference is the new one has the ExecutionContext of the old. I think we should remain consistent and create a new JobExecution in the Resume scenario as well. The second thing is noticed is that you check for Pausing as being analogous to interruption. Meaning, in the checkForInterruption method of SimpleJobRepository, you also check to see if the Execution was set to Pause, and if so, set the StepExecution to terminateOnly, which will cause the job to effectively interrupt. It was my understanding that Pausing meant that a step finished successfully, but the job should end anyway. I thought this was a major distinction between interruption and pause, meaning, pause will wait for the current step to finish and then pause the job, and interruption will stop the step at it's next chunk boundary. Furthermore, because the Job is inspecting a returned Step for a status of PAUSED, and the Step is simply checking for isTerminateOnly and returning as STOPPED, I don't see how it could ever work. In fact, I did a search, and no Step implementation even references BatchStatus.PAUSED. I think the code should be modified to check for pause after a step completes and abending the Job as PAUSED. (also, nulling out the end time in the SimpleLauncher is also a bad idea, IMHO) Lucas: look at https://springframework.svn.sourceforge.net/svnroot/springframework/spring-batch/trunk/src/site/apt/cases/pause.apt. I see the point about the audit trail, but in that case we need another feature because ew need a PartitionHandler to be able to pause / resume if it wants to without creating hundreds of JobExecutions. Also a step should be able to pause if it wants to (although, as you point out, the main use case that we discussed previously has been a pause after a step).
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Sub-task
Resolved
Major
There are at least two issues that make me wonder:
- the logic concerned with JobExecution creation/continuation is now in two places (JobLauncher and JobRepository)
- pausing and interruption work the same way and AbstractJob distinguishes between the two by looking at JobExection's status, which seems a little fishy.