Merge branch...

Merge branch '5933-geo-inconsistent-database-replication-status-when-wal-e-streaming-is-in-use-2' into 'master'

Geo: Support archive recovery or streaming replication types

Closes #5933

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

ee/app/controllers/admin/geo/nodes_controller.rb

@@ -15,7 +15,7 @@ class Admin::Geo::NodesController < Admin::ApplicationController
       flash.now[:alert] = _('You need a different license to enable Geo replication.')
     end
 
-    unless Gitlab::Database.pg_stat_wal_receiver_supported?
+    unless Gitlab::Database.postgresql_minimum_supported_version?
       flash.now[:warning] = _('Please upgrade PostgreSQL to version 9.6 or greater. The status of the replication cannot be determined reliably with the current version.')
     end
   end

ee/changelogs/unreleased/5933-geo-inconsistent-database-replication-status-when-wal-e-streaming-is-in-use-2.yml

+---
+title: 'Geo: Support archive recovery or streaming replication types in health check'
+merge_request: 9935
+author:
+type: fixed

ee/lib/gitlab/geo/health_check.rb

@@ -10,9 +10,9 @@ module Gitlab
 
         return '' unless Gitlab::Geo.secondary?
         return 'Geo database configuration file is missing.' unless Gitlab::Geo.geo_database_configured?
-        return 'Geo node has a database that is not configured for streaming replication with the primary node.' unless Gitlab::Database.db_read_only?
-        return 'Geo node does not appear to be replicating the database from the primary node.' if Gitlab::Database.pg_stat_wal_receiver_supported? && !streaming_active?
-        return "Current Geo database version (#{database_version}) does not match latest migration (#{migration_version}).\nYou may have to run `gitlab-rake geo:db:migrate` as root on the secondary." unless database_migration_version_match?
+        return 'Geo node has a database that is writable which is an indication it is not configured for replication with the primary node.' unless Gitlab::Database.db_read_only?
+        return 'Geo node does not appear to be replicating the database from the primary node.' if replication_enabled? && !replication_working?
+        return "Geo database version (#{database_version}) does not match latest migration (#{migration_version}).\nYou may have to run `gitlab-rake geo:db:migrate` as root on the secondary." unless database_migration_version_match?
         return 'Geo database is not configured to use Foreign Data Wrapper.' unless Gitlab::Geo::Fdw.enabled?
 
         unless Gitlab::Geo::Fdw.foreign_tables_up_to_date?
@@ -42,7 +42,7 @@ module Gitlab
             WHEN #{Gitlab::Database.pg_last_wal_receive_lsn}() = #{Gitlab::Database.pg_last_wal_replay_lsn}()
               THEN 0
             ELSE
