Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change thread pool to a ScalingExecutorBuilder #421

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

dbwiddis
Copy link
Member

Description

Changes Flow Framework thread pool to use a ScalingExecutorBuilder

  • the FixedExecutorBuilder establishes a maximum queue size which is arbitrary and not scalable
  • the ResizableExecutorBuilder allowing users to manually change this isn't used much
  • the ScalingExecutorBuilder is widely used across OpenSearch and plugins and is clearly the preferred option here unless we know better. (Which we don't.)

Matches "generic" thread pool type, with different number of processors since it's more narrowly focused.

Uses similar threadpool to AD except with shorter keepalive to correspond to most OpenSearch thread pool times, and uses full number of allocated processors (smaller # than generic).

Allows user to change # of processors per node via settings.

Issues Resolved

Fixes #420

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions github-actions bot added the backport 2.x backport PRs to 2.x branch label Jan 19, 2024
Copy link

codecov bot commented Jan 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (bf2cad0) 71.84% compared to head (d832437) 71.85%.

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #421   +/-   ##
=========================================
  Coverage     71.84%   71.85%           
  Complexity      614      614           
=========================================
  Files            79       79           
  Lines          3083     3084    +1     
  Branches        237      237           
=========================================
+ Hits           2215     2216    +1     
  Misses          763      763           
  Partials        105      105           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@amitgalitz
Copy link
Member

btw just for reference I think we used fixed as that was used by ml-commons here: https://github.com/opensearch-project/ml-commons/blob/41fac826c8fd1182f83c1f3f6b301bf17022eb2f/plugin/src/main/java/org/opensearch/ml/plugin/MachineLearningPlugin.java#L784

@dbwiddis dbwiddis merged commit 789fc4d into opensearch-project:main Jan 19, 2024
25 of 26 checks passed
@dbwiddis dbwiddis deleted the threadpool-settings branch January 19, 2024 22:41
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 19, 2024
Signed-off-by: Daniel Widdis <[email protected]>
(cherry picked from commit 789fc4d)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
dbwiddis pushed a commit that referenced this pull request Jan 19, 2024
Change thread pool to a ScalingExecutorBuilder (#421)


(cherry picked from commit 789fc4d)

Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport PRs to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Make thread pool size a configurable conatant
4 participants