Commit 6ac1caa0 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch '33279_pc_application_settings_performance' into 'master'

redesign caching of application settings

See merge request !11894
parents 86b4cd61 d9335282
...@@ -189,8 +189,9 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -189,8 +189,9 @@ class ApplicationSetting < ActiveRecord::Base
end end
def self.cached def self.cached
ensure_cache_setup value = Rails.cache.read(CACHE_KEY)
Rails.cache.fetch(CACHE_KEY) ensure_cache_setup if value.present?
value
end end
def self.ensure_cache_setup def self.ensure_cache_setup
......
...@@ -8,39 +8,55 @@ module Gitlab ...@@ -8,39 +8,55 @@ module Gitlab
end end
end end
def ensure_application_settings! delegate :sidekiq_throttling_enabled?, to: :current_application_settings
return fake_application_settings unless connect_to_db?
unless ENV['IN_MEMORY_APPLICATION_SETTINGS'] == 'true' def fake_application_settings
begin OpenStruct.new(::ApplicationSetting.defaults)
settings = ::ApplicationSetting.current end
# In case Redis isn't running or the Redis UNIX socket file is not available
rescue ::Redis::BaseError, ::Errno::ENOENT
settings = ::ApplicationSetting.last
end
settings ||= ::ApplicationSetting.create_from_defaults private
def ensure_application_settings!
unless ENV['IN_MEMORY_APPLICATION_SETTINGS'] == 'true'
settings = retrieve_settings_from_database?
end end
settings || in_memory_application_settings settings || in_memory_application_settings
end end
delegate :sidekiq_throttling_enabled?, to: :current_application_settings def retrieve_settings_from_database?
settings = retrieve_settings_from_database_cache?
return settings if settings.present?
return fake_application_settings unless connect_to_db?
begin
db_settings = ::ApplicationSetting.current
# In case Redis isn't running or the Redis UNIX socket file is not available
rescue ::Redis::BaseError, ::Errno::ENOENT
db_settings = ::ApplicationSetting.last
end
db_settings || ::ApplicationSetting.create_from_defaults
end
def retrieve_settings_from_database_cache?
begin
settings = ApplicationSetting.cached
rescue ::Redis::BaseError, ::Errno::ENOENT
# In case Redis isn't running or the Redis UNIX socket file is not available
settings = nil
end
settings
end
def in_memory_application_settings def in_memory_application_settings
@in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults) @in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults)
# In case migrations the application_settings table is not created yet,
# we fallback to a simple OpenStruct
rescue ActiveRecord::StatementInvalid, ActiveRecord::UnknownAttributeError rescue ActiveRecord::StatementInvalid, ActiveRecord::UnknownAttributeError
# In case migrations the application_settings table is not created yet,
# we fallback to a simple OpenStruct
fake_application_settings fake_application_settings
end end
def fake_application_settings
OpenStruct.new(::ApplicationSetting.defaults)
end
private
def connect_to_db? def connect_to_db?
# When the DBMS is not available, an exception (e.g. PG::ConnectionBad) is raised # When the DBMS is not available, an exception (e.g. PG::ConnectionBad) is raised
active_db_connection = ActiveRecord::Base.connection.active? rescue false active_db_connection = ActiveRecord::Base.connection.active? rescue false
......
...@@ -126,7 +126,7 @@ describe Projects::MergeRequestsController do ...@@ -126,7 +126,7 @@ describe Projects::MergeRequestsController do
recorded = ActiveRecord::QueryRecorder.new { go(format: :json) } recorded = ActiveRecord::QueryRecorder.new { go(format: :json) }
expect(recorded.count).to be_within(5).of(59) expect(recorded.count).to be_within(5).of(50)
expect(recorded.cached_count).to eq(0) expect(recorded.cached_count).to eq(0)
end end
end end
......
...@@ -14,20 +14,20 @@ describe Gitlab::CurrentSettings do ...@@ -14,20 +14,20 @@ describe Gitlab::CurrentSettings do
end end
it 'attempts to use cached values first' do it 'attempts to use cached values first' do
expect(ApplicationSetting).to receive(:current) expect(ApplicationSetting).to receive(:cached)
expect(ApplicationSetting).not_to receive(:last)
expect(current_application_settings).to be_a(ApplicationSetting) expect(current_application_settings).to be_a(ApplicationSetting)
end end
it 'falls back to DB if Redis returns an empty value' do it 'falls back to DB if Redis returns an empty value' do
expect(ApplicationSetting).to receive(:cached).and_return(nil)
expect(ApplicationSetting).to receive(:last).and_call_original expect(ApplicationSetting).to receive(:last).and_call_original
expect(current_application_settings).to be_a(ApplicationSetting) expect(current_application_settings).to be_a(ApplicationSetting)
end end
it 'falls back to DB if Redis fails' do it 'falls back to DB if Redis fails' do
expect(ApplicationSetting).to receive(:current).and_raise(::Redis::BaseError) expect(ApplicationSetting).to receive(:cached).and_raise(::Redis::BaseError)
expect(ApplicationSetting).to receive(:last).and_call_original expect(ApplicationSetting).to receive(:last).and_call_original
expect(current_application_settings).to be_a(ApplicationSetting) expect(current_application_settings).to be_a(ApplicationSetting)
...@@ -37,6 +37,7 @@ describe Gitlab::CurrentSettings do ...@@ -37,6 +37,7 @@ describe Gitlab::CurrentSettings do
context 'with DB unavailable' do context 'with DB unavailable' do
before do before do
allow_any_instance_of(described_class).to receive(:connect_to_db?).and_return(false) allow_any_instance_of(described_class).to receive(:connect_to_db?).and_return(false)
allow_any_instance_of(described_class).to receive(:retrieve_settings_from_database_cache?).and_return(nil)
end end
it 'returns an in-memory ApplicationSetting object' do it 'returns an in-memory ApplicationSetting object' 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