Cache dynamic settings in request cache

fixes FOO-2727

Change-Id: I64c5d7ddaed195a84da93e8b4b49231a7c3a30de
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/286494
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ben Rinaca <brinaca@instructure.com>
QA-Review: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Jacob Burroughs <jburroughs@instructure.com>

config/application.rb

@@ -327,7 +327,12 @@ module CanvasRails
         # do not remove this conditional until the asset build no longer
         # needs the rails app for anything.
 
+        # Do it early with the wrong cache for things super early in boot
         DynamicSettingsInitializer.bootstrap!
+        # Do it at the end when the autoloader is set up correctly
+        config.to_prepare do
+          DynamicSettingsInitializer.bootstrap!
+        end
       end
     end
 

config/environments/development.rb

@@ -28,8 +28,8 @@ environment_configuration(defined?(config) && config) do |config|
   config.cache_classes = false
 
   # Show full error reports and disable caching
-  config.consider_all_requests_local = true
-  config.action_controller.perform_caching = false
+  config.consider_all_requests_local = !ActiveModel::Type::Boolean.new.cast(ENV.fetch("SHOW_PRODUCTION_ERRORS", false))
+  config.action_controller.perform_caching = ActiveModel::Type::Boolean.new.cast(ENV.fetch("ACTION_CONTROLLER_CACHING", false))
 
   # run rake js:build to build the optimized JS if set to true
   # ENV['USE_OPTIMIZED_JS']                            = 'true'

gems/dynamic_settings/lib/dynamic_settings.rb

@@ -24,6 +24,7 @@ require "config_file"
 require "diplomat"
 require "dynamic_settings/circuit_breaker"
 require "dynamic_settings/memory_cache"
+require "dynamic_settings/null_request_cache"
 require "dynamic_settings/fallback_proxy"
 require "dynamic_settings/prefix_proxy"
 
@@ -35,7 +36,7 @@ module DynamicSettings
   class << self
     attr_accessor :environment
     attr_reader :fallback_data, :use_consul, :config
-    attr_writer :fallback_recovery_lambda, :retry_lambda, :cache, :logger
+    attr_writer :fallback_recovery_lambda, :retry_lambda, :cache, :request_cache, :logger
 
     def config=(conf_hash)
       @config = conf_hash
@@ -57,6 +58,7 @@ module DynamicSettings
         @data_center = conf_hash.fetch("global_dc", nil)
         @default_service = conf_hash.fetch("service", :canvas)
         @cache = conf_hash.fetch("cache", ::DynamicSettings::MemoryCache.new)
+        @request_cache = conf_hash.fetch("request_cache", ::DynamicSettings::NullRequestCache.new)
         @fallback_recovery_lambda = conf_hash.fetch("fallback_recovery_lambda", nil)
         @retry_lambda = conf_hash.fetch("retry_lambda", nil)
         @logger = conf_hash.fetch("logger", nil)
@@ -64,7 +66,8 @@ module DynamicSettings
         @environment = nil
         @use_consul = false
         @default_service = :canvas
-        @cache = ::DynamicSettings::MemoryCache.new
+        @cache = nil
+        @request_cache = nil
       end
     end
 
@@ -76,6 +79,10 @@ module DynamicSettings
       @cache ||= ::DynamicSettings::MemoryCache.new
     end
 
+    def request_cache
+      @request_cache ||= ::DynamicSettings::NullRequestCache.new
+    end
+
     def on_fallback_recovery(exception)
       @fallback_recovery_lambda&.call(exception)
     end

gems/dynamic_settings/lib/dynamic_settings/null_request_cache.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+#
+# Copyright (C) 2022 - present Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+module DynamicSettings
+  class NullRequestCache
+    def cache(*_args)
+      yield
+    end
+  end
+end

gems/dynamic_settings/lib/dynamic_settings/prefix_proxy.rb

@@ -83,6 +83,56 @@ module DynamicSettings
       unknown_kwargs = kwargs.keys - [:failsafe]
       raise ArgumentError, "unknown keyword(s): #{unknown_kwargs.map(&:inspect).join(", ")}" unless unknown_kwargs.empty?
 
