Don't use backup AR connections for Sidekiq

Adding two extra connections does nothing other than increasing the number of idle database connections. Given Sidekiq uses N threads it can never use more than N AR connections at a time, thus we don't need more. The initializer mentioned the Sidekiq upgrade guide stating this was required. This is false, the Sidekiq upgrade guide states this is necessary for Redis and not ActiveRecord. On GitLab.com this resulted in a reduction of about 80-100 PostgreSQL connections. Fixes #27713

Don't use backup AR connections for Sidekiq
f116f87c · Yorick Peterse · 53db7d1d · f116f87c · f116f87c
Unverified Commit f116f87c authored Feb 06, 2017 by Yorick Peterse
Hide whitespace changes
Inline Side-by-side

Showing with 5 additions and 3 deletions

remove-sidekiq-backup-ar-threads.yml changelogs/unreleased/remove-sidekiq-backup-ar-threads.yml +4 -0

sidekiq.rb config/initializers/sidekiq.rb +1 -3

No files found.
--- a/changelogs/unreleased/remove-sidekiq-backup-ar-threads.yml
+++ b/changelogs/unreleased/remove-sidekiq-backup-ar-threads.yml
+---
+title: Don't use backup Active Record connections for Sidekiq
+merge_request:
+author:
--- a/config/initializers/sidekiq.rb
+++ b/config/initializers/sidekiq.rb
@@ -36,11 +36,9 @@ Sidekiq.configure_server do |config|

  Gitlab::SidekiqThrottler.execute!

-  # Database pool should be at least `sidekiq_concurrency` + 2
-  # For more info, see: https://github.com/mperham/sidekiq/blob/master/4.0-Upgrade.md
  config = ActiveRecord::Base.configurations[Rails.env] ||
    Rails.application.config.database_configuration[Rails.env]
-  config['pool'] = Sidekiq.options[:concurrency] + 2
+  config['pool'] = Sidekiq.options[:concurrency]
  ActiveRecord::Base.establish_connection(config)
  Rails.logger.debug("Connection Pool size for Sidekiq Server is now: #{ActiveRecord::Base.connection.pool.instance_variable_get('@size')}")