Commit 788373a1 authored by Simon Tomlinson's avatar Simon Tomlinson

Run serivce discovery on load balancing configuration

This solves an issue where load balancing would be configured but then
queries would run before service discovery ran. This could lead to
read queries falling back to the database primary.

See https://gitlab.com/gitlab-org/gitlab/-/issues/323726

Changelog: fixed
parent f320481d
...@@ -108,6 +108,11 @@ module Gitlab ...@@ -108,6 +108,11 @@ module Gitlab
# Configures proxying of requests. # Configures proxying of requests.
def self.configure_proxy(proxy = ConnectionProxy.new(hosts)) def self.configure_proxy(proxy = ConnectionProxy.new(hosts))
ActiveRecord::Base.load_balancing_proxy = proxy ActiveRecord::Base.load_balancing_proxy = proxy
# Populate service discovery immediately if it is configured
if service_discovery_enabled?
ServiceDiscovery.new(service_discovery_configuration).perform_service_discovery
end
end end
def self.active_record_models def self.active_record_models
......
...@@ -63,18 +63,14 @@ module Gitlab ...@@ -63,18 +63,14 @@ module Gitlab
end end
def start def start
# We run service discovery once in the current thread so that the application's main thread
# does not race this thread to use the results of initial service discovery.
next_sleep_duration = perform_service_discovery
Thread.new do Thread.new do
loop do loop do
next_sleep_duration = perform_service_discovery
# We slightly randomize the sleep() interval. This should reduce # We slightly randomize the sleep() interval. This should reduce
# the likelihood of _all_ processes refreshing at the same time, # the likelihood of _all_ processes refreshing at the same time,
# possibly putting unnecessary pressure on the DNS server. # possibly putting unnecessary pressure on the DNS server.
sleep(next_sleep_duration + rand(MAX_SLEEP_ADJUSTMENT)) sleep(next_sleep_duration + rand(MAX_SLEEP_ADJUSTMENT))
next_sleep_duration = perform_service_discovery
end end
end end
end end
......
...@@ -57,16 +57,13 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery do ...@@ -57,16 +57,13 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery do
.and_yield .and_yield
end end
it 'runs service discovery once before starting the worker thread' do it 'starts service discovery in a new thread' do
expect(service).to receive(:perform_service_discovery).ordered.and_return(5)
expect(Thread).to receive(:new).ordered.and_call_original # Thread starts expect(Thread).to receive(:new).ordered.and_call_original # Thread starts
expect(service).to receive(:perform_service_discovery).ordered.and_return(5)
expect(service).to receive(:rand).ordered.and_return(2) expect(service).to receive(:rand).ordered.and_return(2)
expect(service).to receive(:sleep).ordered.with(7) # Sleep runs after thread starts expect(service).to receive(:sleep).ordered.with(7) # Sleep runs after thread starts
expect(service).to receive(:perform_service_discovery).ordered.and_return(1)
service.start.join service.start.join
end end
end end
......
...@@ -212,6 +212,21 @@ RSpec.describe Gitlab::Database::LoadBalancing do ...@@ -212,6 +212,21 @@ RSpec.describe Gitlab::Database::LoadBalancing do
expect(ActiveRecord::Base).to have_received(:load_balancing_proxy=) expect(ActiveRecord::Base).to have_received(:load_balancing_proxy=)
.with(Gitlab::Database::LoadBalancing::ConnectionProxy) .with(Gitlab::Database::LoadBalancing::ConnectionProxy)
end end
context 'when service discovery is enabled' do
let(:service_discovery) { double(Gitlab::Database::LoadBalancing::ServiceDiscovery) }
it 'runs initial service discovery when configuring the connection proxy' do
allow(described_class)
.to receive(:configuration)
.and_return('discover' => { 'record' => 'foo' })
expect(Gitlab::Database::LoadBalancing::ServiceDiscovery).to receive(:new).and_return(service_discovery)
expect(service_discovery).to receive(:perform_service_discovery)
described_class.configure_proxy
end
end
end end
describe '.active_record_models' do describe '.active_record_models' do
......
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