Merge pull request #132 from rails/probabilistic-expiry

Replace expiry counter with random chance

Author: djmb

lib/solid_cache/cluster/expiry.rb

@@ -9,25 +9,29 @@ module Expiry
       # This ensures there is downward pressure on the cache size while there is valid data to delete
       EXPIRY_MULTIPLIER = 1.25
 
-      attr_reader :expiry_batch_size, :expiry_method, :expiry_queue, :expire_every, :max_age, :max_entries
+      attr_reader :expiry_batch_size, :expiry_method, :expiry_queue, :expiry_chance, :max_age, :max_entries
 
       def initialize(options = {})
         super(options)
         @expiry_batch_size = options.fetch(:expiry_batch_size, 100)
         @expiry_method = options.fetch(:expiry_method, :thread)
         @expiry_queue = options.fetch(:expiry_queue, :default)
-        @expire_every = [ (expiry_batch_size / EXPIRY_MULTIPLIER).floor, 1 ].max
+        @expiry_chance = (1 / expiry_batch_size.to_f) * EXPIRY_MULTIPLIER
         @max_age = options.fetch(:max_age, 2.weeks.to_i)
         @max_entries = options.fetch(:max_entries, nil)
 
         raise ArgumentError, "Expiry method must be one of `:thread` or `:job`" unless [ :thread, :job ].include?(expiry_method)
       end
 
       def track_writes(count)
-        expire_later if expiry_counter.count(count)
+        expire_later if expire?
       end
 
       private
+        def expire?
+          rand < expiry_chance
+        end
+
         def expire_later
           if expiry_method == :job
             ExpiryJob
@@ -37,30 +41,6 @@ def expire_later
             async { Entry.expire(expiry_batch_size, max_age: max_age, max_entries: max_entries) }
           end
         end
-
-        def expiry_counter
-          @expiry_counters ||= connection_names.index_with { |connection_name| Counter.new(expire_every) }
-          @expiry_counters[Entry.current_shard]
-        end
-
-        class Counter
-          attr_reader :expire_every, :counter
-
-          def initialize(expire_every)
-            @expire_every = expire_every
-            @counter = Concurrent::AtomicFixnum.new(rand(expire_every).to_i)
-          end
-
-          def count(count)
-            value = counter.increment(count)
-            new_multiple_of_expire_every?(value - count, value)
-          end
-
-          private
-            def new_multiple_of_expire_every?(first_value, second_value)
-              first_value / expire_every != second_value / expire_every
-            end
-        end
     end
   end
 end

test/unit/expiry_test.rb

@@ -9,11 +9,12 @@ class SolidCache::ExpiryTest < ActiveSupport::TestCase
 
   setup do
     @namespace = "test-#{SecureRandom.hex}"
-    SolidCache::Cluster.any_instance.stubs(:rand).returns(0)
   end
 
   [ :thread, :job ].each do |expiry_method|
     test "expires old records (#{expiry_method})" do
+      SolidCache::Cluster.any_instance.stubs(:rand).returns(0)
+
       @cache = lookup_store(expiry_batch_size: 3, max_age: 2.weeks, expiry_method: expiry_method)
       default_shard_keys = shard_keys(@cache, :default)
       @cache.write(default_shard_keys[0], 1)
@@ -36,6 +37,8 @@ class SolidCache::ExpiryTest < ActiveSupport::TestCase
     end
 
     test "expires records when the cache is full (#{expiry_method})" do
+      SolidCache::Cluster.any_instance.stubs(:rand).returns(0)
+
       @cache = lookup_store(expiry_batch_size: 3, max_age: nil, max_entries: 2, expiry_method: expiry_method)
       default_shard_keys = shard_keys(@cache, :default)
       @cache.write(default_shard_keys[0], 1)
@@ -54,6 +57,8 @@ class SolidCache::ExpiryTest < ActiveSupport::TestCase
     end
 
     test "expires records no shards (#{expiry_method})" do
+      SolidCache::Cluster.any_instance.stubs(:rand).returns(0)
+
       @cache = ActiveSupport::Cache.lookup_store(:solid_cache_store, expiry_batch_size: 3, namespace: @namespace, max_entries: 2, expiry_method: expiry_method)
       default_shard_keys = shard_keys(@cache, :default)
 
@@ -72,8 +77,39 @@ class SolidCache::ExpiryTest < ActiveSupport::TestCase
       assert_equal 1, SolidCache.each_shard.sum { SolidCache::Entry.count }
     end
 
+    test "expires when random number is below threshold (#{expiry_method})" do
+      SolidCache::Cluster.any_instance.stubs(:rand).returns(0.416)
+
+      @cache = ActiveSupport::Cache.lookup_store(:solid_cache_store, expiry_batch_size: 3, namespace: @namespace, max_entries: 1, expiry_method: expiry_method)
+      default_shard_keys = shard_keys(@cache, :default)
+
+      @cache.write(default_shard_keys[0], 1)
+      @cache.write(default_shard_keys[1], 2)
+
+      sleep 0.1
+      perform_enqueued_jobs
+
+      assert_equal 0, SolidCache.each_shard.sum { SolidCache::Entry.count }
+    end
+
+    test "doesn't expire when random number is above threshold (#{expiry_method})" do
+      SolidCache::Cluster.any_instance.stubs(:rand).returns(0.417)
+
+      @cache = ActiveSupport::Cache.lookup_store(:solid_cache_store, expiry_batch_size: 3, namespace: @namespace, max_entries: 1, expiry_method: expiry_method)
+      default_shard_keys = shard_keys(@cache, :default)
+
+      @cache.write(default_shard_keys[0], 1)
+      @cache.write(default_shard_keys[1], 2)
+
+      sleep 0.1
+      perform_enqueued_jobs
+
+      assert_equal 2, SolidCache.each_shard.sum { SolidCache::Entry.count }
+    end
+
     unless ENV["NO_CONNECTS_TO"]
       test "expires old records multiple shards (#{expiry_method})" do
+        SolidCache::Cluster.any_instance.stubs(:rand).returns(0, 1, 0, 1, 0, 1, 0, 1)
         @cache = lookup_store(expiry_batch_size: 2, cluster: { shards: [ :default, :primary_shard_one ] }, expiry_method: expiry_method)
         default_shard_keys = shard_keys(@cache, :default)
         shard_one_keys = shard_keys(@cache, :primary_shard_one)
@@ -118,6 +154,8 @@ class SolidCache::ExpiryTest < ActiveSupport::TestCase
   end
 
   test "expires old records with a custom queue" do
+    SolidCache::Cluster.any_instance.stubs(:rand).returns(0, 1, 0, 1)
+
     @cache = lookup_store(expiry_batch_size: 3, max_entries: 2, expiry_method: :job, expiry_queue: :cache_expiry)
 
     default_shard_keys = shard_keys(@cache, :default)