Commit c0536ea6 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Removes the DB_USE_NEW_POOL_SIZE_LOGIC flag

This removes the ENV flag for trying out new database config. This has
been running on all Puma en Sidekiq VMs on GitLab.com.
parent fc182138
---
title: Automatically calculate the database connection pool size
merge_request: 38049
author:
type: other
...@@ -20,64 +20,34 @@ Gitlab.ee do ...@@ -20,64 +20,34 @@ Gitlab.ee do
end end
end end
# TODO get rid of feature flag https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/495 # Because of the way Ruby on Rails manages database connections, it is
if ENV['DB_USE_NEW_POOL_SIZE_LOGIC'] == '1' # important that we have at least as many connections as we have
# Because of the way Ruby on Rails manages database connections, it is # threads. While there is a 'pool' setting in database.yml, it is not
# important that we have at least as many connections as we have # very practical because you need to maintain it in tandem with the
# threads. While there is a 'pool' setting in database.yml, it is not # number of application threads. Because of this we override the number
# very practical because you need to maintain it in tandem with the # of allowed connections in the database connection pool based on the
# number of application threads. Because of this we override the number # configured number of application threads.
# of allowed connections in the database connection pool based on the #
# configured number of application threads. # Gitlab::Runtime.max_threads is the number of "user facing" application
# # threads the process has been configured with. We also have auxiliary
# Gitlab::Runtime.max_threads is the number of "user facing" application # threads that use database connections. Because it is not practical to
# threads the process has been configured with. We also have auxiliary # keep an accurate count of the number auxiliary threads as the
# threads that use database connections. Because it is not practical to # application evolves over time, we just add a fixed headroom to the
# keep an accurate count of the number auxiliary threads as the # number of user-facing threads. It is OK if this number is too large
# application evolves over time, we just add a fixed headroom to the # because connections are instantiated lazily.
# number of user-facing threads. It is OK if this number is too large
# because connections are instantiated lazily. headroom = (ENV["DB_POOL_HEADROOM"].presence || 10).to_i
calculated_pool_size = Gitlab::Runtime.max_threads + headroom
headroom = (ENV["DB_POOL_HEADROOM"].presence || 10).to_i
calculated_pool_size = Gitlab::Runtime.max_threads + headroom db_config = Gitlab::Database.config ||
db_config = Gitlab::Database.config ||
Rails.application.config.database_configuration[Rails.env] Rails.application.config.database_configuration[Rails.env]
db_config['pool'] = calculated_pool_size db_config['pool'] = calculated_pool_size
ActiveRecord::Base.establish_connection(db_config) ActiveRecord::Base.establish_connection(db_config)
Gitlab.ee do Gitlab.ee do
if Gitlab::Runtime.sidekiq? && Gitlab::Geo.geo_database_configured? if Gitlab::Runtime.sidekiq? && Gitlab::Geo.geo_database_configured?
Rails.configuration.geo_database['pool'] = calculated_pool_size Rails.configuration.geo_database['pool'] = calculated_pool_size
Geo::TrackingBase.establish_connection(Rails.configuration.geo_database) Geo::TrackingBase.establish_connection(Rails.configuration.geo_database)
end end
end
elsif Gitlab::Runtime.multi_threaded?
# When running on multi-threaded runtimes like Puma or Sidekiq,
# set the number of threads per process as the minimum DB connection pool size.
# This is to avoid connectivity issues as was documented here:
# https://github.com/rails/rails/pull/23057
max_threads = Gitlab::Runtime.max_threads
db_config = Gitlab::Database.config ||
Rails.application.config.database_configuration[Rails.env]
previous_db_pool_size = db_config['pool']
db_config['pool'] = [db_config['pool'].to_i, max_threads].max + ENV["DB_POOL_HEADROOM"].to_i
ActiveRecord::Base.establish_connection(db_config)
current_db_pool_size = ActiveRecord::Base.connection.pool.size
log_pool_size('DB', previous_db_pool_size, current_db_pool_size)
Gitlab.ee do
if Gitlab::Runtime.sidekiq? && Gitlab::Geo.geo_database_configured?
previous_geo_db_pool_size = Rails.configuration.geo_database['pool']
Rails.configuration.geo_database['pool'] = max_threads
Geo::TrackingBase.establish_connection(Rails.configuration.geo_database)
current_geo_db_pool_size = Geo::TrackingBase.connection_pool.size
log_pool_size('Geo DB', previous_geo_db_pool_size, current_geo_db_pool_size)
end
end
end end
...@@ -7,43 +7,9 @@ RSpec.describe 'Database config initializer for GitLab EE' do ...@@ -7,43 +7,9 @@ RSpec.describe 'Database config initializer for GitLab EE' do
load Rails.root.join('config/initializers/database_config.rb') load Rails.root.join('config/initializers/database_config.rb')
end end
before do
stub_geo_database_config(pool_size: 1)
end
context "when using multi-threaded runtime" do
let(:max_threads) { 8 }
before do
allow(Gitlab::Runtime).to receive(:multi_threaded?).and_return(true)
allow(Gitlab::Runtime).to receive(:max_threads).and_return(max_threads)
allow(ActiveRecord::Base).to receive(:establish_connection)
expect(Geo::TrackingBase).to receive(:establish_connection)
end
context "and the runtime is Sidekiq" do
before do
allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true)
end
it "sets Geo DB connection pool size to the max number of worker threads" do
expect { subject }.to change { Rails.configuration.geo_database['pool'] }.from(1).to(max_threads)
end
end
end
context "when using single-threaded runtime" do
it "does nothing" do
expect { subject }.not_to change { Rails.configuration.geo_database['pool'] }
end
end
context "with new pool size logic" do
let(:max_threads) { 8 } let(:max_threads) { 8 }
before do before do
stub_env('DB_USE_NEW_POOL_SIZE_LOGIC', '1')
allow(Gitlab::Runtime).to receive(:max_threads).and_return(max_threads) allow(Gitlab::Runtime).to receive(:max_threads).and_return(max_threads)
allow(ActiveRecord::Base).to receive(:establish_connection) allow(ActiveRecord::Base).to receive(:establish_connection)
...@@ -52,6 +18,7 @@ RSpec.describe 'Database config initializer for GitLab EE' do ...@@ -52,6 +18,7 @@ RSpec.describe 'Database config initializer for GitLab EE' do
context "and the runtime is Sidekiq" do context "and the runtime is Sidekiq" do
before do before do
stub_geo_database_config(pool_size: 1)
allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true) allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true)
end end
...@@ -59,7 +26,6 @@ RSpec.describe 'Database config initializer for GitLab EE' do ...@@ -59,7 +26,6 @@ RSpec.describe 'Database config initializer for GitLab EE' do
expect { subject }.to change { Rails.configuration.geo_database['pool'] }.from(1).to(18) expect { subject }.to change { Rails.configuration.geo_database['pool'] }.from(1).to(18)
end end
end end
end
def stub_geo_database_config(pool_size:) def stub_geo_database_config(pool_size:)
config = { config = {
......
...@@ -9,77 +9,12 @@ RSpec.describe 'Database config initializer' do ...@@ -9,77 +9,12 @@ RSpec.describe 'Database config initializer' do
before do before do
allow(ActiveRecord::Base).to receive(:establish_connection) allow(ActiveRecord::Base).to receive(:establish_connection)
end
context "when using multi-threaded runtime" do
let(:max_threads) { 8 }
before do
allow(Gitlab::Runtime).to receive(:multi_threaded?).and_return(true)
allow(Gitlab::Runtime).to receive(:max_threads).and_return(max_threads) allow(Gitlab::Runtime).to receive(:max_threads).and_return(max_threads)
end end
context "and no existing pool size is set" do
before do
stub_database_config(pool_size: nil)
end
it "sets it to the max number of worker threads" do
expect { subject }.to change { Gitlab::Database.config['pool'] }.from(nil).to(max_threads)
end
end
context "and the existing pool size is smaller than the max number of worker threads" do
before do
stub_database_config(pool_size: max_threads - 1)
end
it "sets it to the max number of worker threads" do
expect { subject }.to change { Gitlab::Database.config['pool'] }.by(1)
end
end
context "and the existing pool size is larger than the max number of worker threads" do
before do
stub_database_config(pool_size: max_threads + 1)
end
it "keeps the configured pool size" do
expect { subject }.not_to change { Gitlab::Database.config['pool'] }
end
end
context "when specifying headroom through an ENV variable" do
let(:headroom) { 10 }
before do
stub_database_config(pool_size: 1)
stub_env("DB_POOL_HEADROOM", headroom)
end
it "adds headroom on top of the calculated size" do
expect { subject }.to change { Gitlab::Database.config['pool'] }
.from(1)
.to(max_threads + headroom)
end
end
end
context "when using single-threaded runtime" do
it "does nothing" do
expect { subject }.not_to change { Gitlab::Database.config['pool'] }
end
end
context "with new pool size logic" do
let(:max_threads) { 8 } let(:max_threads) { 8 }
before do context "no existing pool size is set" do
stub_env('DB_USE_NEW_POOL_SIZE_LOGIC', '1')
allow(Gitlab::Runtime).to receive(:max_threads).and_return(max_threads)
end
context "and no existing pool size is set" do
before do before do
stub_database_config(pool_size: nil) stub_database_config(pool_size: nil)
end end
...@@ -89,7 +24,7 @@ RSpec.describe 'Database config initializer' do ...@@ -89,7 +24,7 @@ RSpec.describe 'Database config initializer' do
end end
end end
context "and the existing pool size is smaller than the max number of worker threads" do context "the existing pool size is smaller than the max number of worker threads" do
before do before do
stub_database_config(pool_size: 1) stub_database_config(pool_size: 1)
end end
...@@ -123,7 +58,6 @@ RSpec.describe 'Database config initializer' do ...@@ -123,7 +58,6 @@ RSpec.describe 'Database config initializer' do
.to(max_threads + headroom) .to(max_threads + headroom)
end end
end end
end
def stub_database_config(pool_size:) def stub_database_config(pool_size:)
config = { config = {
......
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