diff options
| -rw-r--r-- | app/models/application_setting.rb | 5 | ||||
| -rw-r--r-- | lib/gitlab/current_settings.rb | 54 | ||||
| -rw-r--r-- | spec/controllers/projects/merge_requests_controller_spec.rb | 2 | ||||
| -rw-r--r-- | spec/lib/gitlab/current_settings_spec.rb | 7 |
4 files changed, 43 insertions, 25 deletions
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 3b49cb4e3bf..2192f76499d 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -189,8 +189,9 @@ class ApplicationSetting < ActiveRecord::Base end def self.cached - ensure_cache_setup - Rails.cache.fetch(CACHE_KEY) + value = Rails.cache.read(CACHE_KEY) + ensure_cache_setup if value.present? + value end def self.ensure_cache_setup diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 9e14b35b0f8..48735fd197d 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -8,39 +8,55 @@ module Gitlab end end - def ensure_application_settings! - return fake_application_settings unless connect_to_db? + delegate :sidekiq_throttling_enabled?, to: :current_application_settings - unless ENV['IN_MEMORY_APPLICATION_SETTINGS'] == 'true' - begin - settings = ::ApplicationSetting.current - # In case Redis isn't running or the Redis UNIX socket file is not available - rescue ::Redis::BaseError, ::Errno::ENOENT - settings = ::ApplicationSetting.last - end + def fake_application_settings + OpenStruct.new(::ApplicationSetting.defaults) + end - settings ||= ::ApplicationSetting.create_from_defaults + private + + def ensure_application_settings! + unless ENV['IN_MEMORY_APPLICATION_SETTINGS'] == 'true' + settings = retrieve_settings_from_database? end settings || in_memory_application_settings 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 @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 + # In case migrations the application_settings table is not created yet, + # we fallback to a simple OpenStruct fake_application_settings end - def fake_application_settings - OpenStruct.new(::ApplicationSetting.defaults) - end - - private - def connect_to_db? # When the DBMS is not available, an exception (e.g. PG::ConnectionBad) is raised active_db_connection = ActiveRecord::Base.connection.active? rescue false diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index a25db7a65fb..08024a2148b 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -126,7 +126,7 @@ describe Projects::MergeRequestsController do 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) end end diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb index c796c98ec9f..fda39d78610 100644 --- a/spec/lib/gitlab/current_settings_spec.rb +++ b/spec/lib/gitlab/current_settings_spec.rb @@ -14,20 +14,20 @@ describe Gitlab::CurrentSettings do end it 'attempts to use cached values first' do - expect(ApplicationSetting).to receive(:current) - expect(ApplicationSetting).not_to receive(:last) + expect(ApplicationSetting).to receive(:cached) expect(current_application_settings).to be_a(ApplicationSetting) end 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(current_application_settings).to be_a(ApplicationSetting) end 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(current_application_settings).to be_a(ApplicationSetting) @@ -37,6 +37,7 @@ describe Gitlab::CurrentSettings do context 'with DB unavailable' 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(:retrieve_settings_from_database_cache?).and_return(nil) end it 'returns an in-memory ApplicationSetting object' do |