Skip to content

Fix skip policy with JPA ItemWriter in scan mode#5379

Closed
MinChul-Son wants to merge 2 commits into
spring-projects:mainfrom
MinChul-Son:gh-5377
Closed

Fix skip policy with JPA ItemWriter in scan mode#5379
MinChul-Son wants to merge 2 commits into
spring-projects:mainfrom
MinChul-Son:gh-5377

Conversation

@MinChul-Son

Copy link
Copy Markdown
Contributor

Problem

Closes #5377

When using a JPA-based ItemWriter with a skip policy, the step fails with:

Transaction silently rolled back because it has been marked as rollback-only

This only happens with JPA; JDBC-based writers work correctly.

Root Cause

When a chunk write fails and the step enters scan mode, ChunkOrientedStep was processing all pending items in a single transaction. JPA/Hibernate marks the entire transaction as rollback-only after a flush failure. This causes the next item write in the same transaction to throw "Transaction silently rolled back", which is not in the skip list and gets promoted to a NonSkippableWriteException, failing the step.

JDBC does not have this problem because a caught SQL exception does not poison the surrounding transaction.

A secondary issue also existed: when scan() catches a skippable exception and returns normally, doExecute() detects transactionStatus.isRollbackOnly() and returns from the lambda without calling transactionStatus.setRollbackOnly() explicitly. Since JPA sets the transaction as globally rollback-only (via ConnectionHolder) but not locally, AbstractPlatformTransactionManager.commit() calls processRollback(defStatus, unexpected=true) and throws UnexpectedRollbackException.

Changes

ChunkOrientedStep

One item per scan transaction

Replaced ChunkTracker.pendingChunk: Chunk<O> with pendingScanItems: LinkedList<O>. Instead of passing the entire pending chunk to scan() at once, the doExecute() while loop now dequeues one item at a time via pollNextScanItem(), so each scan item runs in its own independent transaction. A JPA rollback-only marker from a failed item no longer affects subsequent items.

Prevent UnexpectedRollbackException

In doExecute(), when a rollback-only state is detected, transactionStatus.setRollbackOnly() is now called explicitly before returning from the lambda. This converts the global rollback-only (set by JPA) to a local one, so AbstractPlatformTransactionManager performs a clean rollback instead of throwing UnexpectedRollbackException.

beforeChunk / afterChunk consistency (concurrent mode)

The concurrent scan path was calling compositeChunkListener.beforeChunk(new Chunk<>()) with an empty chunk. This is now aligned with the sequential path — both pass the actual single-item chunk, and afterChunk is guarded by !status.isRollbackOnly().

Tests (ChunkOrientedStepScanModeIntegrationTests)

Four new test cases are added:

Test What it verifies
testSkipPolicyWithJpaLikeRollbackOnlyBehaviorInSequentialMode Items after a failing item are processed in a fresh transaction (ThreadLocal simulation)
testSkipPolicyWithJpaLikeRollbackOnlyBehaviorInConcurrentMode Same, concurrent mode
testUnexpectedRollbackExceptionPreventedInSequentialScanMode No UnexpectedRollbackException when ConnectionHolder is globally rollback-only (direct ConnectionHolder.setRollbackOnly() simulation of JPA behavior)
testUnexpectedRollbackExceptionPreventedInConcurrentScanMode Same, concurrent mode

The globalRollbackOnlyWriter helper directly calls ConnectionHolder.setRollbackOnly() on failure, which is exactly what JpaTransactionManager/Hibernate does internally, and is the minimal reproduction of the UnexpectedRollbackException path.

Process one item per transaction in scan mode to prevent JPA rollback-only
contamination across items. Also explicitly set local rollback-only in
doExecute() to avoid UnexpectedRollbackException from JpaTransactionManager.

Closes spring-projectsgh-5377

Signed-off-by: Minchul-Son <smc5236@naver.com>
Signed-off-by: Minchul-Son <smc5236@naver.com>
@fmbenhassine

Copy link
Copy Markdown
Contributor

Thank you for the PR! LGTM 👍 Rebased and merged as 05b2484.

I introduced two changes:

  • I noticed that the 3rd modification "Add TransactionStatus parameter to scan()" that you mentioned in Incorrect handling of skip policy with JPA infrastructure #5377 (comment) is not part of this PR, so I amended it.
  • The tests added in this PR simulate the behaviour of JPA while being in an integration test class. In an integration test, we typically do not simulate things, but rather setup the entire stack and test against real implementations. For this reason, I omitted those tests and added a full JPA-based integration in 2e9ca2f.

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect handling of skip policy with JPA infrastructure

2 participants