Make worker and API thread pool size configurable#13565
Open
Damans227 wants to merge 2 commits into
Open
Conversation
The AsyncJobManager's API and Worker executor thread pool sizes are currently derived solely from db.cloud.maxActive (maxActive/2 and maxActive*2/3 respectively). This coupling doesn't account for environments with predominantly I/O-bound workers, where threads are held waiting on external responses for extended periods and higher pool sizes may be needed independently of the DB connection count. This adds two global config keys: - api.job.pool.size (default: 50) - work.job.pool.size (default: 50) The actual pool size is Math.max(configured value, db-derived default), preserving existing behavior by default while allowing operators to increase pool sizes when workloads require it. Pool sizes and their derivation are logged at startup for observability.
…rtions The previous unit tests for the api.job.pool.size/work.job.pool.size config keys only re-ran Math.max() on local hardcoded values, or checked ConfigKey plumbing directly, without ever calling AsyncJobManagerImpl.configure() itself. That meant a regression in the actual pool-sizing logic wouldn't have been caught. These tests now invoke configure() directly and assert on the real core pool size of the created _apiJobExecutor/_workerJobExecutor via reflection, with @Before/@after hooks to stub db.properties and reset config/executor state between tests.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #13565 +/- ##
=========================================
Coverage 17.67% 17.67%
- Complexity 15797 15798 +1
=========================================
Files 5923 5923
Lines 533349 533355 +6
Branches 65248 65248
=========================================
+ Hits 94253 94267 +14
+ Misses 428437 428428 -9
- Partials 10659 10660 +1
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:
|
Contributor
|
@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 18490 |
| Integer.class, "vm.job.lock.timeout", "1800", | ||
| "Time in seconds to wait in acquiring lock to submit a vm worker job", false); | ||
| private static final ConfigKey<Boolean> HidePassword = new ConfigKey<Boolean>("Advanced", Boolean.class, "log.hide.password", "true", "If set to true, the password is hidden", true, ConfigKey.Scope.Global); | ||
| public static final ConfigKey<Integer> ApiJobPoolSize = new ConfigKey<>("Advanced", Integer.class, "api.job.pool.size", "50", |
Member
There was a problem hiding this comment.
I recall Vishesh had some changes in the past so that the pool size can be updated dynamically when the value is changed, no need to restart management server.
maybe @vishesh92 can give some advice
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The AsyncJobManager's API and Worker executor thread pool sizes are currently derived solely from
db.cloud.maxActive(maxActive/2andmaxActive*2/3respectively). This coupling doesn't account for environments with predominantly I/O-bound workers, where threads are held waiting on external responses for extended periods and higher pool sizes may be needed independently of the DB connection count.This PR adds two global config keys:
api.job.pool.size(default: 50)work.job.pool.size(default: 50)The actual pool size is
Math.max(configured value, db-derived default), preserving existing behavior by default while allowing operators to increase pool sizes when workloads require it. Pool sizes and their derivation are logged at startup for observability.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
How Has This Been Tested?
Added unit tests in
AsyncJobManagerImplTestthat callconfigure()directly and assert on the real core pool size of the created executors (via reflection), covering both the "configured value wins" and "db-derived default wins" cases for both pools. Ran the fullAsyncJobManagerImplTestsuite locally (7 tests), all passing.How did you try to break this feature and the system with this change?
Verified the new config keys follow the existing
ConfigKeypattern in this class (category, type, name, default, description, isDynamic, scope), and thatisDynamicis correctly set tofalsesince the executor pools are only created once at startup, so a dynamic config change wouldn't take effect without a restart anyway.