fix(sidekiq): make sidekiq PID 1
What does this MR do?
What
- Update the
CMD
to be inexec form
rather thenshell form
. - Update
process-wrapper
script to useexec
infront ofsidekiq-cluster
so it replaces the PID of the bash script.
Why
When using CMD command
(shell form) CRI will executor /bin/sh
automatically, whilst using CMD ["executable"]
(exec form) it will run
exec
infront of the command so the executable becomes PID 1.
sidekiq-cluster
was never PID 1 so it never recived SIGTERM
so
graceful termination was not the default behavior as we can see in the
process tree below
git 1 0.0 0.0 2420 528 ? Ss 13:02 0:00 /bin/sh -c /scripts/process-wrapper
git 19 0.0 0.0 5728 3464 ? S 13:02 0:00 /bin/bash /scripts/process-wrapper
git 20 0.0 0.0 2316 628 ? S 13:02 0:00 \_ xtail /var/log/gitlab
git 21 2.4 0.4 146836 42424 ? Sl 13:02 0:03 \_ ruby bin/sidekiq-cluster -r /srv/gitlab -e production --min-concurrency 25 --max-concurrency 25 -t 25 *
git 22 56.1 9.4 1593864 970860 ? Sl 13:02 1:16 \_ sidekiq 6.4.0 queues:authorized_project_update:authorized_project_update_project_create,authorized_project_update:authorized_project_update_project_group_link_cre
git 24 0.0 0.3 140700 38624 ? Sl 13:02 0:00 \_ sidekiq_exporter
After the change:
USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
git 112 0.5 0.0 5988 3732 pts/0 Ss 15:28 0:00 bash
git 120 0.0 0.0 8592 3224 pts/0 R+ 15:28 0:00 \_ ps faux
git 1 5.0 0.4 146836 42292 ? Ssl 15:26 0:05 ruby /srv/gitlab/bin/sidekiq-cluster -r /srv/gitlab -e production --min-concurrency 25 --max-concurrency 25 -t 25 *
git 19 0.0 0.0 2316 632 ? S 15:26 0:00 xtail /var/log/gitlab
git 20 58.1 9.9 1570372 1019944 ? Sl 15:26 0:55 sidekiq 6.4.0 queues:authorized_project_update:authorized_project_update_project_recalculate,authorized_project_update:authorized_project_update_project_recalculate_per_user,authorized_project_update:authorized_project_update_user_refresh_from_replica,authorized_project_update:authorized_project_update_user_refresh_over_user_range,authorized_project_update:authorized_project_update_user_refresh_with_low_urg
git 22 0.0 0.3 140700 38784 ? Sl 15:26 0:00 sidekiq_exporter
For GitLab Helm charts this was never a problem because we use pkill
Related issues
gitlab-org/charts/gitlab#3249 (closed)
Checklist
See Definition of done.
For anything in this list which will not be completed, please provide a reason in the MR discussion
Required
-
Merge Request Title, and Description are up to date, accurate, and descriptive -
MR targeting the appropriate branch -
MR has a green pipeline on GitLab.com
Expected (please provide an explanation if not completing)
-
Test plan indicating conditions for success has been posted and passes -
Documentation created/updated -
Integration tests added to GitLab QA -
The impact any change in container size has should be evaluated
Edited by Jason Plum