You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by bgutjahr <gi...@git.apache.org> on 2016/04/25 15:48:05 UTC
[GitHub] activemq-artemis pull request: New thread pool for client threads
GitHub user bgutjahr opened a pull request:
https://github.com/apache/activemq-artemis/pull/488
New thread pool for client threads
- Added a thread pool executor, that combines cached and fixed size thread pooling.
It behaves like a cached thread pool in that it reuses exising threads and removes
idle threads after a timeout, limits the maximum number of threads in the pool, but
queues additional request instead of rejecting them.
- changed existing code to use the new thread pool instead of a fixed-size thread pool in
all places that are configured with a client thread pool size.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bgutjahr/activemq-artemis new-thread-pool
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/488.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #488
----
commit e1fd6a3dd7d54b2d22cbd5f6afe1acdd5e58838f
Author: Bernd Gutjahr <be...@hpe.com>
Date: 2016-04-25T13:30:42Z
New thread pool for client threads
- Added a thread pool executor, that combines cached and fixed size thread pooling.
It behaves like a cached thread pool in that it reuses exising threads and removes
idle threads after a timeout, limits the maximum number of threads in the pool, but
queue additional request instead of rejecting them.
- changed existing code to use the new thread pool instead of a fixed-size thread pool in
all places that are configured with a client thread pool size.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: New thread pool for client threads
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the pull request:
https://github.com/apache/activemq-artemis/pull/488#issuecomment-214720318
@bgutjahr Hi, There's a couple tests that are failing. They likely just need updating to reflect the change:
org.apache.activemq.artemis.ClientThreadPoolsTest.testSmallPool
org.apache.activemq.artemis.ClientThreadPoolsTest.testSystemPropertyThreadPoolSettings
org.apache.activemq.artemis.ClientThreadPoolsTest.testStaticPropertiesThreadPoolSettings
Could you please update. Thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: New thread pool for client threads
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the pull request:
https://github.com/apache/activemq-artemis/pull/488#issuecomment-215039113
@bgutjahr I created a JIRA here: https://issues.apache.org/jira/browse/ARTEMIS-507 and linked and merged your PR. Please assign the JIRA to yourself.
This is really great stuff. Thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: New thread pool for client threads
Posted by bgutjahr <gi...@git.apache.org>.
Github user bgutjahr commented on the pull request:
https://github.com/apache/activemq-artemis/pull/488#issuecomment-214989160
I have renamed a few variables. In addition, I converted the added source file from DOS to UNIX style line endings.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: New thread pool for client threads
Posted by bgutjahr <gi...@git.apache.org>.
Github user bgutjahr commented on the pull request:
https://github.com/apache/activemq-artemis/pull/488#issuecomment-214745279
I've updated the failing test cases.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: New thread pool for client threads
Posted by bgutjahr <gi...@git.apache.org>.
Github user bgutjahr commented on the pull request:
https://github.com/apache/activemq-artemis/pull/488#issuecomment-214982951
Great that it works in the testsuite. I'll fix the m_ name, I've just let my workplace coding rules slip in.
I can also enter a JIRA. Would that be issue type "Bug" or "New Feature"?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: New thread pool for client threads
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/488#issuecomment-214758799
Awesome! This was on my wish list for a while!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: New thread pool for client threads
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/488
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: New thread pool for client threads
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the pull request:
https://github.com/apache/activemq-artemis/pull/488#issuecomment-214337824
@bgutjahr Nicely done! Given the nature of the change I will need to run the extended test suite which might take a while...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: New thread pool for client threads
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/488#issuecomment-214939137
I think we should merge this... Testsuite seems ok... I ran quite a few tests on this... evaluated the code.. it all seems nice... the testsuite is saving 30 min overall time after this PR. which is great.
although 2 minor things:
I - I think this deserves a JIRA, and a commit ID associated with this JIRA.. it's a nice feature to be on the release notes:
II - The executor has a property called m_executor... which is the C++ style of variable names. I would just use java style... such as .. .currentExecutor.. or whatever... although I don't have a strong objection on it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: New thread pool for client threads
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/488#issuecomment-214796020
There must be something great after this. The whole testsuite finished in 1:50 Hrs on our CI, as opposed to the 2:07 Hrs it usually takes.
It's probably the thread leak check not taking a long time to shutdown unused threads.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---