Commit a61bd329 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'remove_query_cache_for_load_balancing_feature_flag' into 'master'

Remove feature flag query_cache_for_load_balancing

See merge request gitlab-org/gitlab!50959
parents 83c4d68b 272ec0df
...@@ -7,7 +7,7 @@ module Gitlab ...@@ -7,7 +7,7 @@ module Gitlab
class Host class Host
attr_reader :pool, :last_checked_at, :intervals, :load_balancer, :host, :port attr_reader :pool, :last_checked_at, :intervals, :load_balancer, :host, :port
delegate :connection, :release_connection, :enable_query_cache!, :disable_query_cache!, to: :pool delegate :connection, :release_connection, :enable_query_cache!, :disable_query_cache!, :query_cache_enabled, to: :pool
CONNECTION_ERRORS = CONNECTION_ERRORS =
if defined?(PG) if defined?(PG)
......
...@@ -12,7 +12,6 @@ module Gitlab ...@@ -12,7 +12,6 @@ module Gitlab
# always returns a connection to the primary. # always returns a connection to the primary.
class LoadBalancer class LoadBalancer
CACHE_KEY = :gitlab_load_balancer_host CACHE_KEY = :gitlab_load_balancer_host
ENSURE_CACHING_KEY = 'ensure_caching'
attr_reader :host_list attr_reader :host_list
...@@ -103,7 +102,6 @@ module Gitlab ...@@ -103,7 +102,6 @@ module Gitlab
host.release_connection host.release_connection
end end
RequestStore.delete(ENSURE_CACHING_KEY)
RequestStore.delete(CACHE_KEY) RequestStore.delete(CACHE_KEY)
end end
...@@ -180,19 +178,8 @@ module Gitlab ...@@ -180,19 +178,8 @@ module Gitlab
private private
# TODO:
# Move enable_query_cache! to ConnectionPool (https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/database.rb#L223)
# when the feature flag is removed in https://gitlab.com/gitlab-org/gitlab/-/issues/276203.
def ensure_caching! def ensure_caching!
# Feature (Flipper gem) reads the data from the database, and it would cause the infinite loop here. host.enable_query_cache! unless host.query_cache_enabled
# We need to ensure that the code below is executed only once, until the feature flag is removed.
return if RequestStore[ENSURE_CACHING_KEY]
RequestStore[ENSURE_CACHING_KEY] = true
if Feature.enabled?(:query_cache_for_load_balancing, default_enabled: true)
host.enable_query_cache!
end
end end
end end
end end
......
...@@ -50,29 +50,24 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do ...@@ -50,29 +50,24 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
host = double(:host) host = double(:host)
allow(lb).to receive(:host).and_return(host) allow(lb).to receive(:host).and_return(host)
allow(host).to receive(:query_cache_enabled).and_return(true)
expect(host).to receive(:connection).and_return(connection) expect(host).to receive(:connection).and_return(connection)
expect(host).to receive(:enable_query_cache!).once
expect { |b| lb.read(&b) }.to yield_with_args(connection) expect { |b| lb.read(&b) }.to yield_with_args(connection)
expect(RequestStore[described_class::ENSURE_CACHING_KEY]).to be true
end end
context 'when :query_cache_for_load_balancing feature flag is disabled' do it 'ensures that query cache is enabled' do
before do connection = double(:connection)
stub_feature_flags(query_cache_for_load_balancing: false) host = double(:host)
end
it 'yields a connection for a read without enabling query cache' do allow(lb).to receive(:host).and_return(host)
connection = double(:connection) allow(host).to receive(:query_cache_enabled).and_return(false)
host = double(:host) allow(host).to receive(:connection).and_return(connection)
allow(lb).to receive(:host).and_return(host) expect(host).to receive(:enable_query_cache!).once
expect(host).to receive(:connection).and_return(connection)
expect(host).not_to receive(:enable_query_cache!)
expect { |b| lb.read(&b) }.to yield_with_args(connection) lb.read { 10 }
end
end end
it 'marks hosts that are offline' do it 'marks hosts that are offline' do
...@@ -168,7 +163,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do ...@@ -168,7 +163,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
lb.release_host lb.release_host
expect(RequestStore[described_class::CACHE_KEY]).to be_nil expect(RequestStore[described_class::CACHE_KEY]).to be_nil
expect(RequestStore[described_class::ENSURE_CACHING_KEY]).to be_nil
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