Catch errors in LoadBalancing::Host#online?

Prior to this commit it was possible for Host#online? to raise a
connection error when checking an already offline host. This could
happen because certain methods used for checking if a host is available
call the "connection" method, without catching any errors. As a result,
if this method raises then #online? would also raise. All of this could
lead to GitLab becoming unavailable depending on how many times it would
try to use this offline host.

To work around this we catch the errors that might be raised in
Host#online?. Unlike the code in LoadBalancing::LoadBalancer we don't
need to catch individual PG errors as we don't have to worry about
application errors. This means we only have to catch up to three errors:

1. ActionView::Template::Error (used inside views).

2. ActiveRecord::StatementInvalid (used basically everywhere outside of
   views).

3. PG::Error: used in some rare cases where Rails (for reasons unknown)
   doesn't wrap the original error.

ee/changelogs/unreleased/load-balancer-connection-bug.yml

+---
+title: Catch errors in LoadBalancing::Host#online?
+merge_request:
+author:
+type: fixed

ee/lib/gitlab/database/load_balancing/host.rb

@@ -7,6 +7,20 @@ module Gitlab
 
         delegate :connection, :release_connection, to: :pool
 
+        CONNECTION_ERRORS =
+          if defined?(PG)
+            [
+              ActionView::Template::Error,
+              ActiveRecord::StatementInvalid,
+              PG::Error
+            ].freeze
+          else
+            [
+              ActionView::Template::Error,
+              ActiveRecord::StatementInvalid
+            ].freeze
+          end
+
         # host - The address of the database.
         # load_balancer - The LoadBalancer that manages this Host.
         def initialize(host, load_balancer)
@@ -36,6 +50,9 @@ module Gitlab
           LoadBalancing.log(:info, "Host #{@host} came back online") if @online
 
           @online
+        rescue *CONNECTION_ERRORS
+          offline!
+          false
         end
 
         def refresh_status

ee/spec/lib/gitlab/database/load_balancing/host_spec.rb

@@ -59,6 +59,36 @@ describe Gitlab::Database::LoadBalancing::Host, :postgresql do
         expect(host).to be_online
       end
     end
+
+    context 'when the replica is not online' do
+      it 'returns false when ActionView::Template::Error is raised' do
+        error = StandardError.new
+
+        allow(host)
+          .to receive(:check_replica_status?)
+          .and_raise(ActionView::Template::Error.new('boom', error))
+
+        expect(host).not_to be_online
+      end
+
+      it 'returns false when ActiveRecord::StatementInvalid is raised' do
+        allow(host)
+          .to receive(:check_replica_status?)
+          .and_raise(ActiveRecord::StatementInvalid.new('foo'))
+
+        expect(host).not_to be_online
+      end
+
+      if Gitlab::Database.postgresql?
+        it 'returns false when PG::Error is raised' do
+          allow(host)
+            .to receive(:check_replica_status?)
+            .and_raise(PG::Error)
+
+          expect(host).not_to be_online
+        end
+      end
+    end
   end
 
   describe '#refresh_status' do