Skip to content

fix(sidekiq): make sidekiq PID 1

Steve Xuereb requested to merge fix/sidekiq-pid1 into master

What does this MR do?

What

  • Update the CMD to be in exec form rather then shell form.
  • Update process-wrapper script to use exec infront of sidekiq-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

Merge request reports