Make AgentConnectTaskPool core and max pool sizes configurable via global config#13552
Make AgentConnectTaskPool core and max pool sizes configurable via global config#13552nvazquez wants to merge 3 commits into
Conversation
…obal config (apache#839) Replaces hardcoded values of 100/500 in initConnectExecutor() with ConfigKeys agent.connect.executor.core.pool.size (default 100) and agent.connect.executor.max.pool.size (default 500).
|
@blueorangutan package |
|
@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #13552 +/- ##
============================================
+ Coverage 17.67% 18.77% +1.10%
- Complexity 15796 15832 +36
============================================
Files 5923 5476 -447
Lines 533347 495373 -37974
Branches 65248 58203 -7045
============================================
- Hits 94253 92992 -1261
+ Misses 428435 391889 -36546
+ Partials 10659 10492 -167
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18480 |
|
|
||
| private void initConnectExecutor() { | ||
| _connectExecutor = new ThreadPoolExecutor(100, 500, 60L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), new NamedThreadFactory("AgentConnectTaskPool")); | ||
| _connectExecutor = new ThreadPoolExecutor(AgentConnectExecutorCorePoolSize.value(), AgentConnectExecutorMaxPoolSize.value(), 60L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), new NamedThreadFactory("AgentConnectTaskPool")); |
There was a problem hiding this comment.
Why no validation here? If max ends up lower than core, this throws and crashes management server startup. Maybe clamp the values before use so a bad config doesn't break things.
There was a problem hiding this comment.
Thanks @Damans227 - added a validation. Can you please review?
There was a problem hiding this comment.
Thanks @nvazquez, looks good. One question: should the max check be agentConnectMaxSize <= 0? ThreadPoolExecutor requires max to be strictly positive, so setting both keys to 0 passes validation but still throws at startup.
vladimirpetrov
left a comment
There was a problem hiding this comment.
LGTM based on manual testing. Tested with simultaneously restarting all the agents, which are more than the max pool size.
|
@blueorangutan test |
|
@vladimirpetrov a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-16525)
|
|
@blueorangutan package |
|
@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18575 |
|
@blueorangutan test |
vladimirpetrov
left a comment
There was a problem hiding this comment.
LGTM, tested with core > max and invalid values.
Description
This PR replaces the hardcoded values of 100/500 in initConnectExecutor() with ConfigKeys:
agent.connect.executor.core.pool.size(default 100) andagent.connect.executor.max.pool.size(default 500)In case the pool size > max pool size, the default values 100/500 will be used to prevent exceptions on startup
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?