[
https://issues.apache.org/jira/browse/QPIDJMS-552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17441090#comment-17441090
]
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_r745494450
##########
File path:
qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnectionFactory.java
##########
@@ -368,6 +377,19 @@ protected static URI createURI(String name) {
return null;
}
+ protected Supplier<Holder<ExecutorService>>
getCompletionExecutorServiceFactory() {
+ if (this.completionThreads == 0) {
+ return null;
+ }
+ synchronized (this) {
+ if (completionExecutorServiceFactory == null) {
+ QpidJMSForkJoinWorkerThreadFactory fjThreadFactory = new
QpidJMSForkJoinWorkerThreadFactory("completion thread pool", true);
+ completionExecutorServiceFactory = sharedRefCnt(() -> new
ForkJoinPool(completionThreads, fjThreadFactory, null, false),
ThreadPoolUtils::shutdown);
Review comment:
I meant was it an attempt to avoid creating the pool until the
completion executor is used. Not 'avoid creating it' in terms of pre-allocating
it with the factory.
This bit is only used after the point createConnection has been called, and
only if the option was set. I think its reasonable to assume the pool is to be
used from then given its an explicitly set option, and I dont see anyone ever
setting it unless they want the behaviour.
Given that, the mechanism still all seems rather overcomplicated. This feels
like a relatively simple case, an 'if there is an existing pool, then use that,
otherwise create one' check coupled with the opposing cleanup. One that should
be relatively infrequently used. It seems like even a simple synchronized block
with a count inside could do?
I'm also weighing this vast mechanism against against a change that enables
supplying a pool via the factory extension mechanism, which would probably be
something ridiculous like 5-10 lines in comparison.
--
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]
> 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: [email protected]
For additional commands, e-mail: [email protected]
