Merge pull request #131 from rails/entry-expiration

Extract expiration to its own concern

app/models/solid_cache/entry.rb

@@ -2,9 +2,8 @@
 
 module SolidCache
   class Entry < Record
-    # This is all quite awkward but it achieves a couple of performance aims
-    # 1. We skip the query cache
-    # 2. We avoid the overhead of building queries and active record objects
+    include Expiration
+
     class << self
       def write(key, value)
         upsert_all_no_query_cache([ { key: key, value: value } ])
@@ -54,18 +53,6 @@ def decrement(key, amount)
         increment(key, -amount)
       end
 
-      def id_range
-        uncached do
-          pick(Arel.sql("max(id) - min(id) + 1")) || 0
-        end
-      end
-
-      def expire(count, max_age:, max_entries:)
-        if (ids = expiry_candidate_ids(count, max_age: max_age, max_entries: max_entries)).any?
-          delete(ids)
-        end
-      end
-
       private
         def upsert_all_no_query_cache(attributes)
           insert_all = ActiveRecord::InsertAll.new(self, attributes, unique_by: upsert_unique_by, on_duplicate: :update, update_only: [ :value ])
@@ -137,31 +124,6 @@ def delete_no_query_cache(attribute, values)
         def to_binary(key)
           ActiveModel::Type::Binary.new.serialize(key)
         end
-
-        def expiry_candidate_ids(count, max_age:, max_entries:)
-          cache_full = max_entries && max_entries < id_range
-          return [] unless cache_full || max_age
-
-          # In the case of multiple concurrent expiry operations, it is desirable to
-          # reduce the overlap of entries being addressed by each. For that reason,
-          # retrieve more ids than are being expired, and use random
-          # sampling to reduce that number to the actual intended count.
-          retrieve_count = count * 3
-
-          uncached do
-            candidates = order(:id).limit(retrieve_count)
-
-            candidate_ids = if cache_full
-              candidates.pluck(:id)
-            else
-              min_created_at = max_age.seconds.ago
-              candidates.pluck(:id, :created_at)
-                        .filter_map { |id, created_at| id if created_at < min_created_at }
-            end
-
-            candidate_ids.sample(count)
-          end
-        end
     end
   end
 end

app/models/solid_cache/entry/expiration.rb

@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+module SolidCache
+  class Entry
+    module Expiration
+      extend ActiveSupport::Concern
+
+      class_methods do
+        def id_range
+          uncached do
+            pick(Arel.sql("max(id) - min(id) + 1")) || 0
+          end
+        end
+
+        def expire(count, max_age:, max_entries:)
+          if (ids = expiry_candidate_ids(count, max_age: max_age, max_entries: max_entries)).any?
+            delete(ids)
+          end
+        end
+
+        private
+          def expiry_candidate_ids(count, max_age:, max_entries:)
+            cache_full = max_entries && max_entries < id_range
+            return [] unless cache_full || max_age
+
+            # In the case of multiple concurrent expiry operations, it is desirable to
+            # reduce the overlap of entries being addressed by each. For that reason,
+            # retrieve more ids than are being expired, and use random
+            # sampling to reduce that number to the actual intended count.
+            retrieve_count = count * 3
+
+            uncached do
+              candidates = order(:id).limit(retrieve_count)
+
+              candidate_ids = if cache_full
+                candidates.pluck(:id)
+              else
+                min_created_at = max_age.seconds.ago
+                candidates.pluck(:id, :created_at)
+                          .filter_map { |id, created_at| id if created_at < min_created_at }
+              end
+
+              candidate_ids.sample(count)
+            end
+          end
+      end
+    end
+  end
+end

test/unit/execution_test.rb

@@ -34,9 +34,8 @@ def test_async_errors_are_reported
     Rails.error.unsubscribe(error_subscriber) if Rails.error.respond_to?(:unsubscribe)
   end
 
-  def test_active_record_instrumention
+  def test_active_record_instrumention_instrumented
     instrumented_cache = lookup_store
-    uninstrumented_cache = lookup_store(active_record_instrumentation: false)
 
     calls = 0
     callback = ->(*args) { calls += 1 }
@@ -47,6 +46,15 @@ def test_active_record_instrumention
       assert_changes -> { calls } do
         instrumented_cache.write("foo", "bar")
       end
+    end
+  end
+
+  def test_active_record_instrumention_uninstrumented
+    uninstrumented_cache = lookup_store(active_record_instrumentation: false)
+
+    calls = 0
+    callback = ->(*args) { calls += 1 }
+    ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do
       assert_no_changes -> { calls } do
         uninstrumented_cache.read("foo")
       end