Ensure freshness of settings with snippet creation

Application settings from the database are currently cached locally in
the thread for a full minute after the changes are made. That means if
an administrator attempts to change the default storages in the admin
page, it might take up to a full minute for those changes to take
effect.

Since we don't have a great cache invalidation strategy for local caches
yet, the simplest approach to solving this problem would be to expire
the local application setting cache whenever we create a snippet. This
causes every snippet creation to make an additional SELECT call for
`application_settings`.

app/models/concerns/has_repository.rb

@@ -15,6 +15,15 @@ module HasRepository
 
   delegate :base_dir, :disk_path, to: :storage
 
+  class_methods do
+    def pick_repository_storage
+      # We need to ensure application settings are fresh when we pick
+      # a repository storage to use.
+      Gitlab::CurrentSettings.expire_current_application_settings
+      Gitlab::CurrentSettings.pick_repository_storage
+    end
+  end
+
   def valid_repo?
     repository.exists?
   rescue

app/models/project.rb

@@ -67,10 +67,7 @@ class Project < ApplicationRecord
   default_value_for :resolve_outdated_diff_discussions, false
   default_value_for :container_registry_enabled, gitlab_config_features.container_registry
   default_value_for(:repository_storage) do
-    # We need to ensure application settings are fresh when we pick
-    # a repository storage to use.
-    Gitlab::CurrentSettings.expire_current_application_settings
-    Gitlab::CurrentSettings.pick_repository_storage
+    pick_repository_storage
   end
 
   default_value_for(:shared_runners_enabled) { Gitlab::CurrentSettings.shared_runners_enabled }

app/models/snippet.rb

@@ -288,8 +288,7 @@ class Snippet < ApplicationRecord
   end
 
   def repository_storage
-    snippet_repository&.shard_name ||
-      Gitlab::CurrentSettings.pick_repository_storage
+    snippet_repository&.shard_name || self.class.pick_repository_storage
   end
 
   def create_repository

changelogs/unreleased/fj-ensure-freshness-snippet-creation.yml

+---
+title: Ensure freshness of settings with snippet creation
+merge_request: 27897
+author:
+type: changed

spec/models/project_spec.rb

@@ -1391,35 +1391,14 @@ describe Project do
   context 'repository storage by default' do
     let(:project) { build(:project) }
 
-    before do
-      storages = {
-        'default' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/repositories'),
-        'picked'  => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/repositories')
-      }
-      allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
-    end
-
     it 'picks storage from ApplicationSetting' do
-      expect_any_instance_of(ApplicationSetting).to receive(:pick_repository_storage).and_return('picked')
+      expect_next_instance_of(ApplicationSetting) do |instance|
+        expect(instance).to receive(:pick_repository_storage).and_return('picked')
+      end
+      expect(described_class).to receive(:pick_repository_storage).and_call_original
 
       expect(project.repository_storage).to eq('picked')
     end
-
-    it 'picks from the latest available storage', :request_store do
-      stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
-      Gitlab::CurrentSettings.current_application_settings
-
-      settings = ApplicationSetting.last
-      settings.repository_storages = %w(picked)
-      settings.save!
-
-      expect(Gitlab::CurrentSettings.repository_storages).to eq(%w(default))
-
-      project
-
-      expect(project.repository.storage).to eq('picked')
-      expect(Gitlab::CurrentSettings.repository_storages).to eq(%w(picked))
-    end
   end
 
   context 'shared runners by default' do

spec/models/snippet_spec.rb

@@ -655,10 +655,18 @@ describe Snippet do
   describe '#repository_storage' do
     let(:snippet) { create(:snippet) }
 
-    it 'returns default repository storage' do
-      expect(Gitlab::CurrentSettings).to receive(:pick_repository_storage)
+    subject { snippet.repository_storage }
 
-      snippet.repository_storage
+    before do
+      expect_next_instance_of(ApplicationSetting) do |instance|
+        expect(instance).to receive(:pick_repository_storage).and_return('picked')
+      end
+    end
+
+    it 'returns repository storage from ApplicationSetting' do
+      expect(described_class).to receive(:pick_repository_storage).and_call_original
+
+      expect(subject).to eq 'picked'
     end
 
     context 'when snippet_project is already created' do
@@ -669,9 +677,7 @@ describe Snippet do
       end
 
       it 'returns repository_storage from snippet_project' do
-        expect(Gitlab::CurrentSettings).not_to receive(:pick_repository_storage)
-
-        expect(snippet.repository_storage).to eq 'foo'
+        expect(subject).to eq 'foo'
       end
     end
   end

spec/support/shared_examples/models/concerns/has_repository_shared_examples.rb

@@ -168,4 +168,37 @@ RSpec.shared_examples 'model with repository' do
     it { is_expected.to respond_to(:base_dir) }
     it { is_expected.to respond_to(:disk_path) }
   end
+
+  describe '.pick_repository_storage' do
+    subject { described_class.pick_repository_storage }
+
+    before do
+      storages = {
+        'default' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/repositories'),
+        'picked'  => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/repositories')
+      }
+      allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
+    end
+
+    it 'picks storage from ApplicationSetting' do
+      expect_next_instance_of(ApplicationSetting) do |instance|
+        expect(instance).to receive(:pick_repository_storage).and_return('picked')
+      end
+
+      expect(subject).to eq('picked')
+    end
+
+    it 'picks from the latest available storage', :request_store do
+      stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+      Gitlab::CurrentSettings.current_application_settings
+
+      settings = ApplicationSetting.last
+      settings.repository_storages = %w(picked)
+      settings.save!
+
+      expect(Gitlab::CurrentSettings.repository_storages).to eq(%w(default))
+      expect(subject).to eq('picked')
+      expect(Gitlab::CurrentSettings.repository_storages).to eq(%w(picked))
+    end
+  end
 end