Commit ed0fe440 authored by Gabriel Mazetto's avatar Gabriel Mazetto

Rewrote location query for load-balancer to use single query

Instead of relying on one query, catching and exception and then another
 fallback query, we now use a case statement which always work.
parent b36051f4
---
title: 'Geo: adds database load balancing support to work on standby clusters'
merge_request: 53147
author:
type: added
...@@ -135,8 +135,6 @@ module Gitlab ...@@ -135,8 +135,6 @@ module Gitlab
def primary_write_location def primary_write_location
location = read_write do |connection| location = read_write do |connection|
::Gitlab::Database.get_write_location(connection) ::Gitlab::Database.get_write_location(connection)
rescue ActiveRecord::StatementInvalid
::Gitlab::Database.get_replay_write_location(connection)
end end
return location if location return location if location
......
...@@ -243,8 +243,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do ...@@ -243,8 +243,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
end end
describe '#primary_write_location' do describe '#primary_write_location' do
it 'returns a String' do it 'returns a String in the right format' do
expect(lb.primary_write_location).to be_an_instance_of(String) expect(lb.primary_write_location).to match(/[A-E0-9]{1,8}\/[A-E0-9]{1,8}/)
end end
it 'raises an error if the write location could not be retrieved' do it 'raises an error if the write location could not be retrieved' do
...@@ -255,15 +255,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do ...@@ -255,15 +255,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
expect { lb.primary_write_location }.to raise_error(RuntimeError) expect { lb.primary_write_location }.to raise_error(RuntimeError)
end end
it 'fallbacks to #get_replay_write_location when #get_write_location raises error' do
connection = double(:connection)
allow(lb).to receive(:read_write).and_yield(connection)
allow(::Gitlab::Database).to receive(:get_write_location).and_raise(ActiveRecord::StatementInvalid)
expect(::Gitlab::Database).to receive(:get_replay_write_location).and_return('0/C73A0D88')
lb.primary_write_location
end
end end
describe '#all_caught_up?' do describe '#all_caught_up?' do
......
...@@ -260,17 +260,7 @@ module Gitlab ...@@ -260,17 +260,7 @@ module Gitlab
# @return [String] # @return [String]
def self.get_write_location(ar_connection) def self.get_write_location(ar_connection)
row = ar_connection row = ar_connection
.select_all("SELECT pg_current_wal_insert_lsn()::text AS location") .select_all('SELECT CASE WHEN pg_is_in_recovery() = true THEN pg_last_wal_replay_lsn()::text ELSE pg_current_wal_insert_lsn()::text END AS location;')
.first
row['location'] if row
end
# @param [ActiveRecord::Connection] ar_connection
# @return [String]
def self.get_replay_write_location(ar_connection)
row = ar_connection
.select_all("SELECT pg_last_wal_replay_lsn()::text AS location")
.first .first
row['location'] if row row['location'] if row
......
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