Merge branch 'tidy-up-gitlab-database' into 'master'

Remove Gitlab::Database.postgresql_9_or_less? method

See merge request gitlab-org/gitlab!43564

app/models/postgresql/replication_slot.rb

@@ -22,8 +22,8 @@ module Postgresql
     def self.lag_too_great?(max = 100.megabytes)
       return false unless in_use?
 
-      lag_function = "#{Gitlab::Database.pg_wal_lsn_diff}" \
-        "(#{Gitlab::Database.pg_current_wal_insert_lsn}(), restart_lsn)::bigint"
+      lag_function = "pg_wal_lsn_diff" \
+        "(pg_current_wal_insert_lsn(), restart_lsn)::bigint"
 
       # We force the use of a transaction here so the query always goes to the
       # primary, even when using the EE DB load balancer.

ee/app/models/pg_replication_slot.rb

@@ -23,7 +23,7 @@ class PgReplicationSlot
   def self.slots_retained_bytes
     ApplicationRecord.connection.execute(<<-SQL.squish)
       SELECT slot_name, database,
-             active, #{Gitlab::Database.pg_wal_lsn_diff}(#{Gitlab::Database.pg_current_wal_insert_lsn}(), restart_lsn)
+             active, pg_wal_lsn_diff(pg_current_wal_insert_lsn(), restart_lsn)
         AS retained_bytes
         FROM pg_replication_slots;
     SQL
@@ -33,7 +33,7 @@ class PgReplicationSlot
   # returns the max number WAL space (in bytes) being used across the replication slots
   def self.max_retained_wal
     ApplicationRecord.connection.execute(<<-SQL.squish)
