ramu11 commented on PR #27314:
URL: https://github.com/apache/camel/pull/27314#issuecomment-5994599276
> Since Hibernate 8 is still in beta and the runtimes won't adopt it soon,
I've reviewed the PR from a transaction point of view, and there are some
improvements and bug fixes that can be done now. That way, once it's merged,
the component is ready to work with Camel transactions
(`from(...).transacted().to("hibernate:...")`) without renaming options or
changing behaviour users already rely on.
>
> Full transaction joining doesn't need to land in this PR; the description
already defers it. Below is what I measured, then what I'd ask for in this PR.
>
> ### What happens today
> The producer and the consumer always do `openSession()` +
`beginTransaction()` + `commit()` per call, and never look at
`exchange.isTransacted()`. I wrote a probe test (Narayana + Agroal + H2 for
JTA, Spring 7.0.9 for the Spring cases):
https://gist.github.com/Croway/287a26c890854f6f6a54a236e77d67e0. It passes
against the PR head and asserts today's behaviour, so it can be turned into
regression tests as each point below is fixed.
>
> Setup Result after the route fails past the hibernate step
> `SpringTransactionPolicy` + `JpaTransactionManager` over the component's
`SessionFactory` hibernate rows **survive** the rollback
> `SpringTransactionPolicy` + `DataSourceTransactionManager`, JDBC insert
then hibernate insert JDBC row rolled back, hibernate row **committed**
(split writes). A hibernate query in the same route can't see the JDBC row
written earlier in the same transaction
> `JtaTransactionPolicy` (Narayana) + Agroal, component bootstrapped as
documented **every exchange fails**: `TransactionException: Unable to commit
against JDBC Connection` / `Attempting to commit while taking part in a
transaction`. The DataSource is always registered as
`JAKARTA_NON_JTA_DATASOURCE`, so Hibernate commits a connection that Agroal has
enlisted in JTA
> Same, plus `hibernate.transaction.coordinator_class=jta` and
`hibernate.transaction.jta.platform=Narayana` **correct**: rollback removes the
rows; on success nothing is visible outside the route until the JTA commit
> The last row works only because of how Hibernate itself behaves. With a
JTA coordinator, `Transaction.begin()` skips the begin when a JTA transaction
is already active, and `commit()`/`rollback()` only commit, or mark
rollback-only, when Hibernate did not start the transaction. Nothing in the
component documents or tests this.
>
> ### What to settle in this PR
> 1. **Move session and transaction handling into one place.** Put session
acquisition and begin/commit/rollback behind a small strategy (like
`TransactionStrategy` in camel-jpa), with today's resource-local behaviour as
the default. Producer, consumer and the stateless path then call it instead of
`beginTransaction()`/`commit()` directly, and JTA and Spring become extra
implementations later, with no change to the endpoint options.
> 2. **Don't break JTA out of the box.** The default bootstrap fails on any
DataSource enlisted in JTA, which is the default on Quarkus. Either:
>
> * add a `transactionType` component option now (`RESOURCE_LOCAL`
default, `JTA`). With `JTA` it registers the DataSource as
`JAKARTA_JTA_DATASOURCE` and sets the coordinator, plus the platform when one
is given. The option name is then fixed from the first release; or
> * at least don't force `JAKARTA_NON_JTA_DATASOURCE` when
`hibernateProperties` set a JTA coordinator.
> 3. **Fail fast instead of splitting writes.** Until Spring joining exists,
a `transacted()` exchange that reaches the producer with a non-JTA
`SessionFactory` should throw a clear error rather than quietly commit on its
own. You can detect this with
`sessionFactory.unwrap(SessionFactoryImplementor.class).getServiceRegistry().requireService(TransactionCoordinatorBuilder.class).isJta()`.
Allowing it later is not a breaking change; silently committing now would be
one to change later.
> 4. **Don't commit or roll back a transaction the component didn't start.**
When the coordinator is JTA, use `sessionFactory.getCurrentSession()` (JTA
session context) when a transaction is active, and leave the outcome to the
`TransactedPolicy`. Store the session in `Exchange.TRANSACTION_CONTEXT_DATA`
(Splitter, Multicast and Aggregate already pass it to sub-exchanges) so several
hibernate steps in one exchange share a session. Close it on exchange
completion, not in a `finally` around each call.
> 5. **Reject `streaming=true` inside a transacted exchange.** The stream's
session and transaction would outlive the route's transaction.
> 6. **Consumer.** The poll currently runs the whole downstream route inside
the consumer's own private transaction. So rows locked with `SKIP_LOCKED`, and
a later `.transacted()`, commit independently of each other. Run the poll
through the strategy from point 1 so that later, a downstream `transacted()`
(propagation REQUIRED) joins the same transaction, as `JpaConsumer` does.
> 7. **Spring support optional, not a hard dependency.** Later, a Spring
implementation of the strategy can join
`JpaTransactionManager`/`HibernateTransactionManager`
(`EntityManagerFactoryUtils.getTransactionalEntityManager(sf).unwrap(Session.class)`)
and `DataSourceTransactionManager`
(`sf.withOptions().connection(DataSourceUtils.getConnection(ds))`, and
`withStatelessOptions()` for insert/upsert). Keeping it behind the interface
from point 1 with an optional `spring-orm` keeps the native path free of
Spring, as requested earlier in this review.
> 8. **Tests.** Add a JTA test with Narayana + Agroal (same test
dependencies as `camel-jta`) covering commit, rollback, and the "not visible
before commit" case. Add a test that pins down the behaviour of a transacted
exchange with a non-JTA `SessionFactory` (fail fast, per point 3).
> 9. **Docs.** Add a _Transactions_ section to `hibernate-component.adoc`
stating today's behaviour: each operation commits on its own unless the
`SessionFactory` uses JTA. Then the intended setup per runtime:
>
> * **Camel / Camel Spring Boot**: Spring transaction managers and
Narayana JTA.
> * **Camel Quarkus**: Narayana JTA only, XA and non-XA. There the
`SessionFactory` would normally come from Quarkus's Hibernate ORM extension
through the registry, which is already JTA-managed.
> 10. **Hibernate version.** On Camel Spring Boot and Camel Quarkus, the
`SessionFactory` that joins transactions is normally the one the runtime
builds. Until they move to Hibernate 8, the component can't reuse it, so keep
the registry lookup and the component's own bootstrap independent of each other.
>
> _Claude Code on behalf of Croway. This comment was generated by an AI
agent and may contain inaccuracies. Please verify before applying._
You can reply with:
> Thanks for the detailed transaction review. For this PR, we intentionally
keep full Camel/Spring/JTA transaction joining deferred, as noted in the PR
description.
>
> The immediate transaction-related issues that are in scope have been
addressed:
>
> * Component-owned resource-local transactions remain the default behavior.
> * Explicit JTA datasource configuration is preserved during native
bootstrap.
> * Consumer Sessions are propagated to downstream Hibernate producers, so
`skipLocked` routes no longer open a second Session and self-deadlock.
> * Producer reuses the consumer transaction and does not commit/close a
transaction it did not start.
> * Stateless operations correctly honor `tenantIdentifier`.
> * Regression coverage now passes **30/30 tests**.
>
> Full transaction strategy/propagation, Spring/JTA integration, and
associated Narayana integration tests are intentionally deferred to the
transaction-integration work rather than expanding the scope of this Hibernate
8 migration PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
