Skip to content

Conversation

@joshuay03
Copy link
Collaborator

@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
Collaborator 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
Collaborator 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
@joshuay03 joshuay03 force-pushed the avoid-spawning-trim-thread-if-pool-size-is-fixed branch from fed4d4a to 60d3de6 Compare October 16, 2024 11:35
@joshuay03 joshuay03 force-pushed the avoid-spawning-trim-thread-if-pool-size-is-fixed branch from 60d3de6 to e556608 Compare October 16, 2024 11:40
@nateberkopec
Copy link
Member

That was really sweet that you both added each other as co-author ❤️

I like the tests on this one a bit more, so I'll go with this one.

@nateberkopec nateberkopec merged commit 5f41574 into puma:master Nov 23, 2024
@joshuay03 joshuay03 deleted the avoid-spawning-trim-thread-if-pool-size-is-fixed branch November 23, 2024 01:16
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.

4 participants