-      SELECT COALESCE(MAX(#{Gitlab::Database.pg_wal_lsn_diff}(#{Gitlab::Database.pg_current_wal_insert_lsn}(), restart_lsn)), 0)
+      SELECT COALESCE(MAX(pg_wal_lsn_diff(pg_current_wal_insert_lsn(), restart_lsn)), 0)
         FROM pg_replication_slots;
     SQL
     .first.fetch('coalesce').to_i

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

@@ -148,7 +148,7 @@ module Gitlab
         def replication_lag_size
           location = connection.quote(primary_write_location)
           row = query_and_release(<<-SQL.squish)
-            SELECT #{Gitlab::Database.pg_wal_lsn_diff}(#{location}, #{Gitlab::Database.pg_last_wal_replay_lsn}())::float
+            SELECT pg_wal_lsn_diff(#{location}, pg_last_wal_replay_lsn())::float
               AS diff
           SQL
 
@@ -175,7 +175,7 @@ module Gitlab
           # such a case.
           query = <<-SQL.squish
             SELECT NOT pg_is_in_recovery()
-              OR #{Gitlab::Database.pg_wal_lsn_diff}(#{Gitlab::Database.pg_last_wal_replay_lsn}(), #{string}) >= 0
+              OR pg_wal_lsn_diff(pg_last_wal_replay_lsn(), #{string}) >= 0
               AS result
           SQL
 

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

@@ -107,7 +107,7 @@ module Gitlab
         def primary_write_location
           read_write do |connection|
             row = connection
-              .select_all("SELECT #{Gitlab::Database.pg_current_wal_insert_lsn}()::text AS location")
+              .select_all("SELECT pg_current_wal_insert_lsn()::text AS location")
               .first
 
             if row

ee/lib/gitlab/geo/health_check.rb

@@ -48,10 +48,10 @@ module Gitlab
       def db_replication_lag_seconds_query
         <<-SQL.squish
           SELECT CASE
-            WHEN #{Gitlab::Database.pg_last_wal_receive_lsn}() = #{Gitlab::Database.pg_last_wal_replay_lsn}()
+            WHEN pg_last_wal_receive_lsn() = pg_last_wal_replay_lsn()
               THEN 0
             ELSE
-              EXTRACT (EPOCH FROM now() - #{Gitlab::Database.pg_last_xact_replay_timestamp}())::INTEGER
+              EXTRACT (EPOCH FROM now() - pg_last_xact_replay_timestamp())::INTEGER
             END
             AS replication_lag
         SQL
@@ -108,7 +108,7 @@ module Gitlab
 
       def streaming_replication_enabled?
         !ActiveRecord::Base.connection
-          .execute("SELECT * FROM #{Gitlab::Database.pg_last_wal_receive_lsn}() as result")
+          .execute("SELECT * FROM pg_last_wal_receive_lsn() as result")
           .first['result']
           .nil?
       end
@@ -116,7 +116,7 @@ module Gitlab
       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")
+          .execute("SELECT * FROM pg_last_xact_replay_timestamp() as result")
           .first['result']
           .nil?
       end

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

@@ -66,8 +66,7 @@ RSpec.describe Gitlab::Geo::HealthCheck, :geo do
 
         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.execute').with(no_args).with('SELECT * FROM pg_last_wal_receive_lsn() 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/)
@@ -78,7 +77,6 @@ RSpec.describe Gitlab::Geo::HealthCheck, :geo 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/)

lib/gitlab/database.rb

@@ -92,10 +92,6 @@ module Gitlab
       @version ||= database_version.match(/\A(?:PostgreSQL |)([^\s]+).*\z/)[1]
     end
 
-    def self.postgresql_9_or_less?
-      version.to_f < 10
-    end
-
     def self.postgresql_minimum_supported_version?
       version.to_f >= MINIMUM_POSTGRES_VERSION
     end
@@ -127,28 +123,6 @@ module Gitlab
       # ignore - happens when Rake tasks yet have to create a database, e.g. for testing
     end
 
-    # map some of the function names that changed between PostgreSQL 9 and 10
-    # https://wiki.postgresql.org/wiki/New_in_postgres_10
-    def self.pg_wal_lsn_diff
-      Gitlab::Database.postgresql_9_or_less? ? 'pg_xlog_location_diff' : 'pg_wal_lsn_diff'
-    end
-
-    def self.pg_current_wal_insert_lsn
-      Gitlab::Database.postgresql_9_or_less? ? 'pg_current_xlog_insert_location' : 'pg_current_wal_insert_lsn'
-    end
-
-    def self.pg_last_wal_receive_lsn
-      Gitlab::Database.postgresql_9_or_less? ? 'pg_last_xlog_receive_location' : 'pg_last_wal_receive_lsn'
-    end
-
-    def self.pg_last_wal_replay_lsn
-      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')
       Arel.sql("#{field} #{direction} NULLS LAST")
     end

spec/lib/gitlab/database_spec.rb

@@ -70,25 +70,6 @@ RSpec.describe Gitlab::Database do
     end
   end
 
-  describe '.postgresql_9_or_less?' do
-    it 'returns true when using postgresql 8.4' do
-      allow(described_class).to receive(:version).and_return('8.4')
-      expect(described_class.postgresql_9_or_less?).to eq(true)
-    end
-
-    it 'returns true when using PostgreSQL 9.6' do
-      allow(described_class).to receive(:version).and_return('9.6')
-
-      expect(described_class.postgresql_9_or_less?).to eq(true)
-    end
-
-    it 'returns false when using PostgreSQL 10 or newer' do
-      allow(described_class).to receive(:version).and_return('10')
-
-      expect(described_class.postgresql_9_or_less?).to eq(false)
-    end
-  end
-
   describe '.postgresql_minimum_supported_version?' do
     it 'returns false when using PostgreSQL 10' do
       allow(described_class).to receive(:version).and_return('10')
@@ -150,68 +131,6 @@ RSpec.describe Gitlab::Database do
     end
   end
 
-  describe '.pg_wal_lsn_diff' do
-    it 'returns old name when using PostgreSQL 9.6' do
-      allow(described_class).to receive(:version).and_return('9.6')
-
-      expect(described_class.pg_wal_lsn_diff).to eq('pg_xlog_location_diff')
-    end
-
-    it 'returns new name when using PostgreSQL 10 or newer' do
-      allow(described_class).to receive(:version).and_return('10')
-
-      expect(described_class.pg_wal_lsn_diff).to eq('pg_wal_lsn_diff')
-    end
-  end
-
-  describe '.pg_current_wal_insert_lsn' do
-    it 'returns old name when using PostgreSQL 9.6' do
-      allow(described_class).to receive(:version).and_return('9.6')
-
-      expect(described_class.pg_current_wal_insert_lsn).to eq('pg_current_xlog_insert_location')
-    end
-
-    it 'returns new name when using PostgreSQL 10 or newer' do
-      allow(described_class).to receive(:version).and_return('10')
-
-      expect(described_class.pg_current_wal_insert_lsn).to eq('pg_current_wal_insert_lsn')
-    end
-  end
-
-  describe '.pg_last_wal_receive_lsn' do
-    it 'returns old name when using PostgreSQL 9.6' do
-      allow(described_class).to receive(:version).and_return('9.6')
-
-      expect(described_class.pg_last_wal_receive_lsn).to eq('pg_last_xlog_receive_location')
-    end
-
-    it 'returns new name when using PostgreSQL 10 or newer' do
-      allow(described_class).to receive(:version).and_return('10')
-
-      expect(described_class.pg_last_wal_receive_lsn).to eq('pg_last_wal_receive_lsn')
-    end
-  end
-
-  describe '.pg_last_wal_replay_lsn' do
-    it 'returns old name when using PostgreSQL 9.6' do
-      allow(described_class).to receive(:version).and_return('9.6')
-
-      expect(described_class.pg_last_wal_replay_lsn).to eq('pg_last_xlog_replay_location')
-    end
-
-    it 'returns new name when using PostgreSQL 10 or newer' do
-      allow(described_class).to receive(:version).and_return('10')
-
-      expect(described_class.pg_last_wal_replay_lsn).to eq('pg_last_wal_replay_lsn')
-    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
     it { expect(described_class.nulls_last_order('column', 'ASC')).to eq 'column ASC NULLS LAST'}
     it { expect(described_class.nulls_last_order('column', 'DESC')).to eq 'column DESC NULLS LAST'}