[#BATCH-339] ItemStream.mark() and reset() should throw a checked exception

Spring Batch

ItemStream.mark() and reset() should throw a checked exception

Created: 05/Feb/08 01:30 PM Updated: 07/Aug/08 10:06 AM

Component/s:

Infrastructure

Affects Version/s:

1.0.0.m4

Fix Version/s:

1.0.0.m5

Time Tracking:

Original Estimate:

0.25d

Remaining Estimate:

0.25d

Time Spent:

Not Specified

Description

« Hide

mark and reset are critical operations for maintaing data integrity, but they may fail (e.g. fileChannel.truncate() throws IOException). For that reason, mark and reset should throw checked exceptions to ensure data integrity is either maintained or a user is properly notified.

All

Comments

Work Log

Change History

FishEye

Builds

Sort Order:

[ Permlink | « Hide ]

Dave Syer added a comment - 07/Feb/08 12:29 PM - edited

I'm not sure I agree that they should throw a checked exception (what would a client do with it?). But I will add a StreamException to the signature.

[ Permlink | « Hide ]

Gregory Kick added a comment - 07/Feb/08 01:30 PM

Well, it seems that the only client is going to be some implementation of StreamManager right? So let's say we're using the SimpleStreamManager. And that gets used by the SimpleStepExecutor in execute(). When transaction.rollback() fails with exception it will eventually bubble up to

catch (RuntimeException e) {

// classify exception so an exit code can be stored.
status = exceptionClassifier.classifyForExitCode(e);
if (e.getCause() instanceof StepInterruptedException) {
updateStatus(stepExecution, BatchStatus.STOPPED);
throw (StepInterruptedException) e.getCause();
}
else {
updateStatus(stepExecution, BatchStatus.FAILED);
throw e;
}

}

the concern is that while FAILED is pretty clear, there's no differentiation between the normal "i failed with a StreamException while i was reading some data so the job failed, but the output is still fine with regards to the transaction" scenario and the "i failed while maintaing the transaction and i now have output that doesn't match my restart data" scenario.

I would think that having the checked exception would force that consideration all the way up the API because the RuntimeException certainly doesn't. Although now that I look at it, it seems that there would need to be an additional BatchStatus as well in order to make that useful...

[ Permlink | « Hide ]

Dave Syer added a comment - 15/Feb/08 02:15 AM

Added (unchecked) exception and special logic in Step and Job to ensure that a step that failed in this nasty corner case cannot be restarted automatically (status=UNKNOWN). Lucas should make sure that these features are all still available in the new Step implementation, so I'm leaving it open and re-assigning to him.

[ Permlink | « Hide ]

Gregory Kick added a comment - 15/Feb/08 09:33 AM

interestingly, this becomes much less of a problem if ~~BATCH-365~~ gets dealt with because it significantly decreases the chance of failure in mark and reset.

[ Permlink | « Hide ]

Lucas Ward added a comment - 26/Feb/08 02:13 PM

The ItemWriter interface now contains specific unchecked exceptions for flush and clear: FlushFailedException and ClearFailedException

The approach is the same as what is currently done for ItemReader#mark() and reset()

I don't think the step is currently handling all of the approaches separately the way that the exceptions imply. I'm going to create an issue against 1.1 to have that reviewed.

[ Permlink | « Hide ]

Dave Syer added a comment - 07/Aug/08 10:06 AM

Assume closed as resolved and released