+      # Within a given request, no reason to talk to redis/consul multiple times for the same key in the same tree
+      # The TTL is only relevant for the underlying cache-within a request we don't exceed the ttl boundary
+      DynamicSettings.request_cache.cache(CACHE_KEY_PREFIX + full_key(key)) do
+        fetch_without_request_cache(key, ttl: ttl, **kwargs)
+      end
+    end
+    alias_method :[], :fetch
+
+    # Extend the prefix from this instance returning a new one.
+    #
+    # @param prefix_extension [String]
+    # @param default_ttl [ActiveSupport::Duration] The default TTL to use when
+    #  fetching keys from the extended keyspace, defaults to the same value as
+    #  the receiver
+    # @return [ProxyPrefix]
+    def for_prefix(prefix_extension, default_ttl: @default_ttl)
+      self.class.new(
+        "#{@prefix}/#{prefix_extension}",
+        tree: tree,
+        service: service,
+        environment: environment,
+        cluster: cluster,
+        default_ttl: default_ttl,
+        data_center: @data_center
+      )
+    end
+
+    # Set multiple key value pairs
+    #
+    # @param kvs [Hash] Key value pairs where the hash key is the key
+    #   and the hash value is the value
+    # @param global [boolean] Is it a global key?
+    # @return Consul txn response
+    def set_keys(kvs, global: false)
+      opts = @data_center.present? && global ? { dc: @data_center } : {}
+      value = kvs.map do |k, v|
+        {
+          "KV" => {
+            "Verb" => "set",
+            "Key" => full_key(k, global: global),
+            "Value" => v,
+          }
+        }
+      end
+      Diplomat::Kv.txn(value, opts)
+    end
+
+    private
+
+    def fetch_without_request_cache(key, ttl: @default_ttl, **kwargs)
       retry_count = 1
 
       keys = [
@@ -181,48 +231,6 @@ module DynamicSettings
         raise
       end
     end
-    alias_method :[], :fetch
-
-    # Extend the prefix from this instance returning a new one.
-    #
-    # @param prefix_extension [String]
-    # @param default_ttl [ActiveSupport::Duration] The default TTL to use when
-    #  fetching keys from the extended keyspace, defaults to the same value as
-    #  the receiver
-    # @return [ProxyPrefix]
-    def for_prefix(prefix_extension, default_ttl: @default_ttl)
-      self.class.new(
-        "#{@prefix}/#{prefix_extension}",
-        tree: tree,
-        service: service,
-        environment: environment,
-        cluster: cluster,
-        default_ttl: default_ttl,
-        data_center: @data_center
-      )
-    end
-
-    # Set multiple key value pairs
-    #
-    # @param kvs [Hash] Key value pairs where the hash key is the key
-    #   and the hash value is the value
-    # @param global [boolean] Is it a global key?
-    # @return Consul txn response
-    def set_keys(kvs, global: false)
-      opts = @data_center.present? && global ? { dc: @data_center } : {}
-      value = kvs.map do |k, v|
-        {
-          "KV" => {
-            "Verb" => "set",
-            "Key" => full_key(k, global: global),
-            "Value" => v,
-          }
-        }
-      end
-      Diplomat::Kv.txn(value, opts)
-    end
-
-    private
 
     # bit of helper indirection
     # so that we can log actual

lib/dynamic_settings_initializer.rb

@@ -48,6 +48,7 @@ module DynamicSettingsInitializer
     # dependency injection stuff from when
     # this got pulled out into a local gem
     ::DynamicSettings.cache = LocalCache
+    ::DynamicSettings.request_cache = RequestCache
     ::DynamicSettings.fallback_recovery_lambda = ->(e) { Canvas::Errors.capture_exception(:consul, e, :warn) }
     ::DynamicSettings.retry_lambda = ->(e) { Canvas::Errors.capture_exception(:consul, e, :warn) }
     ::DynamicSettings.logger = Rails.logger