Commit 4b59d55a authored by Alex Kalderimis's avatar Alex Kalderimis

Handle recursion when creating ApplicationSettings

This can happen in some situations if we check feature flags before
the application settings object has been initialized. If that happens,
we will try to cache the flag states in redis, which causes redis
metrics to be monitored, which in turn depends on the application
settings. If there is a flag check at the DB transaction level (or at
any other point in this chain) we get infinite recursion, which slowly
causes the stack to overflow (slowly, because there is IO at almost
every layer of the stack).

Instead, we mark when we try to create application settings, and if
the method is re-entered, we bail and fall-back to in-memory application
settings.

Changelog: fixed
parent 25b1d1d1
......@@ -657,7 +657,13 @@ class ApplicationSetting < ApplicationRecord
users_count >= INSTANCE_REVIEW_MIN_USERS
end
Recursion = Class.new(RuntimeError)
def self.create_from_defaults
raise Recursion if Thread.current[:application_setting_create_from_defaults]
Thread.current[:application_setting_create_from_defaults] = true
check_schema!
transaction(requires_new: true) do # rubocop:disable Performance/ActiveRecordSubtransactions
......@@ -666,6 +672,8 @@ class ApplicationSetting < ApplicationRecord
rescue ActiveRecord::RecordNotUnique
# We already have an ApplicationSetting record, so just return it.
current_without_cache
ensure
Thread.current[:application_setting_create_from_defaults] = nil
end
def self.find_or_create_without_cache
......
......@@ -70,6 +70,8 @@ module Gitlab
else
::ApplicationSetting.create_from_defaults
end
rescue ::ApplicationSetting::Recursion
in_memory_application_settings
end
def fake_application_settings(attributes = {})
......
......@@ -179,6 +179,21 @@ RSpec.describe Gitlab::CurrentSettings do
expect(settings).to have_attributes(settings_from_defaults)
end
context 'when we hit a recursive loop' do
before do
expect(ApplicationSetting).to receive(:create_from_defaults) do
raise ApplicationSetting::Recursion
end
end
it 'recovers and returns in-memory settings' do
settings = described_class.current_application_settings
expect(settings).to be_a(ApplicationSetting)
expect(settings).not_to be_persisted
end
end
context 'when ApplicationSettings does not have a primary key' do
before do
allow(ApplicationSetting.connection).to receive(:primary_key).with('application_settings').and_return(nil)
......
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