Commit 9579939d authored by Drew Blessing's avatar Drew Blessing

Cache namespace first Auto DevOps config

Retrieving a namespace's first Auto DevOps config can be an
expensive task. The value cascades so the method queries up the
hierarchy to find the first non-nil value.

This change caches that result to reduce the frequency of the N+1
necessary to get these results. Since the setting should rarely
change, it's helpful to cache the result and selectively expire
the cache.

Changelog: fixed
parent 9e20fe77
......@@ -117,6 +117,7 @@ class Namespace < ApplicationRecord
before_create :sync_share_with_group_lock_with_parent
before_update :sync_share_with_group_lock_with_parent, if: :parent_changed?
after_update :force_share_with_group_lock_on_descendants, if: -> { saved_change_to_share_with_group_lock? && share_with_group_lock? }
after_update :expire_first_auto_devops_config_cache, if: -> { saved_change_to_auto_devops_enabled? }
# Legacy Storage specific hooks
......@@ -401,7 +402,11 @@ class Namespace < ApplicationRecord
return { scope: :group, status: auto_devops_enabled } unless auto_devops_enabled.nil?
strong_memoize(:first_auto_devops_config) do
if has_parent?
if has_parent? && cache_first_auto_devops_config?
Rails.cache.fetch(first_auto_devops_config_cache_key_for(id), expires_in: 1.day) do
parent.first_auto_devops_config
end
elsif has_parent?
parent.first_auto_devops_config
else
{ scope: :instance, status: Gitlab::CurrentSettings.auto_devops_enabled? }
......@@ -621,6 +626,20 @@ class Namespace < ApplicationRecord
.update_all(share_with_group_lock: true)
end
def expire_first_auto_devops_config_cache
return unless cache_first_auto_devops_config?
descendants_to_expire = self_and_descendants.as_ids
return if descendants_to_expire.load.empty?
keys = descendants_to_expire.map { |group| first_auto_devops_config_cache_key_for(group.id) }
Rails.cache.delete_multi(keys)
end
def cache_first_auto_devops_config?
::Feature.enabled?(:namespaces_cache_first_auto_devops_config, default_enabled: :yaml)
end
def write_projects_repository_config
all_projects.find_each do |project|
project.set_full_path
......@@ -638,6 +657,13 @@ class Namespace < ApplicationRecord
Namespaces::SyncEvent.enqueue_worker
end
end
def first_auto_devops_config_cache_key_for(group_id)
return "namespaces:{first_auto_devops_config}:#{group_id}" unless sync_traversal_ids?
# Use SHA2 of `traversal_ids` to account for moving a namespace within the same root ancestor hierarchy.
"namespaces:{#{traversal_ids.first}}:first_auto_devops_config:#{group_id}:#{Digest::SHA2.hexdigest(traversal_ids.join(' '))}"
end
end
Namespace.prepend_mod_with('Namespace')
---
name: namespaces_cache_first_auto_devops_config
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/80937
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/353503
milestone: '14.9'
type: development
group: group::authentication and authorization
default_enabled: false
......@@ -2191,7 +2191,7 @@ RSpec.describe Group do
let(:group) { create(:group) }
subject { group.first_auto_devops_config }
subject(:fetch_config) { group.first_auto_devops_config }
where(:instance_value, :group_value, :config) do
# Instance level enabled
......@@ -2216,6 +2216,8 @@ RSpec.describe Group do
end
context 'with parent groups' do
let(:parent) { create(:group) }
where(:instance_value, :parent_value, :group_value, :config) do
# Instance level enabled
true | nil | nil | { status: true, scope: :instance }
......@@ -2245,17 +2247,82 @@ RSpec.describe Group do
end
with_them do
def define_cache_expectations(cache_key)
if group_value.nil?
expect(Rails.cache).to receive(:fetch).with(start_with(cache_key), expires_in: 1.day)
else
expect(Rails.cache).not_to receive(:fetch).with(start_with(cache_key), expires_in: 1.day)
end
end
before do
stub_application_setting(auto_devops_enabled: instance_value)
parent = create(:group, auto_devops_enabled: parent_value)
group.update!(
auto_devops_enabled: group_value,
parent: parent
)
parent.update!(auto_devops_enabled: parent_value)
group.reload # Reload so we get the populated traversal IDs
end
it { is_expected.to eq(config) }
it 'caches the parent config when group auto_devops_enabled is nil' do
cache_key = "namespaces:{#{group.traversal_ids.first}}:first_auto_devops_config:#{group.id}"
define_cache_expectations(cache_key)
fetch_config
end
context 'when traversal ID feature flags are disabled' do
before do
stub_feature_flags(sync_traversal_ids: false)
end
it 'caches the parent config when group auto_devops_enabled is nil' do
cache_key = "namespaces:{first_auto_devops_config}:#{group.id}"
define_cache_expectations(cache_key)
fetch_config
end
end
end
context 'cache expiration' do
before do
group.update!(parent: parent)
reload_models(parent)
end
it 'clears both self and descendant cache when the parent value is updated' do
expect(Rails.cache).to receive(:delete_multi)
.with(
match_array([
start_with("namespaces:{#{parent.traversal_ids.first}}:first_auto_devops_config:#{parent.id}"),
start_with("namespaces:{#{parent.traversal_ids.first}}:first_auto_devops_config:#{group.id}")
])
)
parent.update!(auto_devops_enabled: true)
end
it 'only clears self cache when there are no dependents' do
expect(Rails.cache).to receive(:delete_multi)
.with([start_with("namespaces:{#{group.traversal_ids.first}}:first_auto_devops_config:#{group.id}")])
group.update!(auto_devops_enabled: true)
end
it 'does not clear cache when the feature is disabled' do
stub_feature_flags(namespaces_cache_first_auto_devops_config: false)
expect(Rails.cache).not_to receive(:delete_multi)
parent.update!(auto_devops_enabled: true)
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