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
nio4r
for 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_stats
plugin 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.