Commit abbc8b8f authored by Yorick Peterse's avatar Yorick Peterse

Fix Connection#exists? when using the DB LB

Database::Connection#exists? would call `connection` to determine if
the database exists. Without load balancing enabled, this would call
`ActiveRecord::Base.connection`, which in turn calls
`ActiveRecord::Base.retrieve_connection`. If no connection could be
established or the database doesn't exist, this will raise an error.

When using the load balancer, the `Connection#connection` method instead
just returns a `ConnectionProxy`, without running any database queries.
The result is that `exists?` would return `true` even if the database
didn't exist.

To work around this, `Connection#exists?` obtains the database version
and caches this using Rails' schema cache. This ensures we run an actual
database query the first time, without running a query _every_ time.
Using the schema cache means we don't need to implement our own caching
logic.

Changelog: fixed
parent 4e503205
......@@ -174,8 +174,11 @@ module Gitlab
end
def exists?
connection
# We can't _just_ check if `connection` raises an error, as it will
# point to a `ConnectionProxy`, and obtaining those doesn't involve any
# database queries. So instead we obtain the database version, which is
# cached after the first call.
connection.schema_cache.database_version
true
rescue StandardError
false
......
......@@ -393,34 +393,28 @@ RSpec.describe Gitlab::Database::Connection do
end
describe '#cached_column_exists?' do
it 'only retrieves data once' do
expect(connection.scope.connection)
.to receive(:columns)
.once.and_call_original
2.times do
expect(connection.cached_column_exists?(:projects, :id)).to be_truthy
expect(connection.cached_column_exists?(:projects, :bogus_column)).to be_falsey
it 'only retrieves the data from the schema cache' do
queries = ActiveRecord::QueryRecorder.new do
2.times do
expect(connection.cached_column_exists?(:projects, :id)).to be_truthy
expect(connection.cached_column_exists?(:projects, :bogus_column)).to be_falsey
end
end
expect(queries.count).to eq(0)
end
end
describe '#cached_table_exists?' do
it 'only retrieves data once per table' do
expect(connection.scope.connection)
.to receive(:data_source_exists?)
.with(:projects)
.once.and_call_original
expect(connection.scope.connection)
.to receive(:data_source_exists?)
.with(:bogus_table_name)
.once.and_call_original
2.times do
expect(connection.cached_table_exists?(:projects)).to be_truthy
expect(connection.cached_table_exists?(:bogus_table_name)).to be_falsey
it 'only retrieves the data from the schema cache' do
queries = ActiveRecord::QueryRecorder.new do
2.times do
expect(connection.cached_table_exists?(:projects)).to be_truthy
expect(connection.cached_table_exists?(:bogus_table_name)).to be_falsey
end
end
expect(queries.count).to eq(0)
end
it 'returns false when database does not exist' do
......@@ -433,16 +427,14 @@ RSpec.describe Gitlab::Database::Connection do
end
describe '#exists?' do
it 'returns true if `ActiveRecord::Base.connection` succeeds' do
expect(connection.scope).to receive(:connection)
it 'returns true if the database exists' do
expect(connection.exists?).to be(true)
end
it 'returns false if `ActiveRecord::Base.connection` fails' do
expect(connection.scope).to receive(:connection) do
raise ActiveRecord::NoDatabaseError, 'broken'
end
it "returns false if the database doesn't exist" do
expect(connection.scope.connection.schema_cache)
.to receive(:database_version)
.and_raise(ActiveRecord::NoDatabaseError)
expect(connection.exists?).to be(false)
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