diff --git a/README.md b/README.md index 465d5d6..dddea76 100644 --- a/README.md +++ b/README.md @@ -80,7 +80,7 @@ Solid Cache supports these options in addition to the standard `ActiveSupport::C - `error_handler` - a Proc to call to handle any `ActiveRecord::ActiveRecordError`s that are raises (default: log errors as warnings) - `expiry_batch_size` - the batch size to use when deleting old records (default: `100`) - `expiry_method` - what expiry method to use `thread` or `job` (default: `thread`) -- `max_age` - the maximum age of entries in the cache (default: `2.weeks.to_i`) +- `max_age` - the maximum age of entries in the cache (default: `2.weeks.to_i`). Can be set to `nil`, but this is not recommended unless using `max_entries` to limit the size of the cache. - `max_entries` - the maximum number of entries allowed in the cache (default: `nil`, meaning no limit) - `cluster` - a Hash of options for the cache database cluster, e.g `{ shards: [:database1, :database2, :database3] }` - `clusters` - and Array of Hashes for multiple cache clusters (ignored if `:cluster` is set) diff --git a/app/models/solid_cache/entry.rb b/app/models/solid_cache/entry.rb index efc5efe..fc395cb 100644 --- a/app/models/solid_cache/entry.rb +++ b/app/models/solid_cache/entry.rb @@ -142,14 +142,26 @@ def to_binary(key) def expiry_candidate_ids(count, max_age:, max_entries:) cache_full = max_entries && max_entries < id_range - min_created_at = max_age.seconds.ago + 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 - order(:id) - .limit(count * 3) - .pluck(:id, :created_at) - .filter_map { |id, created_at| id if cache_full || created_at < min_created_at } - .sample(count) + 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 diff --git a/test/unit/expiry_test.rb b/test/unit/expiry_test.rb index 71ccc3b..3be9495 100644 --- a/test/unit/expiry_test.rb +++ b/test/unit/expiry_test.rb @@ -36,7 +36,7 @@ class SolidCache::ExpiryTest < ActiveSupport::TestCase end test "expires records when the cache is full (#{expiry_method})" do - @cache = lookup_store(expiry_batch_size: 3, max_age: 2.weeks, max_entries: 2, expiry_method: expiry_method) + @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) @cache.write(default_shard_keys[1], 2)