[
https://issues.apache.org/jira/browse/QPIDJMS-552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17441100#comment-17441100
]
ASF GitHub Bot commented on QPIDJMS-552:
----------------------------------------
gemmellr commented on a change in pull request #44:
URL: https://github.com/apache/qpid-jms/pull/44#discussion_r745512309
##########
File path: qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java
##########
@@ -194,6 +198,48 @@ public void onPendingFailure(ProviderException cause) {
}
}
+ private void processCompletions() {
+ assert processCompletion.get();
+ completionThread = Thread.currentThread();
+ try {
+ final Runnable completionTask = completionTasks.poll();
+ if (completionTask != null) {
+ try {
+ completionTask.run();
+ } catch (Throwable t) {
+ LOG.debug("errored on processCompletions duty cycle", t);
+ }
+ }
+ } finally {
+ completionThread = null;
+ processCompletion.set(false);
+ }
+ if (completionTasks.isEmpty()) {
+ return;
+ }
+ // a racing asyncProcessCompletion has won: no need to fire a
continuation
+ if (!processCompletion.compareAndSet(false, true)) {
+ return;
+ }
+ getCompletionExecutor().execute(this::processCompletions);
+ }
+
+ private void asyncProcessCompletion(final Runnable completionTask) {
+ asyncProcessCompletion(completionTask, false);
+ }
+
+ private void asyncProcessCompletion(final Runnable completionTask, final
boolean ignoreSessionClosed) {
+ if (!ignoreSessionClosed) {
Review comment:
Ill admit I forgot about that bit, but yes it is a little different -
that is something that only occurs later, after the executor is shut down in
the existing non-shared-completion case, and is handling a case that probably
wont occur, and also shouldnt matter if it did because something else already
cleaned up anything related to it before killing the executor.
Here, the work would start being thrown away immediatelym even before the
producer itself is necessarily notified its being considered closed, and long
before the executor is being shut down (which in the regular non-shared-pool
case would still happen), so it could actually be a quite significant change in
behaviour. I think this introduces a hole that didnt exist before.
--
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: dev-unsubscr...@qpid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> JMS 2 Completion threads shouldn't scale with the number of sessions
> --------------------------------------------------------------------
>
> Key: QPIDJMS-552
> URL: https://issues.apache.org/jira/browse/QPIDJMS-552
> Project: Qpid JMS
> Issue Type: Bug
> Components: qpid-jms-client
> Affects Versions: 1.3.0
> Reporter: Francesco Nigro
> Priority: Major
>
> JMS 2 Completion threads are now tied to be one per JMS Session ie a client
> application with N JMS sessions need N completion threads to handle the
> completion events.
> Given that the asynchronous model of JMS 2 allows users to have few threads
> handling many JMS sessions, should be better to reduce the amount of
> completion threads without exceeding the number of available cores and
> shrink/grow according to the completion event processing load.
> If the user confine a connection in a thread handling many JMS sessions and
> the completion events are issued by the same Netty thread in sequence, if the
> completion processing for a JMS Session is fast enough, next JMS Sessions can
> reuse existing completion threads instead of using a new one.
> This model save using too many completion threads for users tasks that are
> supposed to be very fast: if the user task cause a specific JMS Session
> completion thread to block, the expectation is that the system should be able
> to create a new completion thread to handle other JMS Session completion
> events, as expected.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
