Commit 64a8cb07 authored by Aleksei Lipniagov's avatar Aleksei Lipniagov

Deprecate `LoadBalancer#select_caught_up_hosts`

Deprecate redudnant host search implementation with the simpler method
we already have in the class.
parent 9c80c1a6
---
name: load_balancing_refine_load_balancer_methods
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65356
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/335109
milestone: '14.1'
type: development
group: group::memory
default_enabled: false
...@@ -165,6 +165,7 @@ module Gitlab ...@@ -165,6 +165,7 @@ module Gitlab
# while we only need a single host: https://gitlab.com/gitlab-org/gitlab/-/issues/326125#note_615271604 # while we only need a single host: https://gitlab.com/gitlab-org/gitlab/-/issues/326125#note_615271604
# Also, shuffling the list afterwards doesn't seem to be necessary. # Also, shuffling the list afterwards doesn't seem to be necessary.
# This may be improved by merging this method with `select_up_to_date_host`. # This may be improved by merging this method with `select_up_to_date_host`.
# Could be removed when `:load_balancing_refine_load_balancer_methods` FF is rolled out
def select_caught_up_hosts(location) def select_caught_up_hosts(location)
all_hosts = @host_list.hosts all_hosts = @host_list.hosts
valid_hosts = all_hosts.select { |host| host.caught_up?(location) } valid_hosts = all_hosts.select { |host| host.caught_up?(location) }
...@@ -201,6 +202,7 @@ module Gitlab ...@@ -201,6 +202,7 @@ module Gitlab
true true
end end
# Could be removed when `:load_balancing_refine_load_balancer_methods` FF is rolled out
def set_consistent_hosts_for_request(hosts) def set_consistent_hosts_for_request(hosts)
RequestStore[VALID_HOSTS_CACHE_KEY] = hosts RequestStore[VALID_HOSTS_CACHE_KEY] = hosts
end end
......
...@@ -53,8 +53,14 @@ module Gitlab ...@@ -53,8 +53,14 @@ module Gitlab
# write location. If no such location exists, err on the side of caution. # write location. If no such location exists, err on the side of caution.
return false unless location return false unless location
load_balancer.select_caught_up_hosts(location).tap do |selected| if ::Feature.enabled?(:load_balancing_refine_load_balancer_methods)
unstick(namespace, id) if selected load_balancer.select_up_to_date_host(location).tap do |selected|
unstick(namespace, id) if selected
end
else
load_balancer.select_caught_up_hosts(location).tap do |selected|
unstick(namespace, id) if selected
end
end end
end end
......
...@@ -325,10 +325,22 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do ...@@ -325,10 +325,22 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do
end end
it 'returns true, selects hosts, and unsticks if any secondary has caught up' do it 'returns true, selects hosts, and unsticks if any secondary has caught up' do
expect(lb).to receive(:select_caught_up_hosts).and_return(true) expect(lb).to receive(:select_up_to_date_host).and_return(true)
expect(described_class).to receive(:unstick).with(:project, 42) expect(described_class).to receive(:unstick).with(:project, 42)
expect(described_class.select_caught_up_replicas(:project, 42)).to be true expect(described_class.select_caught_up_replicas(:project, 42)).to be true
end end
context 'when :load_balancing_refine_load_balancer_methods FF is disabled' do
before do
stub_feature_flags(load_balancing_refine_load_balancer_methods: false)
end
it 'returns true, selects hosts, and unsticks if any secondary has caught up' do
expect(lb).to receive(:select_caught_up_hosts).and_return(true)
expect(described_class).to receive(:unstick).with(:project, 42)
expect(described_class.select_caught_up_replicas(:project, 42)).to be true
end
end
end end
end end
end end
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