Commit 76ac934c authored by Yorick Peterse's avatar Yorick Peterse

Remove caching of Connection#config

Caching was added in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65262, which lead
to an incident and support for using uncached configs in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66988.

In this commit we remove caching of the config settings, and add a test
and comment to explain why we don't want the data to be cached. The
performance impact is likely not worth potentially breaking code that
expects uncached settings.

This fixes https://gitlab.com/gitlab-org/gitlab/-/issues/337054.
parent 1b348769
......@@ -34,12 +34,17 @@ module Gitlab
Gitlab::Runtime.max_threads + headroom
end
def uncached_config
scope.connection_db_config.configuration_hash.with_indifferent_access
end
def config
@config ||= uncached_config
# The result of this method must not be cached, as other methods may use
# it after making configuration changes and expect those changes to be
# present. For example, `disable_prepared_statements` expects the
# configuration settings to always be up to date.
#
# See the following for more information:
#
# - https://gitlab.com/gitlab-org/release/retrospectives/-/issues/39
# - https://gitlab.com/gitlab-com/gl-infra/production/-/issues/5238
scope.connection_db_config.configuration_hash.with_indifferent_access
end
def pool_size
......@@ -72,7 +77,7 @@ module Gitlab
# Disables prepared statements for the current database connection.
def disable_prepared_statements
scope.establish_connection(uncached_config.merge(prepared_statements: false))
scope.establish_connection(config.merge(prepared_statements: false))
end
def read_only?
......
......@@ -29,12 +29,19 @@ RSpec.describe Gitlab::Database::Connection do
it 'returns a default pool size' do
expect(connection.config).to include(pool: connection.default_pool_size)
end
it 'does not cache its results' do
a = connection.config
b = connection.config
expect(a).not_to equal(b)
end
end
describe '#pool_size' do
context 'when no explicit size is configured' do
it 'returns the default pool size' do
expect(connection.config).to receive(:[]).with(:pool).and_return(nil)
expect(connection).to receive(:config).and_return({ pool: nil })
expect(connection.pool_size).to eq(connection.default_pool_size)
end
......@@ -42,7 +49,7 @@ RSpec.describe Gitlab::Database::Connection do
context 'when an explicit pool size is set' do
it 'returns the pool size' do
expect(connection.config).to receive(:[]).with(:pool).and_return(4)
expect(connection).to receive(:config).and_return({ pool: 4 })
expect(connection.pool_size).to eq(4)
end
......
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