Commit 23aaf63f authored by Mark Chao's avatar Mark Chao

Revert 'Enable Query Cache for Load balancing'

A quick fix for https://gitlab.com/gitlab-org/gitlab/-/issues/290733

Revert "Merge branch '276188-enable-query-cache-for-load-balancer' into
'master'"

This reverts commit 54f0e919, reversing
changes made to 0098407c.
parent 511736f2
---
name: query_cache_for_load_balancing
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46765
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/276203
milestone: '13.7'
type: development
group: group::memory
default_enabled: false
......@@ -7,7 +7,7 @@ module Gitlab
class Host
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, to: :pool
CONNECTION_ERRORS =
if defined?(PG)
......
......@@ -12,7 +12,6 @@ module Gitlab
# always returns a connection to the primary.
class LoadBalancer
CACHE_KEY = :gitlab_load_balancer_host
ENSURE_CACHING_KEY = 'ensure_caching'
attr_reader :host_list
......@@ -29,8 +28,6 @@ module Gitlab
conflict_retried = 0
while host
ensure_caching!
begin
return yield host.connection
rescue => error
......@@ -98,12 +95,7 @@ module Gitlab
# Releases the host and connection for the current thread.
def release_host
if host = RequestStore[CACHE_KEY]
host.disable_query_cache!
host.release_connection
end
RequestStore.delete(ENSURE_CACHING_KEY)
RequestStore[CACHE_KEY]&.release_connection
RequestStore.delete(CACHE_KEY)
end
......@@ -177,23 +169,6 @@ module Gitlab
error.is_a?(PG::TRSerializationFailure)
end
end
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!
# Feature (Flipper gem) reads the data from the database, and it would cause the infinite loop here.
# 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)
host.enable_query_cache!
end
end
end
end
end
......
......@@ -12,7 +12,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer do
after do
RequestStore.delete(described_class::CACHE_KEY)
RequestStore.delete(described_class::ENSURE_CACHING_KEY)
end
def raise_and_wrap(wrapper, original)
......@@ -53,28 +52,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer do
allow(lb).to receive(:host).and_return(host)
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(RequestStore[described_class::ENSURE_CACHING_KEY]).to be true
end
context 'when :query_cache_for_load_balancing feature flag is disabled' do
before do
stub_feature_flags(query_cache_for_load_balancing: false)
end
it 'yields a connection for a read without enabling query cache' do
connection = double(:connection)
host = double(:host)
allow(lb).to receive(:host).and_return(host)
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)
end
end
it 'marks hosts that are offline' do
......@@ -163,14 +142,10 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer do
describe '#release_host' do
it 'releases the host and its connection' do
host = lb.host
expect(host).to receive(:disable_query_cache!)
lb.host
lb.release_host
expect(RequestStore[described_class::CACHE_KEY]).to be_nil
expect(RequestStore[described_class::ENSURE_CACHING_KEY]).to be_nil
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