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 2021/09/13 14:35:15 UTC
[GitHub] [camel] jeremyross opened a new pull request #6085: CAMEL-16877: camel-salesforce: Fix timeouts occurring in synchronous/transacted routes
jeremyross opened a new pull request #6085:
URL: https://github.com/apache/camel/pull/6085
This issue is caused by the fact that the Jetty client uses a single thread to invoke completion listeners, and camel reuses that thread (if in synchronous mode) for subsequent operations before returning from AsyncCallback.done(). Result is a deadlock.
I solved this by quickly passing the Jetty response to another thread, using Jetty's thread pool. Would like some eyeballs on this please.
--
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 change in pull request #6085: CAMEL-16877: camel-salesforce: Fix timeouts occurring in synchronous/transacted routes
Posted by GitBox <gi...@apache.org>.
davsclaus commented on a change in pull request #6085:
URL: https://github.com/apache/camel/pull/6085#discussion_r708836602
##########
File path: components/camel-salesforce/camel-salesforce-component/src/main/java/org/apache/camel/component/salesforce/SalesforceHttpClient.java
##########
@@ -113,6 +113,12 @@ protected void doStart() throws Exception {
super.doStart();
}
+ @Override
+ protected void doStop() throws Exception {
+ workerPool.shutdown();
Review comment:
Use executorServiceManager to shutdown the pool, and also do a if != null check (as camel can stop a service if it failed to start so the pool can potentially be null)
--
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] jeremyross commented on pull request #6085: CAMEL-16877: camel-salesforce: Fix timeouts occurring in synchronous/transacted routes
Posted by GitBox <gi...@apache.org>.
jeremyross commented on pull request #6085:
URL: https://github.com/apache/camel/pull/6085#issuecomment-918567998
@davsclaus how's this?
--
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] jeremyross commented on pull request #6085: CAMEL-16877: camel-salesforce: Fix timeouts occurring in synchronous/transacted routes
Posted by GitBox <gi...@apache.org>.
jeremyross commented on pull request #6085:
URL: https://github.com/apache/camel/pull/6085#issuecomment-919219800
@davsclaus Since this uses Camel's ExecutorServiceManager's newThreadPool() method, it should get shutdown as a part of context shutdown, but I added an explicit shutdown in case the http client is shut down outside of camel context.
--
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 pull request #6085: CAMEL-16877: camel-salesforce: Fix timeouts occurring in synchronous/transacted routes
Posted by GitBox <gi...@apache.org>.
davsclaus commented on pull request #6085:
URL: https://github.com/apache/camel/pull/6085#issuecomment-918484002
Ah yeah its not good to block that thread, so what you do is on the tight track.
However I dont think we should use the http client thread for continue routing. Instead the salesforce component should have a thread pool for worker threads to continue routing.
For example camel-kafka has this
https://github.com/apache/camel/blob/c0def695648fe93623d393950f8c06f8c526630f/components/camel-kafka/src/main/java/org/apache/camel/component/kafka/KafkaProducer.java#L481
--
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] jeremyross commented on a change in pull request #6085: CAMEL-16877: camel-salesforce: Fix timeouts occurring in synchronous/transacted routes
Posted by GitBox <gi...@apache.org>.
jeremyross commented on a change in pull request #6085:
URL: https://github.com/apache/camel/pull/6085#discussion_r709278550
##########
File path: components/camel-salesforce/camel-salesforce-component/src/main/java/org/apache/camel/component/salesforce/SalesforceHttpClient.java
##########
@@ -113,6 +113,12 @@ protected void doStart() throws Exception {
super.doStart();
}
+ @Override
+ protected void doStop() throws Exception {
+ workerPool.shutdown();
Review comment:
@davsclaus ok, now uses ExecutorServiceManager, if available. It won't be available if SalesforceHttpClient is instantiated outside of Camel, as is the case for the tooling that uses SalesforceHttpClient.
--
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] zregvart commented on pull request #6085: CAMEL-16877: camel-salesforce: Fix timeouts occurring in synchronous/transacted routes
Posted by GitBox <gi...@apache.org>.
zregvart commented on pull request #6085:
URL: https://github.com/apache/camel/pull/6085#issuecomment-919061955
I was looking at this when @jeremyross reported it a while back and completely forgot about it on my PTO. It looked like a deadlock but it was really difficult to find why, awesome work here @jeremyross!
--
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 pull request #6085: CAMEL-16877: camel-salesforce: Fix timeouts occurring in synchronous/transacted routes
Posted by GitBox <gi...@apache.org>.
davsclaus commented on pull request #6085:
URL: https://github.com/apache/camel/pull/6085#issuecomment-918797823
Yeah but there needs to be some code that shutdown that worker pool when the component is shutting down. And you may also want to add some options so you can set the pool sizes
--
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] jeremyross merged pull request #6085: CAMEL-16877: camel-salesforce: Fix timeouts occurring in synchronous/transacted routes
Posted by GitBox <gi...@apache.org>.
jeremyross merged pull request #6085:
URL: https://github.com/apache/camel/pull/6085
--
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