Skip to content
Snippets Groups Projects

Ability to start dedicated Sidekiq processes

Merged Yorick Peterse requested to merge sidekiq-cluster into master
2 unresolved threads

This adds the ability to start Sidekiq processes for specific queues. See https://gitlab.com/gitlab-org/gitlab-ee/issues/1282 for more information.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • 1 require 'open3'
    2
    3 module Gitlab
    4 module SidekiqCluster
    5 # The signals that should terminate both the master and workers.
    6 TERMINATE_SIGNALS = %i(INT TERM)
    7
    8 # The signals that should simply be forwarded to the workers.
    9 FORWARD_SIGNALS = %i(TTIN USR1 USR2 HUP)
  • @marin From a packaging perspective, what are your requests when adding a new process to monitor by runit? For example: the script writes to a PID file if told to do so, but I'd like to know if there are more requests.

    Edited by Yorick Peterse
  • @yorickpeterse FYI, @marin is out for a few days. As far as I can tell, I think if we follow the conventions of the current runit processes, I think we'll be okay. Maybe @twk3 can chime in here too.

  • Added ~901587 label

  • Looks good to me, I don't see anything scary here.

  • Yorick Peterse Added 1 commit:

    Added 1 commit:

    • 7aa95d04 - Ability to start dedicated Sidekiq processes

    Compare with previous version

  • @stanhu Therein lies the problem: I'm not really aware of what those patterns/standards are other than processes not detaching themselves.

  • Yorick Peterse Added 1 commit:

    Added 1 commit:

    • 9ceecb99 - Ability to start dedicated Sidekiq processes

    Compare with previous version

  • Yorick Peterse added 710 commits

    added 710 commits

    Compare with previous version

  • Yorick Peterse added 1 commit

    added 1 commit

    • b137e5ba - Ability to start dedicated Sidekiq processes

    Compare with previous version

  • @marin @twk3 Could one of you shed some light on what we need to make packaging/supervising this (using runit) as easy as possible?

  • @yorickpeterse I am having a bit of a problem following what you are trying to achieve (and ask). So I will write what I think is happening and we can get to the common ground from there:

    The idea here is to be able to start up a process on demand and letting the supervisor handle the process from there on?

    You are asking whether you need to do anything special for the supervisor? If that is the question, the answer is no. Supervisor watches the processes and daemonizes them automatically. If the process gets killed or stops, it will try to restart it and it will do that till the concept of time changes.

    One question I have for you is, this is supposed to allow us to quickly spin up additional background workers and not as a replacement for the "main" sidekiq?

  • @marin The idea is to provide a tool that lets us start extra Sidekiq workers that only process a given set of queues. For GitLab.com we have a hack that uses tmux, but this of course doesn't work very pleasantly. These extra workers would be used to process important queues so processing timings are not affected by other queues.

    To make this as easy as possible we came up with the idea of having some sort of wrapper script. This script would take a set of arguments (e.g. the queues) and spawn a number of sidekiq processes with the right settings. This is what sidekiq-cluster in this MR does. This can lead to a setup where sidekiq processes everything as usual, and sidekiq-cluster starts two processes that process process_commit and post_receive respectively (thus we'd have 3 sidekiq processes).

    One question I have for you is, this is supposed to allow us to quickly spin up additional background workers and not as a replacement for the "main" sidekiq?

    It would not replace the main sidekiq process, instead it would complement it. This does mean that instead of something like sudo gitlab-ctl restart sidekiq we'd also have to run sudo gitlab-ctl restart sidekiq-cluster. For Omnibus this also means storing some configuration settings (e.g. the queues to use) somewhere, otherwise restart & friends won't know what settings to use. This could be something as simple as just a string passed to sidekiq-cluster.

    Does that clear things up a bit?

    Edited by Yorick Peterse
  • @yorickpeterse Cool, thanks for explaining.

  • Yorick Peterse unmarked as a Work In Progress

    unmarked as a Work In Progress

  • Yorick Peterse added 259 commits

    added 259 commits

    Compare with previous version

  • @marin OK, so is there anything extra we need in this MR to make packaging this up as easy as possible in Omnbius? If not I'll start adding documentation for how this thing works.

  • @yorickpeterse No, I think this will Just Work (TM). If we need something extra, you will be pinged.

    I just need an issue from you in omnibus-gitlab with parts of this discussion in how you see this being used so we can assign people on it.

  • Yorick Peterse added 1 commit

    added 1 commit

    • 9660d2f0 - Ability to start dedicated Sidekiq processes

    Compare with previous version

  • Yorick Peterse added 1 commit

    added 1 commit

    • 56122dba - Ability to start dedicated Sidekiq processes

    Compare with previous version

  • @yorickpeterse does this need to be ~"Pick into Stable" ?

  • @smcgivern Shoot I forgot to add that label, yes.

  • Yorick Peterse added ~149424 label

    added ~149424 label

  • Sean McGivern
  • Sean McGivern
  • @yorickpeterse this looks good to me! Neither of my questions are important; if this needs to be in 8.15, please add ~"Pick into Stable" and merge!

  • Ah, you already did :smiley:

    Then just see the questions above!

  • Yorick Peterse added 855 commits

    added 855 commits

    Compare with previous version

  • Yorick Peterse enabled an automatic merge when the build for 160fc3f9 succeeds

    enabled an automatic merge when the build for 160fc3f9 succeeds

  • Yorick Peterse canceled the automatic merge

    canceled the automatic merge

  • Yorick Peterse added 1 commit

    added 1 commit

    • b588a0c4 - Ability to start dedicated Sidekiq processes

    Compare with previous version

  • Yorick Peterse enabled an automatic merge when the build for b588a0c4 succeeds

    enabled an automatic merge when the build for b588a0c4 succeeds

  • Yorick Peterse mentioned in commit 7ff93264

    mentioned in commit 7ff93264

  • mentioned in merge request omnibus-gitlab!1180 (merged)

  • Picked into 8-15-stable-ee, will go into 8.15.0-rc4.

  • Douglas Barbosa Alexandre removed ~149424 label

    removed ~149424 label

  • Yorick Peterse mentioned in commit 450865f9

    mentioned in commit 450865f9

  • 62 # queues to use for a single process.
    63 #
    64 # Returns an Array containing the threads monitoring each process.
    65 def self.start(queues, env)
    66 queues.map { |pair| start_sidekiq(pair, env) }
    67 end
    68
    69 # Starts a Sidekiq process that processes _only_ the given queues.
    70 def self.start_sidekiq(queues, env)
    71 switches = queues.map { |q| "-q #{q},1" }
    72
    73 Open3.popen3({ 'ENABLE_SIDEKIQ_CLUSTER' => '1' },
    74 'bundle',
    75 'exec',
    76 'sidekiq',
    77 "-c #{queues.length + 1}",
  • Sean McGivern mentioned in merge request !23408 (merged)

    mentioned in merge request !23408 (merged)

  • Please register or sign in to reply
    Loading