You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2022/08/10 07:17:10 UTC
[GitHub] [camel] zhfeng opened a new pull request, #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
zhfeng opened a new pull request, #8141:
URL: https://github.com/apache/camel/pull/8141
…saction if possible
<!-- Uncomment and fill this section if your PR is not trivial
- [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] If you're unsure, you can format the pull request title like `[CAMEL-XXX] Fixes bug in camel-file component`, where you replace `CAMEL-XXX` with the appropriate JIRA issue.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Run `mvn clean install -Psourcecheck` in your module with source check enabled to make sure basic checks pass and there are no checkstyle violations. A more thorough check will be performed on your pull request automatically.
Below are the contribution guidelines:
https://github.com/apache/camel/blob/main/CONTRIBUTING.md
-->
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on a diff in pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on code in PR #8141:
URL: https://github.com/apache/camel/pull/8141#discussion_r943215986
##########
core/camel-core-processor/src/main/java/org/apache/camel/processor/Splitter.java:
##########
@@ -203,6 +205,8 @@ public Iterator<ProcessorExchangePair> iterator() {
private int index;
private boolean closed;
+ private final Map<String, Object> data = new ConcurrentHashMap<>();
Review Comment:
You should only create this map if its transacted (otherwise its waste of cpu for regular non transacted). And the name data should be txData to better indicate its for transaction
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #8141:
URL: https://github.com/apache/camel/pull/8141#issuecomment-1216138371
:x: Finished component verification: **1 component(s) test failed** out of 1 component(s) tested
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #8141:
URL: https://github.com/apache/camel/pull/8141#issuecomment-1216087752
:heavy_check_mark: Finished component verification: 0 component(s) test failed out of **1 component(s) tested**
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] zhfeng commented on a diff in pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #8141:
URL: https://github.com/apache/camel/pull/8141#discussion_r943184276
##########
components/camel-jpa/src/main/java/org/apache/camel/component/jpa/JpaHelper.java:
##########
@@ -60,6 +62,15 @@ public static EntityManager getTargetEntityManager(
em = getEntityManagerMap(exchange).get(getKey(entityManagerFactory));
}
+ // then try to reuse one from the current transaction if possible
+ if (em == null && TransactionSynchronizationManager.hasResource(entityManagerFactory)) {
Review Comment:
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.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on a diff in pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on code in PR #8141:
URL: https://github.com/apache/camel/pull/8141#discussion_r943125337
##########
components/camel-jpa/src/main/java/org/apache/camel/component/jpa/JpaHelper.java:
##########
@@ -60,6 +62,15 @@ public static EntityManager getTargetEntityManager(
em = getEntityManagerMap(exchange).get(getKey(entityManagerFactory));
}
+ // then try to reuse one from the current transaction if possible
+ if (em == null && TransactionSynchronizationManager.hasResource(entityManagerFactory)) {
Review Comment:
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 #2 (Multicast, Recipient List, Enricher) but not wiretap
https://github.com/apache/camel/blob/991f9ca7898bfea362821ef115d930fece317230/core/camel-core-processor/src/main/java/org/apache/camel/processor/Splitter.java#L231
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] zhfeng commented on a diff in pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #8141:
URL: https://github.com/apache/camel/pull/8141#discussion_r943104211
##########
components/camel-jpa/src/main/java/org/apache/camel/component/jpa/JpaHelper.java:
##########
@@ -60,6 +62,15 @@ public static EntityManager getTargetEntityManager(
em = getEntityManagerMap(exchange).get(getKey(entityManagerFactory));
}
+ // then try to reuse one from the current transaction if possible
+ if (em == null && TransactionSynchronizationManager.hasResource(entityManagerFactory)) {
Review Comment:
It's not good to reuse the one from JpaTransactionManager. I just wonder if there is any way to reuse one in current transaction
```java
if (em == null && exchange != null && exhange.isTransacted()) {
// get em from the transaction context in camel?
}
```
@davsclaus any thoughts here?
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #8141:
URL: https://github.com/apache/camel/pull/8141#issuecomment-1211704056
:heavy_check_mark: Finished component verification: 0 component(s) test failed out of **2 component(s) tested**
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] zhfeng merged pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
zhfeng merged PR #8141:
URL: https://github.com/apache/camel/pull/8141
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on a diff in pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on code in PR #8141:
URL: https://github.com/apache/camel/pull/8141#discussion_r943125337
##########
components/camel-jpa/src/main/java/org/apache/camel/component/jpa/JpaHelper.java:
##########
@@ -60,6 +62,15 @@ public static EntityManager getTargetEntityManager(
em = getEntityManagerMap(exchange).get(getKey(entityManagerFactory));
}
+ // then try to reuse one from the current transaction if possible
+ if (em == null && TransactionSynchronizationManager.hasResource(entityManagerFactory)) {
Review Comment:
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
https://github.com/apache/camel/blob/991f9ca7898bfea362821ef115d930fece317230/core/camel-core-processor/src/main/java/org/apache/camel/processor/Splitter.java#L231
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] zhfeng commented on a diff in pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #8141:
URL: https://github.com/apache/camel/pull/8141#discussion_r943311105
##########
core/camel-core-processor/src/main/java/org/apache/camel/processor/Splitter.java:
##########
@@ -203,6 +205,8 @@ public Iterator<ProcessorExchangePair> iterator() {
private int index;
private boolean closed;
+ private final Map<String, Object> data = new ConcurrentHashMap<>();
Review Comment:
yeah, I will update it.
##########
core/camel-api/src/main/java/org/apache/camel/Exchange.java:
##########
@@ -254,6 +254,7 @@ public interface Exchange {
String TRACING_HEADER_FORMAT = "CamelTracingHeaderFormat";
@Deprecated
String TRACING_OUTPUT_FORMAT = "CamelTracingOutputFormat";
+ String TRANSACTION_CONTEXT_DATA = "CamelTransactionContextData";
Review Comment:
Currently it only needs to store an EntityManager but I'd like to keept it more generic since it could be used for sharing any data in sub exchanges during the transaction.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] zhfeng commented on a diff in pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #8141:
URL: https://github.com/apache/camel/pull/8141#discussion_r946313974
##########
components/camel-jpa/src/main/java/org/apache/camel/component/jpa/JpaHelper.java:
##########
@@ -60,6 +62,15 @@ public static EntityManager getTargetEntityManager(
em = getEntityManagerMap(exchange).get(getKey(entityManagerFactory));
}
+ // then try to reuse one from the current transaction if possible
+ if (em == null && TransactionSynchronizationManager.hasResource(entityManagerFactory)) {
Review Comment:
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.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #8141:
URL: https://github.com/apache/camel/pull/8141#issuecomment-1216140637
:heavy_check_mark: Finished component verification: 0 component(s) test failed out of **1 component(s) tested**
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on a diff in pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on code in PR #8141:
URL: https://github.com/apache/camel/pull/8141#discussion_r943217481
##########
core/camel-api/src/main/java/org/apache/camel/Exchange.java:
##########
@@ -254,6 +254,7 @@ public interface Exchange {
String TRACING_HEADER_FORMAT = "CamelTracingHeaderFormat";
@Deprecated
String TRACING_OUTPUT_FORMAT = "CamelTracingOutputFormat";
+ String TRANSACTION_CONTEXT_DATA = "CamelTransactionContextData";
Review Comment:
its a bit generic this name? Do you need to store something else than the EM factory?
TRANSACTION_MANAGER
or
TRANSACTION_ENTITY_MANAGER
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on a diff in pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on code in PR #8141:
URL: https://github.com/apache/camel/pull/8141#discussion_r943297064
##########
components/camel-jpa/src/main/java/org/apache/camel/component/jpa/JpaHelper.java:
##########
@@ -60,6 +62,15 @@ public static EntityManager getTargetEntityManager(
em = getEntityManagerMap(exchange).get(getKey(entityManagerFactory));
}
+ // then try to reuse one from the current transaction if possible
+ if (em == null && TransactionSynchronizationManager.hasResource(entityManagerFactory)) {
Review Comment:
Yes I added some comments see below
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #8141:
URL: https://github.com/apache/camel/pull/8141#issuecomment-1210261932
:warning: This PR changes Camel components and will be tested automatically.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #8141:
URL: https://github.com/apache/camel/pull/8141#issuecomment-1210306656
:x: Finished component verification: **1 component(s) test failed** out of 1 component(s) tested
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #8141: CAMEL-18377: camel-jpa - resue an EntityManager from the current tran…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #8141:
URL: https://github.com/apache/camel/pull/8141#issuecomment-1210396000
:heavy_check_mark: Finished component verification: 0 component(s) test failed out of **1 component(s) tested**
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org