Update puma Gem to 5.6.5 - #51929
Merged
Merged
Conversation
Also switch back to mainline. Target 5.x for compatibility with Ruby 3 TODO: identify whether the changes in our custom branch are included in this version of puma, and if not whether we still need them. - https://github.com/puma/puma/blob/master/5.0-Upgrade.md#welcome-to-puma-5-spoony-bard
…oesn't provide the log_stats plugin. TODO: verify that we don't actually need this logging anymore
Hamms
added
the
Ruby Update
Hamms
marked this pull request as ready for review
May 18, 2023 20:27
sureshc
approved these changes
May 18, 2023
Contributor
|
🎉 |
This was referenced May 23, 2023
Merged
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
3 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.
Also switch back to mainline. Target 5.x for compatibility with Ruby 3.
This should be a relatively safe upgrade, despite taking us through two major versions. The main breaking change in 4.x was the switch to
nio4rfor I/O management, and in 5.x it was removal of support for plugins and versions of Ruby that we do not use.From what I can tell, we maintained a custom branch for two reasons: one was to provide the
log_statsplugin we were using for some performance monitoring. We don't use this data anywhere and don't need to keep collecting it, so the simple solution is to remove the monitoring logic.The other reason was to implement some performance fixes. Specifically, we add some logic to attempt to balance incoming requests between worker threads in a more informed way. According to Will's notes, we were getting some performance improvements out of this. Puma 5.6.5 does not include our custom logic, but does include this alternative authored by the folks at GitLab which instructs busy worker threads to be less eager to pick up new jobs, thereby allowing the jobs to be distributed to other workers. According to some of Will's notes in that PR, we did also get performance improvements from this change but it's not clear to me how much or how they compare to the other set of improvements.
Ultimately, my proposal at this point is that we simply apply this update and see what the performance implications are. It should be straightforward to revert if we're unhappy with the effects.
Links
Testing story
For correctness testing, I tested locally and on an adhoc that loading the site works fine both on Ruby 2.7 and 3.0.
For performance testing, we plan to rely on post-merge monitoring.
Deployment strategy
Because this is a low-level dependency with a lot of potential performance implications, we plan to deploy this in a standalone deploy after our main deploy with a closed pipeline and a revert prepped.