Bug 4284 - Do we need skip/retry or at least callback w/ exception for error on chunk commit?
Do we need skip/retry or at least callback w/ exception for error on chunk co...
Status: RESOLVED FIXED
Product: jbatch
Classification: Unclassified
Component: source
1
PC All
: P5 normal
: ---
Assigned To: cvignola
: 4548 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-09 21:45 UTC by ScottKurz
Modified: 2013-02-05 22:18 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description ScottKurz 2012-11-09 21:45:58 UTC
This just restates my email on the subject.

To summarize:  I think for JPA, writes typically will get flushed on commit, not writeItems(), upon which the user might want to progammatically react to an exception involving his application code, like he would via a skip listener for an error on writeItems().

Should we extend skip/retry to  this case?
Comment 1 cvignola 2012-11-19 11:45:09 UTC
Flush is the call to ItemWriter.  You can explicitly control flush and commit behavior as follows:

checkpoint-algorithm=item
commit-interval=10
buffer-size=10

That flushes after 10 items and commits.  However, it is valid to flush multiple times before a commit - think of JDBC batching, for instance.  

Skip/retry is available after read, process, and write,  so I think we're covered.
Comment 2 ScottKurz 2013-01-03 16:55:53 UTC
Chris,

I don't think I made my point clear...

So if I have:

 checkpoint-algorithm=item
 commit-interval=10
 buffer-size=10

then yes I am flushing and committing at the same time.   That's not the problem.

The problem is that writeItems() might not really do anything because of some buffering downstream... e.g. I used the example of a JPA scenario where write might update some Java fields but the database doesn't get touched until the tran commits.

It is only on tran commit that the database throws an exception that I wish I could recover from (skip/retry), but I can't...
Comment 3 cvignola 2013-01-04 19:33:11 UTC
Ok, now I see your point. The declarative retry specification in a job definition currently applies to exceptions thrown by only reader, processor, and writer artifacts.  We should consider having it apply to exceptions thrown by exception that occurs during step processing. If the exception happens to be thrown by reader, processor, or writer,  there is a matching retry listener that the developer can supply. All other exceptions should probably go through a separate, "retry step" listener.  I also see a need for a point of control after rollback and before starting a new tran to allow for custom processing - e.g. inserting a retry delay.  I further see a need for a callback that receives control when retry is abandoned - e.g. retry limit is up.  The purpose of such a callback would be for things like compensation logic.
Comment 4 cvignola 2013-01-14 13:49:36 UTC
onException methods were added to StepListener and JobListener to cover this.
Comment 5 ScottKurz 2013-01-25 03:55:53 UTC
Chris,  for the new Job/StepListener.onException() methods... 

1. Do we intend that they be called on any exception?   Or only on exceptions that are not thrown on read/process/write?   Or.. for read/process/write exceptions.. only those that are not caught/handled by skip/retry listeners?

Any ordering required to be specified if multiple listeners are involved?  Otherwise this is undefined which may be OK...

2. It seems you decided for them not to participate in skip/retry.   I know there are some details to pin down and it wouldn't be trivial to do so, but that was my original question/suggestion.. so I wanted to make sure that you didn't intend but forget to include this.
Comment 6 ScottKurz 2013-01-25 05:40:52 UTC
I think I'm seeing now where this wording is found.. you can probably ignore this one now.. I imagine the XSD at least still needs an update if you want to keep this open to remind us.
Comment 7 cvignola 2013-02-04 18:32:28 UTC
The spec introduced StepListener.onException and JobListener.onException as an initial response to this bug.  Those callbacks are overly broad in scope. We need something like ChunkListener.onCommitError.  

I propose we remove StepListener.onException and JobListener.onException and add ChunkListener.onCommitError.  

It was intended all the while that Skip and exclude classes should apply also to chunk commit.  That didn't make it into the last spec update.  It will be PFD v1.3.
Comment 8 cvignola 2013-02-04 23:26:24 UTC
*** Bug 4548 has been marked as a duplicate of this bug. ***
Comment 9 cvignola 2013-02-04 23:27:57 UTC
The method name will be onError and it will be invoked for any failure in chunk processing that results in the chunk transaction getting rolled back.
Comment 10 mminella 2013-02-05 22:18:50 UTC
I had a question about this one.  For the onError, how are we providing the method with some form information on what the error was?  In Spring Batch we provide the ChunkContext (which contains the exception that caused the error) in the ChunkListener#afterChunkError().