Commit 6ee0f133 authored by Stan Hu's avatar Stan Hu

Make sidekiq-cluster play well with Sidekiq 5.2.2+

Sidekiq 5.2.2 introduced a change
(https://github.com/mperham/sidekiq/pull/3936/files) that prevents queue
names from being duplicated. This breaks our current GitLab.com
config because we have some sidekiq-cluster processes configured
as the following:

```ruby
sidekiq_cluster['queue_groups'] = ["process_commit,process_commit"]
```

To keep things backwards compatible, we now only pass unique queue names
but scale the weights accordingly by the number of queues mentioned.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/54253
parent 27a8297d
---
title: Make sidekiq-cluster play well with Sidekiq 5.2.2+
merge_request: 8522
author:
type: other
...@@ -70,14 +70,16 @@ module Gitlab ...@@ -70,14 +70,16 @@ module Gitlab
# #
# Returns the PID of the started process. # Returns the PID of the started process.
def self.start_sidekiq(queues, env, directory = Dir.pwd, max_concurrency = 50, dryrun: false) def self.start_sidekiq(queues, env, directory = Dir.pwd, max_concurrency = 50, dryrun: false)
counts = count_by_queue(queues)
cmd = %w[bundle exec sidekiq] cmd = %w[bundle exec sidekiq]
cmd << "-c #{self.concurrency(queues, max_concurrency)}" cmd << "-c #{self.concurrency(queues, max_concurrency)}"
cmd << "-e#{env}" cmd << "-e#{env}"
cmd << "-gqueues: #{queues.join(', ')}" cmd << "-gqueues: #{proc_details(counts)}"
cmd << "-r#{directory}" cmd << "-r#{directory}"
queues.each do |q| counts.each do |queue, count|
cmd << "-q#{q},1" cmd << "-q#{queue},#{count}"
end end
if dryrun if dryrun
...@@ -97,6 +99,20 @@ module Gitlab ...@@ -97,6 +99,20 @@ module Gitlab
pid pid
end end
def self.count_by_queue(queues)
queues.each_with_object(Hash.new(0)) { |element, hash| hash[element] += 1 }
end
def self.proc_details(counts)
counts.map do |queue, count|
if count == 1
queue
else
"#{queue} (#{count})"
end
end.join(', ')
end
def self.concurrency(queues, max_concurrency) def self.concurrency(queues, max_concurrency)
if max_concurrency.positive? if max_concurrency.positive?
[queues.length + 1, max_concurrency].min [queues.length + 1, max_concurrency].min
......
...@@ -78,12 +78,23 @@ describe Gitlab::SidekiqCluster do ...@@ -78,12 +78,23 @@ describe Gitlab::SidekiqCluster do
end end
describe '.start_sidekiq' do describe '.start_sidekiq' do
let(:env) { { "ENABLE_SIDEKIQ_CLUSTER" => "1" } }
it 'starts a Sidekiq process' do it 'starts a Sidekiq process' do
allow(Process).to receive(:spawn).and_return(1) allow(Process).to receive(:spawn).and_return(1)
expect(described_class).to receive(:wait_async).with(1) expect(described_class).to receive(:wait_async).with(1)
expect(described_class.start_sidekiq(%w(foo), :production)).to eq(1) expect(described_class.start_sidekiq(%w(foo), :production)).to eq(1)
end end
it 'handles duplicate queue names' do
allow(Process).to receive(:spawn)
.with(env, "bundle", "exec", "sidekiq", "-c 5", "-eproduction", "-gqueues: foo (2), bar, baz", anything, "-qfoo,2", "-qbar,1", "-qbaz,1", anything)
.and_return(1)
expect(described_class).to receive(:wait_async).with(1)
expect(described_class.start_sidekiq(%w(foo foo bar baz), :production)).to eq(1)
end
end end
describe '.wait_async' do describe '.wait_async' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment