Merge branch 'sh-add-thread-memory-cache' into 'master'

Add a memory cache local to the thread to reduce Redis load

Closes #63977

See merge request gitlab-org/gitlab-ce!30233

5 files changed, 27 insertions, 8 deletions

diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 42634bf611e..a570da61d54 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -64,7 +64,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   private
 
   def set_application_setting
-    @application_setting = Gitlab::CurrentSettings.current_application_settings
+    @application_setting = ApplicationSetting.current_without_cache
   end
 
   def whitelist_query_limiting
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index aaaa954047f..a7a4e945a99 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -69,7 +69,7 @@ module ApplicationSettingsHelper
   # toggle button effect.
   def import_sources_checkboxes(help_block_id, options = {})
     Gitlab::ImportSources.options.map do |name, source|
-      checked = Gitlab::CurrentSettings.import_sources.include?(source)
+      checked = @application_setting.import_sources.include?(source)
       css_class = checked ? 'active' : ''
       checkbox_name = 'application_setting[import_sources][]'
 
@@ -85,7 +85,7 @@ module ApplicationSettingsHelper
 
   def oauth_providers_checkboxes
     button_based_providers.map do |source|
-      disabled = Gitlab::CurrentSettings.disabled_oauth_sign_in_sources.include?(source.to_s)
+      disabled = @application_setting.disabled_oauth_sign_in_sources.include?(source.to_s)
       css_class = ['btn']
       css_class << 'active' unless disabled
       checkbox_name = 'application_setting[enabled_oauth_sign_in_sources][]'
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index fbd8036653a..8e558487c1c 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -272,4 +272,12 @@ class ApplicationSetting < ApplicationRecord
     # We already have an ApplicationSetting record, so just return it.
     current_without_cache
   end
+
+  # By default, the backend is Rails.cache, which uses
+  # ActiveSupport::Cache::RedisStore. Since loading ApplicationSetting
+  # can cause a significant amount of load on Redis, let's cache it in
+  # memory.
+  def self.cache_backend
+    Gitlab::ThreadMemoryCache.cache_backend
+  end
 end
diff --git a/app/models/concerns/cacheable_attributes.rb b/app/models/concerns/cacheable_attributes.rb
index 3d60f6924c1..8cbf4bcfaf7 100644
--- a/app/models/concerns/cacheable_attributes.rb
+++ b/app/models/concerns/cacheable_attributes.rb
@@ -36,7 +36,7 @@ module CacheableAttributes
     end
 
     def retrieve_from_cache
-      record = Rails.cache.read(cache_key)
+      record = cache_backend.read(cache_key)
       ensure_cache_setup if record.present?
 
       record
@@ -58,7 +58,7 @@ module CacheableAttributes
     end
 
     def expire
-      Rails.cache.delete(cache_key)
+      cache_backend.delete(cache_key)
     rescue
       # Gracefully handle when Redis is not available. For example,
       # omnibus may fail here during gitlab:assets:compile.
@@ -69,9 +69,13 @@ module CacheableAttributes
       # to be loaded when read from cache: https://github.com/rails/rails/issues/27348
       define_attribute_methods
     end
+
+    def cache_backend
+      Rails.cache
+    end
   end
 
   def cache!
-    Rails.cache.write(self.class.cache_key, self, expires_in: 1.minute)
+    self.class.cache_backend.write(self.class.cache_key, self, expires_in: 1.minute)
   end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 38cb4d1a6e8..26be197209a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1460,7 +1460,7 @@ class User < ApplicationRecord
   end
 
   def requires_usage_stats_consent?
-    !consented_usage_stats? && 7.days.ago > self.created_at && !has_current_license? && User.single_user?
+    self.admin? && 7.days.ago > self.created_at && !has_current_license? && User.single_user? && !consented_usage_stats?
   end
 
   # Avoid migrations only building user preference object when needed.
@@ -1495,7 +1495,14 @@ class User < ApplicationRecord
   end
 
   def consented_usage_stats?
-    Gitlab::CurrentSettings.usage_stats_set_by_user_id == self.id
+    # Bypass the cache here because it's possible the admin enabled the
+    # usage ping, and we don't want to annoy the user again if they
+    # already set the value. This is a bit of hack, but the alternative
+    # would be to put in a more complex cache invalidation step. Since
+    # this call only gets called in the uncommon situation where the
+    # user is an admin and the only user in the instance, this shouldn't
+    # cause too much load on the system.
+    ApplicationSetting.current_without_cache&.usage_stats_set_by_user_id == self.id
   end
 
   # Added according to https://github.com/plataformatec/devise/blob/7df57d5081f9884849ca15e4fde179ef164a575f/README.md#activejob-integration