diff options
Diffstat (limited to 'spec/lib/gitlab/database/load_balancing')
6 files changed, 135 insertions, 12 deletions
diff --git a/spec/lib/gitlab/database/load_balancing/configuration_spec.rb b/spec/lib/gitlab/database/load_balancing/configuration_spec.rb index 34370c9a21f..7dc2e0be3e5 100644 --- a/spec/lib/gitlab/database/load_balancing/configuration_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/configuration_spec.rb @@ -23,7 +23,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::Configuration, :request_store do record_type: 'A', interval: 60, disconnect_timeout: 120, - use_tcp: false + use_tcp: false, + max_replica_pools: nil ) expect(config.pool_size).to eq(Gitlab::Database.default_pool_size) end @@ -39,7 +40,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::Configuration, :request_store do replica_check_interval: 3, hosts: %w[foo bar], discover: { - 'record' => 'foo.example.com' + record: 'foo.example.com', + max_replica_pools: 5 } } } @@ -59,7 +61,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::Configuration, :request_store do record_type: 'A', interval: 60, disconnect_timeout: 120, - use_tcp: false + use_tcp: false, + max_replica_pools: 5 ) expect(config.pool_size).to eq(4) end @@ -95,7 +98,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::Configuration, :request_store do record_type: 'A', interval: 60, disconnect_timeout: 120, - use_tcp: false + use_tcp: false, + max_replica_pools: nil ) expect(config.pool_size).to eq(4) end diff --git a/spec/lib/gitlab/database/load_balancing/connection_proxy_spec.rb b/spec/lib/gitlab/database/load_balancing/connection_proxy_spec.rb index 41312dbedd6..a2076f5b950 100644 --- a/spec/lib/gitlab/database/load_balancing/connection_proxy_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/connection_proxy_spec.rb @@ -53,7 +53,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do end Gitlab::Database::LoadBalancing::ConnectionProxy::NON_STICKY_READS.each do |name| - describe "#{name}" do + describe name.to_s do it 'runs the query on the replica' do expect(proxy).to receive(:read_using_load_balancer) .with(name, 'foo') @@ -64,7 +64,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do end Gitlab::Database::LoadBalancing::ConnectionProxy::STICKY_WRITES.each do |name| - describe "#{name}" do + describe name.to_s do it 'runs the query on the primary and sticks to it' do session = Gitlab::Database::LoadBalancing::Session.new diff --git a/spec/lib/gitlab/database/load_balancing/service_discovery/sampler_spec.rb b/spec/lib/gitlab/database/load_balancing/service_discovery/sampler_spec.rb new file mode 100644 index 00000000000..1a49aa2871f --- /dev/null +++ b/spec/lib/gitlab/database/load_balancing/service_discovery/sampler_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Gitlab::Database::LoadBalancing::ServiceDiscovery::Sampler do + let(:sampler) { described_class.new(max_replica_pools: max_replica_pools, seed: 100) } + let(:max_replica_pools) { 3 } + let(:address_class) { ::Gitlab::Database::LoadBalancing::ServiceDiscovery::Address } + let(:addresses) do + [ + address_class.new("127.0.0.1", 6432), + address_class.new("127.0.0.1", 6433), + address_class.new("127.0.0.1", 6434), + address_class.new("127.0.0.1", 6435), + address_class.new("127.0.0.2", 6432), + address_class.new("127.0.0.2", 6433), + address_class.new("127.0.0.2", 6434), + address_class.new("127.0.0.2", 6435) + ] + end + + describe '#sample' do + it 'samples max_replica_pools addresses' do + expect(sampler.sample(addresses).count).to eq(max_replica_pools) + end + + it 'samples random ports across all hosts' do + expect(sampler.sample(addresses)).to eq([ + address_class.new("127.0.0.1", 6432), + address_class.new("127.0.0.2", 6435), + address_class.new("127.0.0.1", 6435) + ]) + end + + it 'returns the same answer for the same input when called multiple times' do + result = sampler.sample(addresses) + expect(sampler.sample(addresses)).to eq(result) + expect(sampler.sample(addresses)).to eq(result) + end + + it 'gives a consistent answer regardless of input ordering' do + expect(sampler.sample(addresses.reverse)).to eq(sampler.sample(addresses)) + end + + it 'samples fairly across all hosts' do + # Choose a bunch of different seeds to prove that it always chooses 2 + # different ports from each host when selecting 4 + (1..10).each do |seed| + sampler = described_class.new(max_replica_pools: 4, seed: seed) + + result = sampler.sample(addresses) + + expect(result.count { |r| r.hostname == "127.0.0.1" }).to eq(2) + expect(result.count { |r| r.hostname == "127.0.0.2" }).to eq(2) + end + end + + context 'when input is an empty array' do + it 'returns an empty array' do + expect(sampler.sample([])).to eq([]) + end + end + + context 'when there are less replicas than max_replica_pools' do + let(:max_replica_pools) { 100 } + + it 'returns the same addresses' do + expect(sampler.sample(addresses)).to eq(addresses) + end + end + + context 'when max_replica_pools is nil' do + let(:max_replica_pools) { nil } + + it 'returns the same addresses' do + expect(sampler.sample(addresses)).to eq(addresses) + end + end + end +end diff --git a/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb b/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb index f05910e5123..984d60e9962 100644 --- a/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb @@ -231,10 +231,13 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery do nameserver: 'localhost', port: 8600, record: 'foo', - record_type: record_type + record_type: record_type, + max_replica_pools: max_replica_pools ) end + let(:max_replica_pools) { nil } + let(:packet) { double(:packet, answer: [res1, res2]) } before do @@ -266,24 +269,51 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery do let(:res1) { double(:resource, host: 'foo1.service.consul.', port: 5432, weight: 1, priority: 1, ttl: 90) } let(:res2) { double(:resource, host: 'foo2.service.consul.', port: 5433, weight: 1, priority: 1, ttl: 90) } let(:res3) { double(:resource, host: 'foo3.service.consul.', port: 5434, weight: 1, priority: 1, ttl: 90) } - let(:packet) { double(:packet, answer: [res1, res2, res3], additional: []) } + let(:res4) { double(:resource, host: 'foo4.service.consul.', port: 5432, weight: 1, priority: 1, ttl: 90) } + let(:packet) { double(:packet, answer: [res1, res2, res3, res4], additional: []) } before do expect_next_instance_of(Gitlab::Database::LoadBalancing::SrvResolver) do |resolver| allow(resolver).to receive(:address_for).with('foo1.service.consul.').and_return(IPAddr.new('255.255.255.0')) allow(resolver).to receive(:address_for).with('foo2.service.consul.').and_return(IPAddr.new('127.0.0.1')) allow(resolver).to receive(:address_for).with('foo3.service.consul.').and_return(nil) + allow(resolver).to receive(:address_for).with('foo4.service.consul.').and_return("127.0.0.2") end end it 'returns a TTL and ordered list of hosts' do addresses = [ described_class::Address.new('127.0.0.1', 5433), + described_class::Address.new('127.0.0.2', 5432), described_class::Address.new('255.255.255.0', 5432) ] expect(service.addresses_from_dns).to eq([90, addresses]) end + + context 'when max_replica_pools is set' do + context 'when the number of addresses exceeds max_replica_pools' do + let(:max_replica_pools) { 2 } + + it 'limits to max_replica_pools' do + expect(service.addresses_from_dns[1].count).to eq(2) + end + end + + context 'when the number of addresses is less than max_replica_pools' do + let(:max_replica_pools) { 5 } + + it 'returns all addresses' do + addresses = [ + described_class::Address.new('127.0.0.1', 5433), + described_class::Address.new('127.0.0.2', 5432), + described_class::Address.new('255.255.255.0', 5432) + ] + + expect(service.addresses_from_dns).to eq([90, addresses]) + end + end + end end context 'when the resolver returns an empty response' do diff --git a/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb b/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb index 88007de53d3..61b63016f1a 100644 --- a/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb @@ -358,7 +358,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ end def process_job(job) - Sidekiq::JobRetry.new.local(worker_class, job.to_json, 'default') do + Sidekiq::JobRetry.new(Sidekiq).local(worker_class, job.to_json, 'default') do worker_class.process_job(job) end end diff --git a/spec/lib/gitlab/database/load_balancing/transaction_leaking_spec.rb b/spec/lib/gitlab/database/load_balancing/transaction_leaking_spec.rb index 6026d979bcf..1eb077fe6ca 100644 --- a/spec/lib/gitlab/database/load_balancing/transaction_leaking_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/transaction_leaking_spec.rb @@ -4,18 +4,18 @@ require 'spec_helper' RSpec.describe 'Load balancer behavior with errors inside a transaction', :redis, :delete do include StubENV - let(:model) { ApplicationRecord } + let(:model) { ActiveRecord::Base } let(:db_host) { model.connection_pool.db_config.host } let(:test_table_name) { '_test_foo' } before do # Patch in our load balancer config, simply pointing at the test database twice - allow(Gitlab::Database::LoadBalancing::Configuration).to receive(:for_model) do |base_model| + allow(Gitlab::Database::LoadBalancing::Configuration).to receive(:for_model).with(model) do |base_model| Gitlab::Database::LoadBalancing::Configuration.new(base_model, [db_host, db_host]) end - Gitlab::Database::LoadBalancing::Setup.new(ApplicationRecord).setup + Gitlab::Database::LoadBalancing::Setup.new(model).setup model.connection.execute(<<~SQL) CREATE TABLE IF NOT EXISTS #{test_table_name} (id SERIAL PRIMARY KEY, value INTEGER) @@ -30,6 +30,10 @@ RSpec.describe 'Load balancer behavior with errors inside a transaction', :redis model.connection.execute(<<~SQL) DROP TABLE IF EXISTS #{test_table_name} SQL + + # reset load balancing to original state + allow(Gitlab::Database::LoadBalancing::Configuration).to receive(:for_model).and_call_original + Gitlab::Database::LoadBalancing::Setup.new(model).setup end def execute(conn) @@ -56,6 +60,7 @@ RSpec.describe 'Load balancer behavior with errors inside a transaction', :redis conn = model.connection expect(::Gitlab::Database::LoadBalancing::Logger).to receive(:warn).with(hash_including(event: :transaction_leak)) + expect(::Gitlab::Database::LoadBalancing::Logger).to receive(:warn).with(hash_including(event: :read_write_retry)) conn.transaction do expect(conn).to be_transaction_open @@ -74,6 +79,8 @@ RSpec.describe 'Load balancer behavior with errors inside a transaction', :redis expect(::Gitlab::Database::LoadBalancing::Logger) .not_to receive(:warn).with(hash_including(event: :transaction_leak)) + expect(::Gitlab::Database::LoadBalancing::Logger) + .to receive(:warn).with(hash_including(event: :read_write_retry)) expect(conn).not_to be_transaction_open @@ -105,6 +112,8 @@ RSpec.describe 'Load balancer behavior with errors inside a transaction', :redis it 'retries when not in a transaction' do expect(::Gitlab::Database::LoadBalancing::Logger) .not_to receive(:warn).with(hash_including(event: :transaction_leak)) + expect(::Gitlab::Database::LoadBalancing::Logger) + .to receive(:warn).with(hash_including(event: :read_write_retry)) expect { execute(model.connection) }.not_to raise_error end |