Spring Batch 6.0.4 - SkipListener and transactions regression ?

[ctrung]

Hi,

Not too sure about this one.

According to https://docs.spring.io/spring-batch/reference/step/chunk-oriented-processing/intercepting-execution.html#skipListenersAndTransactions :

The SkipListener is always called just before the transaction is committed. This is to ensure that any transactional resources call by the listener are not rolled back by a failure within the ItemWriter.

Since fix 5361 in 6.0.4, a rollback has been added right after skipListener's call :

this.compositeSkipListener.onSkipInWrite(item, exception);
contribution.incrementWriteSkipCount();
contribution.getStepExecution().incrementRollbackCount();
status.setRollbackOnly();   // <------ added in 6.0.4

Thus, updates made in a skipListener (eg. insert in an error log table) are also rollbacked.

In 6.0.3, it used to work out of the box. In 6.0.4, one can mitigate this by forcing a separate transaction with @Transactional(propagation = Propagation.REQUIRES_NEW) on onSkipInWrite(...).

What is the expected behaviour ?

[ctrung]

Here's a demo project :
spring-batch-discussion-5436.zip

Let's wait for @fmbenhassine answer before creating an issue.

[fmbenhassine]

Thank you for raising this! This is a documentation issue. The contract of SkipListener is incomplete and that section of the reference doc is misleading. Here is why: other listeners like ItemWriteListener#onWriteError and ChunkListener#onChunkError, explicitly mention that those methods are executed as part of the transaction driven by the step, which is about to be rolled back. As a result, any transactional operation that is called there should use a separate transaction. There is also a specific example for that use case in Logging Item Processing and Failures, with a similar important note about transactions at the bottom of the section:

if your listener does anything in an onError() method, it must be inside a transaction that is going to be
rolled back. If you need to use a transactional resource, such as a database, inside an onError() method,
consider adding a declarative transaction to that method (see Spring Core Reference Guide for details),
and giving its propagation attribute a value of REQUIRES_NEW.

This is no different with the SkipListener: that listener is also called within each transaction when scanning a chunk, during which the specific transaction of the faulty item will be rolled back. Therefore, one should use a separate transaction in the skip listener (consistent behaviour with other listeners). The problem was not visible in 6.0.3 and earlier as scanned items were not retried in separate transactions that should be rolled back for faulty items. Do you see?

[ctrung]

Thank you for your explanations ! If this is the expected behaviour, then all is fine.

In 6.0.3, one can still use SQL save points by annotating @Transactional with propagation NESTED on the write method to have faulty items rollbacked and successful ones commited. That's what i used and it worked pretty well in my case.