-              EXTRACT (EPOCH FROM now() - pg_last_xact_replay_timestamp())::INTEGER
+              EXTRACT (EPOCH FROM now() - #{Gitlab::Database.pg_last_xact_replay_timestamp}())::INTEGER
             END
             AS replication_lag
           SQL
@@ -105,12 +105,40 @@ module Gitlab
         gitlab_schema_tables_count == foreign_schema_tables_count
       end
 
-      def streaming_active?
-        # Get a streaming status
+      def replication_enabled?
+        streaming_replication_enabled? || archive_recovery_replication_enabled?
+      end
+
+      def archive_recovery_replication_enabled?
+        !streaming_replication_enabled? && some_replication_active?
+      end
+
+      def streaming_replication_enabled?
+        !ActiveRecord::Base.connection
+          .execute("SELECT * FROM #{Gitlab::Database.pg_last_wal_receive_lsn}() as result")
+          .first['result']
+          .nil?
+      end
+
+      def some_replication_active?
+        # Is some sort of replication active?
+        !ActiveRecord::Base.connection
+          .execute("SELECT * FROM #{Gitlab::Database.pg_last_xact_replay_timestamp}() as result")
+          .first['result']
+          .nil?
+      end
+
+      def streaming_replication_active?
         # This only works for Postgresql 9.6 and greater
         ActiveRecord::Base.connection
           .select_values('SELECT pid FROM pg_stat_wal_receiver').first.to_i > 0
       end
+
+      def replication_working?
+        return streaming_replication_active? if streaming_replication_enabled?
+
+        some_replication_active?
+      end
     end
   end
 end

ee/lib/tasks/geo.rake

@@ -220,7 +220,7 @@ namespace :geo do
     puts GeoNode.current_node_url
     puts '-----------------------------------------------------'.color(:yellow)
 
-    unless Gitlab::Database.pg_stat_wal_receiver_supported?
+    unless Gitlab::Database.postgresql_minimum_supported_version?
       puts
       puts 'WARNING: Please upgrade PostgreSQL to version 9.6 or greater. The status of the replication cannot be determined reliably with the current version.'.color(:red)
       puts

ee/spec/controllers/admin/geo/nodes_controller_spec.rb

@@ -65,7 +65,7 @@ describe Admin::Geo::NodesController, :postgresql do
 
     context 'with Postgres 9.6 or greater' do
       before do
-        allow(Gitlab::Database).to receive(:pg_stat_wal_receiver_supported?).and_return(true)
+        allow(Gitlab::Database).to receive(:postgresql_minimum_supported_version?).and_return(true)
       end
 
       it_behaves_like 'no flash message', :warning
@@ -73,7 +73,7 @@ describe Admin::Geo::NodesController, :postgresql do
 
     context 'without Postgres 9.6 or greater' do
       before do
-        allow(Gitlab::Database).to receive(:pg_stat_wal_receiver_supported?).and_return(false)
+        allow(Gitlab::Database).to receive(:postgresql_minimum_supported_version?).and_return(false)
       end
 
       it_behaves_like 'with flash message', :warning, 'Please upgrade PostgreSQL to version 9.6 or greater.'

ee/spec/lib/gitlab/geo/health_check_spec.rb

@@ -61,26 +61,48 @@ describe Gitlab::Geo::HealthCheck, :geo do
           let(:db_read_only) { false }
 
           it 'returns an error' do
-            expect(subject.perform_checks).to include('Geo node has a database that is not configured for streaming replication with the primary node.')
+            expect(subject.perform_checks).to include('Geo node has a database that is writable which is an indication it is not configured for replication with the primary node.')
           end
         end
 
         context 'streaming replication' do
+          it 'returns an error when replication is not working' do
+            allow(Gitlab::Database).to receive(:pg_last_wal_receive_lsn).and_return('pg_last_xlog_receive_location')
+            allow(ActiveRecord::Base).to receive_message_chain('connection.execute').with(no_args).with('SELECT * FROM pg_last_xlog_receive_location() as result').and_return(['result' => 'fake'])
+            allow(ActiveRecord::Base).to receive_message_chain('connection.select_values').with(no_args).with('SELECT pid FROM pg_stat_wal_receiver').and_return([])
+
+            expect(subject.perform_checks).to match(/Geo node does not appear to be replicating the database from the primary node/)
+          end
+        end
+
+        context 'archive recovery replication' do
+          it 'returns an error when replication is not working' do
+            allow(subject).to receive(:streaming_replication_enabled?).and_return(false)
+            allow(subject).to receive(:archive_recovery_replication_enabled?).and_return(true)
+            allow(Gitlab::Database).to receive(:pg_last_xact_replay_timestamp).and_return('pg_last_xact_replay_timestamp')
+            allow(ActiveRecord::Base).to receive_message_chain('connection.execute').with(no_args).with('SELECT * FROM pg_last_xact_replay_timestamp() as result').and_return([{ 'result' => nil }])
+
+            expect(subject.perform_checks).to match(/Geo node does not appear to be replicating the database from the primary node/)
+          end
+        end
+
+        context 'some sort of replication' do
           before do
-            allow(Gitlab::Database).to receive(:pg_stat_wal_receiver_supported?).and_return(true)
+            allow(subject).to receive(:replication_enabled?).and_return(true)
           end
 
-          context 'that is supported but not working' do
+          context 'that is not working' do
             it 'returns an error' do
-              allow(ActiveRecord::Base).to receive_message_chain('connection.select_values').with(no_args).with('SELECT pid FROM pg_stat_wal_receiver').and_return([])
+              allow(subject).to receive(:archive_recovery_replication_enabled?).and_return(false)
+              allow(subject).to receive(:streaming_replication_enabled?).and_return(false)
 
               expect(subject.perform_checks).to match(/Geo node does not appear to be replicating the database from the primary node/)
             end
           end
 
-          context 'that is supported and working' do
+          context 'that is working' do
             before do
-              allow(ActiveRecord::Base).to receive_message_chain('connection.select_values').with(no_args).with('SELECT pid FROM pg_stat_wal_receiver').and_return(['123'])
+              allow(subject).to receive(:replication_working?).and_return(true)
               allow(Gitlab::Geo::Fdw).to receive(:enabled?) { true }
               allow(Gitlab::Geo::Fdw).to receive(:foreign_tables_up_to_date?) { true }
             end

lib/gitlab/database.rb

@@ -80,7 +80,7 @@ module Gitlab
       postgresql? && version.to_f >= 9.4
     end
 
-    def self.pg_stat_wal_receiver_supported?
+    def self.postgresql_minimum_supported_version?
       postgresql? && version.to_f >= 9.6
     end
 
@@ -102,6 +102,10 @@ module Gitlab
       Gitlab::Database.postgresql_9_or_less? ? 'pg_last_xlog_replay_location' : 'pg_last_wal_replay_lsn'
     end
 
+    def self.pg_last_xact_replay_timestamp
+      'pg_last_xact_replay_timestamp'
+    end
+
     def self.nulls_last_order(field, direction = 'ASC')
       order = "#{field} #{direction}"
 

spec/lib/gitlab/database_spec.rb

@@ -87,6 +87,38 @@ describe Gitlab::Database do
     end
   end
 
+  describe '.postgresql_minimum_supported_version?' do
+    it 'returns false when not using PostgreSQL' do
+      allow(described_class).to receive(:postgresql?).and_return(false)
+
+      expect(described_class.postgresql_minimum_supported_version?).to eq(false)
+    end
+
+    context 'when using PostgreSQL' do
+      before do
+        allow(described_class).to receive(:postgresql?).and_return(true)
+      end
+
+      it 'returns false when using PostgreSQL 9.5' do
+        allow(described_class).to receive(:version).and_return('9.5')
+
+        expect(described_class.postgresql_minimum_supported_version?).to eq(false)
+      end
+
+      it 'returns true when using PostgreSQL 9.6' do
+        allow(described_class).to receive(:version).and_return('9.6')
+
+        expect(described_class.postgresql_minimum_supported_version?).to eq(true)
+      end
+
+      it 'returns true when using PostgreSQL 10 or newer' do
+        allow(described_class).to receive(:version).and_return('10')
+
+        expect(described_class.postgresql_minimum_supported_version?).to eq(true)
+      end
+    end
+  end
+
   describe '.join_lateral_supported?' do
     it 'returns false when using MySQL' do
       allow(described_class).to receive(:postgresql?).and_return(false)
@@ -195,6 +227,12 @@ describe Gitlab::Database do
     end
   end
 
+  describe '.pg_last_xact_replay_timestamp' do
+    it 'returns pg_last_xact_replay_timestamp' do
+      expect(described_class.pg_last_xact_replay_timestamp).to eq('pg_last_xact_replay_timestamp')
+    end
+  end
+
   describe '.nulls_last_order' do
     context 'when using PostgreSQL' do
       before do