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

Avoid spawning Threadpool#trim thread if pool size is fixed #3384

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joshuay03
Copy link
Contributor

@joshuay03 joshuay03 commented May 19, 2024

Description

While working on #3383 I found that a thread was being spawned to trim the server's thread pool's threads every :auto_trim_time seconds regardless of whether or not the pool needed trimming, which would only be when the thread count is configured to scale automatically i.e. min != max.

This PR ensures that specific thread is only spawned if the thread count is not fixed.

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

@rus-max
Copy link

rus-max commented May 19, 2024

#3306

@joshuay03 joshuay03 force-pushed the avoid-spawning-trim-thread-if-pool-size-is-fixed branch 2 times, most recently from 80a764b to 97baf4e Compare May 19, 2024 11:01
auto_trim_time = 3
server_run(min_threads: 2, max_threads: 2, auto_trim_time: auto_trim_time)
sleep 1 # wait for possible initial trim on run
thread_pool_expect false, :trim, nil, after: auto_trim_time
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without patch:
Screenshot 2024-05-19 at 7 56 23 PM

@joshuay03
Copy link
Contributor Author

#3306

@rus-max Ah I did not see that, thanks!

I'll leave this open cause I took a different approach with the tests. I'll add @OuYangJinTing as a co-author if the maintainers choose to go with this PR.

@joshuay03 joshuay03 force-pushed the avoid-spawning-trim-thread-if-pool-size-is-fixed branch from 97baf4e to fed4d4a Compare May 19, 2024 11:07
@dentarg dentarg added feature waiting-for-review Waiting on review from anyone labels Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature waiting-for-review Waiting on review from anyone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants