From 760c0af22e8d878a390d0c0d844f4e5950a87987 Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Thu, 24 Nov 2022 14:00:18 +0000
Subject: [PATCH] Set Sidekiq default max concurrency to 20
The previous default of 50 was far too high. We don't use this on
GitLab.com and it's mostly inertia that has left it at this value.
Changelog: changed
---
doc/administration/postgresql/pgbouncer.md | 4 ++--
doc/update/index.md | 13 +++++++++++++
sidekiq_cluster/cli.rb | 5 +++--
sidekiq_cluster/sidekiq_cluster.rb | 2 +-
spec/commands/sidekiq_cluster/cli_spec.rb | 2 +-
spec/sidekiq_cluster/sidekiq_cluster_spec.rb | 2 +-
6 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/doc/administration/postgresql/pgbouncer.md b/doc/administration/postgresql/pgbouncer.md
index 91c689fadeabc..25c4c940b9724 100644
--- a/doc/administration/postgresql/pgbouncer.md
+++ b/doc/administration/postgresql/pgbouncer.md
@@ -220,8 +220,8 @@ the database. Each of the listed services below use the following formula to def
- `puma` : `max_threads + headroom` (default `14`)
- `max_threads` is configured via: `gitlab['puma']['max_threads']` (default: `4`)
- `headroom` can be configured via `DB_POOL_HEADROOM` environment variable (default to `10`)
-- `sidekiq` : `max_concurrency + 1 + headroom` (default: `61`)
- - `max_concurrency` is configured via: `sidekiq['max_concurrency']` (default: `50`)
+- `sidekiq` : `max_concurrency + 1 + headroom` (default: `31`)
+ - `max_concurrency` is configured via: `sidekiq['max_concurrency']` (default: `20`)
- `headroom` can be configured via `DB_POOL_HEADROOM` environment variable (default to `10`)
- `geo-logcursor`: `1+headroom` (default: `11`)
- `headroom` can be configured via `DB_POOL_HEADROOM` environment variable (default to `10`)
diff --git a/doc/update/index.md b/doc/update/index.md
index 04082bd642343..33545c05d89d4 100644
--- a/doc/update/index.md
+++ b/doc/update/index.md
@@ -481,6 +481,19 @@ and [Helm Chart deployments](https://docs.gitlab.com/charts/). They come with ap
[backfill `namespace_id` values on issues table](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91921). This
migration might take multiple hours or days to complete on larger GitLab instances. Please make sure the migration
has completed successfully before upgrading to 15.7.0.
+- The default Sidekiq `max_concurrency` has been changed to 20. This is now
+ consistent in our documentation and product defaults.
+
+ For example, previously:
+ - Omnibus GitLab default (`sidekiq['max_concurrency']`): 50
+ - From source installation default: 50
+ - Helm chart default (`gitlab.sidekiq.concurrency`): 25
+
+ Reference architectures still use a default of 10 as this is set specifically
+ for those configurations.
+
+ Sites that have configured `max_concurrency` will not be affected by this change.
+ [Read more about the Sidekiq concurrency setting](../administration/sidekiq/extra_sidekiq_processes.md#concurrency).
### 15.6.0
diff --git a/sidekiq_cluster/cli.rb b/sidekiq_cluster/cli.rb
index 52dc14130fbe9..341ebd9019af4 100644
--- a/sidekiq_cluster/cli.rb
+++ b/sidekiq_cluster/cli.rb
@@ -31,8 +31,9 @@ class CLI
CommandError = Class.new(StandardError)
def initialize(log_output = $stderr)
- # As recommended by https://github.com/mperham/sidekiq/wiki/Advanced-Options#concurrency
- @max_concurrency = 50
+ # https://github.com/mperham/sidekiq/wiki/Advanced-Options#concurrency
+ # https://ruby.social/@getajobmike/109326475545816363
+ @max_concurrency = 20
@min_concurrency = 0
@environment = ENV['RAILS_ENV'] || 'development'
@metrics_dir = ENV["prometheus_multiproc_dir"] || File.absolute_path("tmp/prometheus_multiproc_dir/sidekiq")
diff --git a/sidekiq_cluster/sidekiq_cluster.rb b/sidekiq_cluster/sidekiq_cluster.rb
index c68cbe7c16304..66fb5603d2b78 100644
--- a/sidekiq_cluster/sidekiq_cluster.rb
+++ b/sidekiq_cluster/sidekiq_cluster.rb
@@ -34,7 +34,7 @@ module SidekiqCluster
# directory - The directory of the Rails application.
#
# Returns an Array containing the PIDs of the started processes.
- def self.start(queues, env: :development, directory: Dir.pwd, max_concurrency: 50, min_concurrency: 0, timeout: DEFAULT_SOFT_TIMEOUT_SECONDS, dryrun: false)
+ def self.start(queues, env: :development, directory: Dir.pwd, max_concurrency: 20, min_concurrency: 0, timeout: DEFAULT_SOFT_TIMEOUT_SECONDS, dryrun: false)
queues.map.with_index do |pair, index|
start_sidekiq(pair, env: env,
directory: directory,
diff --git a/spec/commands/sidekiq_cluster/cli_spec.rb b/spec/commands/sidekiq_cluster/cli_spec.rb
index 0b73a62e1e037..4618c6681d32f 100644
--- a/spec/commands/sidekiq_cluster/cli_spec.rb
+++ b/spec/commands/sidekiq_cluster/cli_spec.rb
@@ -13,7 +13,7 @@
let(:cli) { described_class.new('/dev/null') }
let(:timeout) { Gitlab::SidekiqCluster::DEFAULT_SOFT_TIMEOUT_SECONDS }
let(:default_options) do
- { env: 'test', directory: Dir.pwd, max_concurrency: 50, min_concurrency: 0, dryrun: false, timeout: timeout }
+ { env: 'test', directory: Dir.pwd, max_concurrency: 20, min_concurrency: 0, dryrun: false, timeout: timeout }
end
let(:sidekiq_exporter_enabled) { false }
diff --git a/spec/sidekiq_cluster/sidekiq_cluster_spec.rb b/spec/sidekiq_cluster/sidekiq_cluster_spec.rb
index c0a919a4aec8d..822acc3fe0f62 100644
--- a/spec/sidekiq_cluster/sidekiq_cluster_spec.rb
+++ b/spec/sidekiq_cluster/sidekiq_cluster_spec.rb
@@ -35,7 +35,7 @@
expected_options = {
env: :development,
directory: an_instance_of(String),
- max_concurrency: 50,
+ max_concurrency: 20,
min_concurrency: 0,
worker_id: an_instance_of(Integer),
timeout: 25,
--
GitLab