Merge branch 'load_balancing_log_lines' into 'master'

DB Load balancing: Log improvements

See merge request gitlab-org/gitlab-ee!15995

ee/changelogs/unreleased/load_balancing_log_lines.yml

+---
+title: Improve DB load balancing log to log host offline due to replication lag
+merge_request: 15995
+author:
+type: changed

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

@@ -74,7 +74,14 @@ module Gitlab
           if @online
             LoadBalancing::Logger.info(
               event: :host_online,
-              message: 'Host came back online',
+              message: 'Host is online after replica status check',
+              db_host: @host,
+              db_port: @port
+            )
+          else
+            LoadBalancing::Logger.warn(
+              event: :host_offline,
+              message: 'Host is offline after replica status check',
               db_host: @host,
               db_port: @port
             )

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

@@ -77,32 +77,67 @@ describe Gitlab::Database::LoadBalancing::Host do
     it 'marks the host as offline' do
       expect(host.pool).to receive(:disconnect!)
 
+      expect(Gitlab::Database::LoadBalancing::Logger).to receive(:warn)
+        .with(hash_including(event: :host_offline))
+        .and_call_original
+
       host.offline!
     end
   end
 
   describe '#online?' do
     context 'when the replica status is recent enough' do
+      before do
+        expect(host).to receive(:check_replica_status?).and_return(false)
+      end
+
       it 'returns the latest status' do
-        Timecop.freeze do
-          host = described_class.new('localhost', load_balancer)
+        expect(host).not_to receive(:refresh_status)
+        expect(Gitlab::Database::LoadBalancing::Logger).not_to receive(:info)
+        expect(Gitlab::Database::LoadBalancing::Logger).not_to receive(:warn)
 
-          expect(host).not_to receive(:refresh_status)
-          expect(host).to be_online
-        end
+        expect(host).to be_online
       end
-    end
 
-    context 'when the replica status is outdated' do
-      it 'refreshes the status' do
+      it 'returns an offline status' do
         host.offline!
 
+        expect(host).not_to receive(:refresh_status)
+        expect(Gitlab::Database::LoadBalancing::Logger).not_to receive(:info)
+        expect(Gitlab::Database::LoadBalancing::Logger).not_to receive(:warn)
+
+        expect(host).not_to be_online
+      end
+    end
+
+    context 'when the replica status is outdated' do
+      before do
         expect(host)
           .to receive(:check_replica_status?)
           .and_return(true)
+      end
+
+      it 'refreshes the status' do
+        expect(Gitlab::Database::LoadBalancing::Logger).to receive(:info)
+          .with(hash_including(event: :host_online))
+          .and_call_original
 
         expect(host).to be_online
       end
+
+      context 'and replica is not up to date' do
+        before do
+          expect(host).to receive(:replica_is_up_to_date?).and_return(false)
+        end
+
+        it 'marks the host offline' do
+          expect(Gitlab::Database::LoadBalancing::Logger).to receive(:warn)
+            .with(hash_including(event: :host_offline))
+            .and_call_original
+
+          expect(host).not_to be_online
+        end
+      end
     end
 
     context 'when the replica is not online' do
@@ -270,9 +305,7 @@ describe Gitlab::Database::LoadBalancing::Host do
 
   describe '#replication_lag_size' do
     it 'returns the lag size as an Integer' do
-      # On newer versions of Ruby the class is Integer, but on CI we run a
-      # version that still uses Fixnum.
-      expect([Fixnum, Integer]).to include(host.replication_lag_size.class) # rubocop: disable Lint/UnifiedInteger
+      expect(host.replication_lag_size).to be_an_instance_of(Integer)
     end
 
     it 'returns nil when the database query returned no rows' do