Skip to content

Conversation

@zhfeng
Copy link
Contributor

@zhfeng zhfeng commented Aug 10, 2022

…saction if possible

@github-actions
Copy link
Contributor

⚠️ This PR changes Camel components and will be tested automatically.

@github-actions
Copy link
Contributor

❌ Finished component verification: 1 component(s) test failed out of 1 component(s) tested

@github-actions
Copy link
Contributor

✔️ Finished component verification: 0 component(s) test failed out of 1 component(s) tested

@zhfeng zhfeng marked this pull request as draft August 10, 2022 10:58
}

// then try to reuse one from the current transaction if possible
if (em == null && TransactionSynchronizationManager.hasResource(entityManagerFactory)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not good to reuse the one from JpaTransactionManager. I just wonder if there is any way to reuse one in current transaction

if (em == null && exchange != null && exhange.isTransacted()) {
    // get em from the transaction context in camel?
}

@davsclaus any thoughts here?

Copy link
Contributor

@davsclaus davsclaus Aug 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the splitter creates sub exchanges it copies from the parent.

To reuse the same transaction manager, then we could

  1. Add a constant for this in Exchange (so its in core)
  2. Copy over the existing TM in the sub exchange if its transacted (in the splitter) (see link below)
  3. There are other EIPs that should do as 2 (Multicast, Recipient List, Enricher) but not wiretap

newExchange.adapt(ExtendedExchange.class).setTransacted(original.isTransacted());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @davsclaus - I'm thinking to share an EntityManager in the sub exchanges during the transaction. Can you take a review about the changes in Splitter.java? If it's OK, I will apply the same change to other EIPs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I added some comments see below

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I add the similar changes to multicast and recipientList processors. Aslo an unit test has been added to test with a pooling data source. For enrich, the default aggreate stragtegy is using the new exchange, so it works with transacted. But if the custom aggregate stragtegy is used, the users have to copy JpaConstants.ENTITY_MANAGE propery from the newExchange by themseleves. I also provide JpaHelper.copyEntityManagers method to do this copying.

@github-actions
Copy link
Contributor

✔️ Finished component verification: 0 component(s) test failed out of 2 component(s) tested

@github-actions
Copy link
Contributor

✔️ Finished component verification: 0 component(s) test failed out of 1 component(s) tested

@zhfeng zhfeng marked this pull request as ready for review August 16, 2022 03:50
@github-actions
Copy link
Contributor

❌ Finished component verification: 1 component(s) test failed out of 1 component(s) tested

@github-actions
Copy link
Contributor

✔️ Finished component verification: 0 component(s) test failed out of 1 component(s) tested

@zhfeng zhfeng merged commit fe2d88e into apache:main Aug 16, 2022
zhfeng added a commit that referenced this pull request Aug 17, 2022
#8141)

* CAMEL-18377: camel-jpa - reuse an EntityManager in the current transaction if possible

* Fix to create txData only if the exchange is transacted

* Add a unit test with pooling dataSource for transactd() combined with split()

* Fix in multicast

* Fix in RecipientList

* Add a Enrich test

* Fix CS
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.

2 participants