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

Only wait_for_less_busy_worker in cluster mode #3385

Conversation

joshuay03
Copy link
Contributor

@joshuay03 joshuay03 commented May 19, 2024

Description

Refs: #2079 and #2940

It looks like the wait_for_less_busy_worker logic also runs in single mode. This means that when a thread becomes available, the server unnecessarily delays handling the request as there are no other workers whose threads can pick it up instead.

This PR ensures wait_for_less_busy_worker only happens if puma is running in cluster mode.

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.

@joshuay03
Copy link
Contributor Author

joshuay03 commented May 20, 2024

Hmm the tests added in #2079 seems to be just a server test, asserting that requests are delayed within the same thread pool. I'll look into rewriting them.

@joshuay03
Copy link
Contributor Author

I'll look into rewriting them.

Actually, @nateberkopec, does this patch make sense / will it be accepted? Thought I'd check first before going and rewriting tests.

@dentarg
Copy link
Member

dentarg commented May 21, 2024

This makes sense to me, please go ahead 👍🏻

@dentarg
Copy link
Member

dentarg commented May 21, 2024

@joshuay03 have you seen #3325?

@joshuay03
Copy link
Contributor Author

@joshuay03 have you seen #3325?

Ah whoops I did not, I should really check open PRs before opening one 🤦🏽‍♂️ I'll close this in favour of that.

@joshuay03 joshuay03 closed this May 21, 2024
@joshuay03 joshuay03 deleted the avoid-wait-for-less-busy-worker-in-single-mode branch May 21, 2024 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants