Update puma & puma_worker_killer to upstream (5.1.1)
What does this MR do?
Drops custom forks of puma
and puma_worker_killer
.
Replaces them with latest upstream versions.
More at #290004 (closed)
TODO
-
Add wait_for_less_busy_worker
into CNG and Omnibus: omnibus-gitlab!4803 (merged), gitlab-org/build/CNG!566 (merged)
Puma 5 breaking changes highlights
Deprecations, Removals and Breaking API Changes: from the puma/puma
changelog
Upgrade to 5.0 doc: https://github.com/puma/puma/blob/085750428c37feac497e3fc34bf75be86f79266f/5.0-Upgrade.md
Note: only 5.0
has breaking changes, there are no listed for 5.1
: https://github.com/puma/puma/blob/master/History.md
Puma 5 breaking changes checklist
-
--control has been removed. Use --control-url
We don't use it: https://github.com/puma/puma#controlstatus-server -
worker_directory has been removed. Use directory.
I didn't find that we use it -
min_threads now set by environment variables PUMA_MIN_THREADS and MIN_THREADS.
The description was not entirely clear, but according to the MR (https://github.com/puma/puma/pull/2143/files#diff-2dc4e3e83be7fd97cebc482ae07d6a8216944003de82458783fb00b5ae9524c8R181), this is an additional way to set this var, it doesn't replace the usage ofmin_threads
(what we currently do) -
max_threads now set by environment variables PUMA_MAX_THREADS and MAX_THREADS.
Same as above ^ -
max_threads default to 5 in MRI or 16 for all other interpreters.
From what I've seen, we set it explicitly, don't use defaults -
preload_app! is on by default if number of workers > 1 and set via WEB_CONCURRENCY
Shouldn't affect us, as we always preload -
Puma::Plugin.workers_supported? has been removed. Use Puma.forkable? instead.
I didn't find any usages of the mentioned method -
tcp_mode has been removed without replacement.
We don't use it. -
Daemonization has been removed without replacement.
For daemonization, please use a modern process management solution, such as systemd or monit.
- we should userunit
-
Changed #connected_port to #connected_ports
I didn't find we explicitly use it -
Configuration: environment is read from RAILS_ENV, if RACK_ENV can't be found
We setenvironment
explicitly -
Log binding on http:// for TCP bindings to make it clickable
We don't use it.
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry - [-] Documentation (if required)
-
Code review guidelines -
Merge request performance guidelines -
Style guides - [-] Database guides
- [-] Separation of EE specific content
Availability and Testing
- [-] Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
- [-] Tested in all supported browsers
-
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
- [-] Label as security and @ mention
@gitlab-com/gl-security/appsec
- [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
- [-] Security reports checked/validated by a reviewer from the AppSec team
Merge request reports
Activity
changed milestone to %13.7
added bugperformance groupcloud connector labels
added backend label
Reviewer roulette
Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.
To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited, or the chosen person is unavailable.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, mention them as you normally would! Danger does not automatically notify them for you.
Category Reviewer Maintainer backend Aishwarya Subramanian ( @asubramanian1
) (UTC-6, 9 hours behind@alipniagov
)Heinrich Lee Yu ( @engwan
) (UTC+8, 5 hours ahead of@alipniagov
)If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerEdited by 🤖 GitLab Bot 🤖added 1 commit
- fff09776 - Update puma & puma_worker_killer to upstream
marked the checklist item Code review guidelines as completed
marked the checklist item Merge request performance guidelines as completed
marked the checklist item Style guides as completed
added typefeature label
- Resolved by Aleksei Lipniagov
I am getting multiple errors with
An unhandled lowlevel error occurred. The application logs may have details.\n
(probably from https://github.com/puma/puma/blob/master/lib/puma/server.rb#L587), investigating.Happens both on CI and locally. Logs contents a bit different, probably related to environment setup.
Edited by Aleksei Lipniagov
added 2 commits
added 1983 commits
-
b552fbfe...e0dcb376 - 1979 commits from branch
master
- ac5fdda7 - Update puma & puma_worker_killer to upstream
- bc9bc304 - Add changelog entry
- a18ff95a - Bump puma to 5.1
- 4cd2a0c1 - Update capybara to fix failing specs
Toggle commit list-
b552fbfe...e0dcb376 - 1979 commits from branch
added tooling (archive) label
added 8 commits
-
4cd2a0c1...f0afb27e - 4 commits from branch
master
- 88095582 - Update puma & puma_worker_killer to upstream
- dc5c1209 - Add changelog entry
- 938de671 - Bump puma to 5.1
- 146c3332 - Update capybara to fix failing specs
Toggle commit list-
4cd2a0c1...f0afb27e - 4 commits from branch
Setting label(s) ~"Category:Memory" devopsenablement sectionenablement based on groupmemory.
added Category:Cloud Connector devopssystems sectioncore platform labels
added 176 commits
Toggle commit listmentioned in merge request gitlab-org/build/CNG!566 (merged)
mentioned in merge request omnibus-gitlab!4803 (merged)
mentioned in issue #290004 (closed)
marked the checklist item Add
wait_for_less_busy_worker
into CNG and Omnibus: omnibus-gitlab!4803 (merged), gitlab-org/build/CNG!566 (merged) as completedmarked the checklist item Informed Infrastructure department of a default or new setting change, if applicable per definition of done as completed
marked the checklist item Changelog entry as completed