Commit cf3a0e35 authored by Yorick Peterse's avatar Yorick Peterse Committed by Yorick Peterse

Handle ConnectionNotEstablished in the DB LB

The database load balancer would re-raise ConnectionNotEstablished
errors when encountering them. This can lead to user facing errors
whenever the application tries to connect to a new database that is not
(yet) online.

This was discovered while testing the changes for
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68042, after
adding a replica host that didn't exist.

Changelog: fixed
parent f38900c9
...@@ -4,20 +4,19 @@ module Gitlab ...@@ -4,20 +4,19 @@ module Gitlab
module Database module Database
module LoadBalancing module LoadBalancing
# The exceptions raised for connection errors. # The exceptions raised for connection errors.
CONNECTION_ERRORS = if defined?(PG) CONNECTION_ERRORS = [
[ PG::ConnectionBad,
PG::ConnectionBad, PG::ConnectionDoesNotExist,
PG::ConnectionDoesNotExist, PG::ConnectionException,
PG::ConnectionException, PG::ConnectionFailure,
PG::ConnectionFailure, PG::UnableToSend,
PG::UnableToSend, # During a failover this error may be raised when
# During a failover this error may be raised when # writing to a primary.
# writing to a primary. PG::ReadOnlySqlTransaction,
PG::ReadOnlySqlTransaction # This error is raised when we can't connect to the database in the
].freeze # first place (e.g. it's offline or the hostname is incorrect).
else ActiveRecord::ConnectionNotEstablished
[].freeze ].freeze
end
def self.proxy def self.proxy
ActiveRecord::Base.load_balancing_proxy ActiveRecord::Base.load_balancing_proxy
......
...@@ -9,19 +9,12 @@ module Gitlab ...@@ -9,19 +9,12 @@ module Gitlab
delegate :connection, :release_connection, :enable_query_cache!, :disable_query_cache!, :query_cache_enabled, to: :pool delegate :connection, :release_connection, :enable_query_cache!, :disable_query_cache!, :query_cache_enabled, to: :pool
CONNECTION_ERRORS = CONNECTION_ERRORS = [
if defined?(PG) ActionView::Template::Error,
[ ActiveRecord::StatementInvalid,
ActionView::Template::Error, ActiveRecord::ConnectionNotEstablished,
ActiveRecord::StatementInvalid, PG::Error
PG::Error ].freeze
].freeze
else
[
ActionView::Template::Error,
ActiveRecord::StatementInvalid
].freeze
end
# host - The address of the database. # host - The address of the database.
# load_balancer - The LoadBalancer that manages this Host. # load_balancer - The LoadBalancer that manages this Host.
......
...@@ -172,6 +172,14 @@ RSpec.describe Gitlab::Database::LoadBalancing::Host do ...@@ -172,6 +172,14 @@ RSpec.describe Gitlab::Database::LoadBalancing::Host do
expect(host).not_to be_online expect(host).not_to be_online
end end
it 'returns false when ActiveRecord::ConnectionNotEstablished is raised' do
allow(host)
.to receive(:check_replica_status?)
.and_raise(ActiveRecord::ConnectionNotEstablished)
expect(host).not_to be_online
end
end end
end end
......
...@@ -277,31 +277,26 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do ...@@ -277,31 +277,26 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
end end
describe '#connection_error?' do describe '#connection_error?' do
before do
stub_const('Gitlab::Database::LoadBalancing::LoadBalancer::CONNECTION_ERRORS',
[NotImplementedError])
end
it 'returns true for a connection error' do it 'returns true for a connection error' do
error = NotImplementedError.new error = ActiveRecord::ConnectionNotEstablished.new
expect(lb.connection_error?(error)).to eq(true) expect(lb.connection_error?(error)).to eq(true)
end end
it 'returns true for a wrapped connection error' do it 'returns true for a wrapped connection error' do
wrapped = wrapped_exception(ActiveRecord::StatementInvalid, NotImplementedError) wrapped = wrapped_exception(ActiveRecord::StatementInvalid, ActiveRecord::ConnectionNotEstablished)
expect(lb.connection_error?(wrapped)).to eq(true) expect(lb.connection_error?(wrapped)).to eq(true)
end end
it 'returns true for a wrapped connection error from a view' do it 'returns true for a wrapped connection error from a view' do
wrapped = wrapped_exception(ActionView::Template::Error, NotImplementedError) wrapped = wrapped_exception(ActionView::Template::Error, ActiveRecord::ConnectionNotEstablished)
expect(lb.connection_error?(wrapped)).to eq(true) expect(lb.connection_error?(wrapped)).to eq(true)
end end
it 'returns true for deeply wrapped/nested errors' do it 'returns true for deeply wrapped/nested errors' do
top = twice_wrapped_exception(ActionView::Template::Error, ActiveRecord::StatementInvalid, NotImplementedError) top = twice_wrapped_exception(ActionView::Template::Error, ActiveRecord::StatementInvalid, ActiveRecord::ConnectionNotEstablished)
expect(lb.connection_error?(top)).to eq(true) expect(lb.connection_error?(top)).to eq(true)
end 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