From 6fd48cc223d714007943ce8b6ed0affab60fd241 Mon Sep 17 00:00:00 2001
From: Shinji Nakamatsu <19329+snaka@users.noreply.github.com>
Date: Wed, 11 Feb 2026 15:08:03 +0900
Subject: [PATCH 1/2] feat: add cross-process cache invalidation

- Add CacheInvalidator for cross-process cache invalidation via a Redis-backed global timestamp.
- Add cache_global_ts_check_interval configuration option (opt-in; mechanism is disabled when unset).
- Clear experiment cache when winner is set.
- Add tests for cross-process invalidation and the opt-in default.
---
 lib/split.rb                   |   1 +
 lib/split/cache.rb             |  15 ++++-
 lib/split/cache_invalidator.rb |  71 +++++++++++++++++++++++
 lib/split/configuration.rb     |   1 +
 lib/split/experiment.rb        |   1 +
 spec/cache_spec.rb             | 100 +++++++++++++++++++++++++++++++--
 spec/experiment_spec.rb        |   5 ++
 7 files changed, 188 insertions(+), 6 deletions(-)
 create mode 100644 lib/split/cache_invalidator.rb

diff --git a/lib/split.rb b/lib/split.rb
index 74d53204..e723b33d 100755
--- a/lib/split.rb
+++ b/lib/split.rb
@@ -7,6 +7,7 @@
 require "split/algorithms/weighted_sample"
 require "split/algorithms/whiplash"
 require "split/alternative"
+require "split/cache_invalidator"
 require "split/cache"
 require "split/configuration"
 require "split/encapsulated_helper"
diff --git a/lib/split/cache.rb b/lib/split/cache.rb
index f570af7f..b8fbc4e9 100644
--- a/lib/split/cache.rb
+++ b/lib/split/cache.rb
@@ -4,21 +4,32 @@ module Split
   class Cache
     def self.clear
       @cache = nil
+      CacheInvalidator.reset
     end
 
     def self.fetch(namespace, key)
       return yield unless Split.configuration.cache
 
+      # Check global invalidation
+      CacheInvalidator.check_and_clear_if_needed(self)
+
       @cache ||= {}
       @cache[namespace] ||= {}
 
       value = @cache[namespace][key]
-      return value if value
+      unless value
+        value = yield
+        @cache[namespace][key] = value
+      end
 
-      @cache[namespace][key] = yield
+      value
     end
 
     def self.clear_key(key)
+      # Invalidate globally for all processes
+      CacheInvalidator.invalidate
+
+      # Clear from local cache immediately
       @cache&.keys&.each do |namespace|
         @cache[namespace]&.delete(key)
       end
diff --git a/lib/split/cache_invalidator.rb b/lib/split/cache_invalidator.rb
new file mode 100644
index 00000000..160cbea6
--- /dev/null
+++ b/lib/split/cache_invalidator.rb
@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+module Split
+  # Manages global cache invalidation across multiple processes using Redis timestamp.
+  # The mechanism is opt-in: it activates only when
+  # Split.configuration.cache_global_ts_check_interval is set.
+  class CacheInvalidator
+    class << self
+      # Check global timestamp and clear cache if it has been updated.
+      # No-op when cache_global_ts_check_interval is not configured.
+      # @param cache [Split::Cache] the cache instance to potentially clear
+      def check_and_clear_if_needed(cache)
+        return unless check_interval
+
+        now = Time.now
+
+        # Skip if within check interval
+        return if within_check_interval?(now)
+
+        # Get global timestamp from Redis
+        current_global_ts = fetch_global_timestamp
+
+        # Clear cache if timestamp was updated
+        cache.clear if timestamp_updated?(current_global_ts)
+
+        # Update local timestamp and check time
+        update_local_state(current_global_ts, now)
+      end
+
+      # Invalidate cache globally by updating the global timestamp.
+      # No-op when cache_global_ts_check_interval is not configured.
+      def invalidate
+        return unless check_interval
+
+        Split.redis.set(global_timestamp_key, Time.now.to_f)
+      end
+
+      # Reset local state (used when cache is cleared)
+      def reset
+        @global_cache_ts = nil
+        @last_global_ts_check = nil
+      end
+
+      private
+        def within_check_interval?(now)
+          @last_global_ts_check && (now.to_f - @last_global_ts_check.to_f) < check_interval
+        end
+
+        def fetch_global_timestamp
+          Split.redis.get(global_timestamp_key).to_f
+        end
+
+        def timestamp_updated?(current_global_ts)
+          @global_cache_ts && current_global_ts > @global_cache_ts
+        end
+
+        def update_local_state(current_global_ts, now)
+          @global_cache_ts = current_global_ts
+          @last_global_ts_check = now
+        end
+
+        def global_timestamp_key
+          "split:cache:global_ts"
+        end
+
+        def check_interval
+          Split.configuration.cache_global_ts_check_interval
+        end
+    end
+  end
+end
diff --git a/lib/split/configuration.rb b/lib/split/configuration.rb
index b43e5920..a48ab4b3 100644
--- a/lib/split/configuration.rb
+++ b/lib/split/configuration.rb
@@ -30,6 +30,7 @@ class Configuration
     attr_accessor :redis
     attr_accessor :dashboard_pagination_default_per_page
     attr_accessor :cache
+    attr_accessor :cache_global_ts_check_interval
 
     attr_reader :experiments
 
diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb
index d91552da..e1a39206 100644
--- a/lib/split/experiment.rb
+++ b/lib/split/experiment.rb
@@ -153,6 +153,7 @@ def has_winner?
     def winner=(winner_name)
       redis.hset(:experiment_winner, name, winner_name.to_s)
       @has_winner = true
+      Split::Cache.clear_key(@name)
       Split.configuration.on_experiment_winner_choose.call(self)
     end
 
diff --git a/spec/cache_spec.rb b/spec/cache_spec.rb
index 48b6cd46..fdebbe40 100644
--- a/spec/cache_spec.rb
+++ b/spec/cache_spec.rb
@@ -10,9 +10,15 @@
   before { allow(Time).to receive(:now).and_return(now) }
 
   describe "clear" do
-    before { Split.configuration.cache = true }
+    before do
+      Split.configuration.cache = true
+      # Disable global invalidation check for this test
+      allow(Split::CacheInvalidator).to receive(:check_and_clear_if_needed)
+    end
 
     it "clears the cache" do
+      # First fetch: yield calls Time.now (x1)
+      # Second fetch: yield calls Time.now (x1)
       expect(Time).to receive(:now).and_return(now).exactly(2).times
       Split::Cache.fetch(namespace, key) { Time.now }
       Split::Cache.clear
@@ -21,9 +27,19 @@
   end
 
   describe "clear_key" do
-    before { Split.configuration.cache = true }
+    before do
+      Split.configuration.cache = true
+      # Disable global invalidation check for this test
+      allow(Split::CacheInvalidator).to receive(:check_and_clear_if_needed)
+    end
 
     it "clears the cache" do
+      # fetch :key1 (miss): yield (x1)
+      # fetch :key2 (miss): yield (x1)
+      # clear_key(:key1): no Time.now (invalidate is no-op without interval)
+      # fetch :key1 (miss again, cleared): yield (x1)
+      # fetch :key2 (cache hit): 0
+      # Total: 3
       expect(Time).to receive(:now).and_return(now).exactly(3).times
       Split::Cache.fetch(namespace, :key1) { Time.now }
       Split::Cache.fetch(namespace, :key2) { Time.now }
@@ -32,6 +48,17 @@
       Split::Cache.fetch(namespace, :key1) { Time.now }
       Split::Cache.fetch(namespace, :key2) { Time.now }
     end
+
+    it "updates global timestamp in Redis when invalidation is enabled" do
+      Split.configuration.cache = true
+      Split.configuration.cache_global_ts_check_interval = 5
+      current_time = Time.now.to_f
+
+      expect(Split.redis).to receive(:set).with("split:cache:global_ts", current_time)
+      allow(Time).to receive(:now).and_return(Time.at(current_time))
+
+      Split::Cache.clear_key(:some_key)
+    end
   end
 
   describe "fetch" do
@@ -52,14 +79,20 @@
     end
 
     context "when cache enabled" do
-      before { Split.configuration.cache = true }
+      before do
+        Split.configuration.cache = true
+        # Disable global invalidation check for this test
+        allow(Split::CacheInvalidator).to receive(:check_and_clear_if_needed)
+      end
 
       it "returns the yield" do
         expect(subject).to eql(now)
       end
 
       it "yields once" do
-        expect(Time).to receive(:now).and_return(now).once
+        # First fetch: yield calls Time.now (x1)
+        # Second fetch: cache hit, no yield (0)
+        expect(Time).to receive(:now).and_return(now).exactly(1).times
         Split::Cache.fetch(namespace, key) { Time.now }
         Split::Cache.fetch(namespace, key) { Time.now }
       end
@@ -81,4 +114,63 @@
       end
     end
   end
+
+  describe "global timestamp invalidation" do
+    before do
+      Split.configuration.cache = true
+      Split.configuration.cache_global_ts_check_interval = 5
+      Split::Cache.clear
+      Split::CacheInvalidator.reset
+    end
+
+    it "clears cache when global timestamp is updated" do
+      # Mock Redis to return initial timestamp
+      allow(Split.redis).to receive(:get).with("split:cache:global_ts").and_return("1000.0")
+
+      # First fetch - should cache the value
+      result1 = Split::Cache.fetch(namespace, key) { "value1" }
+      expect(result1).to eq("value1")
+
+      # Simulate another process updating the global timestamp
+      allow(Split.redis).to receive(:get).with("split:cache:global_ts").and_return("2000.0")
+
+      # Move time forward to bypass check interval
+      allow(Time).to receive(:now).and_return(Time.at(now + 10))
+
+      # Second fetch - should get new value because cache was cleared
+      result2 = Split::Cache.fetch(namespace, key) { "value2" }
+      expect(result2).to eq("value2")
+    end
+
+    it "does not check Redis within check interval" do
+      allow(Split.redis).to receive(:get).with("split:cache:global_ts").and_return("1000.0")
+
+      # First fetch - checks Redis
+      Split::Cache.fetch(namespace, key) { "value1" }
+
+      # Move time forward by 3 seconds (within interval)
+      allow(Time).to receive(:now).and_return(Time.at(now + 3))
+
+      # Second fetch - should NOT check Redis
+      expect(Split.redis).not_to receive(:get).with("split:cache:global_ts")
+      Split::Cache.fetch(namespace, key) { "value1" }
+    end
+
+    context "when cache_global_ts_check_interval is not configured" do
+      before do
+        Split.configuration.cache_global_ts_check_interval = nil
+        Split::Cache.clear
+        Split::CacheInvalidator.reset
+      end
+
+      it "does not read or write the global timestamp in Redis" do
+        expect(Split.redis).not_to receive(:get).with("split:cache:global_ts")
+        expect(Split.redis).not_to receive(:set).with("split:cache:global_ts", anything)
+
+        Split::Cache.fetch(namespace, key) { "value1" }
+        Split::Cache.clear_key(:some_key)
+        Split::Cache.fetch(namespace, key) { "value2" }
+      end
+    end
+  end
 end
diff --git a/spec/experiment_spec.rb b/spec/experiment_spec.rb
index 4faf0994..a4f4cfca 100644
--- a/spec/experiment_spec.rb
+++ b/spec/experiment_spec.rb
@@ -261,6 +261,11 @@ def alternative(color)
       experiment.winner = "green"
     end
 
+    it "should clear the cache for the experiment" do
+      expect(Split::Cache).to receive(:clear_key).with(experiment.name)
+      experiment.winner = "red"
+    end
+
     context "when has_winner state is memoized" do
       before { expect(experiment).to_not have_winner }
 

From 144b1947ac7921b859a8c46f283c3aad6324fbe3 Mon Sep 17 00:00:00 2001
From: Shinji Nakamatsu <19329+snaka@users.noreply.github.com>
Date: Sun, 30 Aug 2026 17:49:28 +0900
Subject: [PATCH 2/2] Guard cache reads against concurrent clear
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Address review feedback on thread safety: `fetch` and `clear_key`
dereferenced @cache multiple times, so a concurrent `clear` setting
@cache = nil between those reads raised NoMethodError. This becomes a
real production path with this PR, since global invalidation now calls
`clear` from within `fetch` on request threads.

Take a local snapshot of @cache (and the namespace hash) up front; a
racing `clear` now at worst writes into a detached hash, which is just
an extra cache miss. Apply the same snapshot pattern to the ivars read
twice in CacheInvalidator's predicates.

Also restore `fetch`'s original tail (`return value if value`) — the
previous rewrite was semantically identical and only added diff noise;
the falsy-value caching behavior is unchanged and tracked separately.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 lib/split/cache.rb             | 24 +++++++++++++-----------
 lib/split/cache_invalidator.rb |  8 ++++++--
 spec/cache_spec.rb             | 22 ++++++++++++++++++++++
 3 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/lib/split/cache.rb b/lib/split/cache.rb
index b8fbc4e9..01e657f3 100644
--- a/lib/split/cache.rb
+++ b/lib/split/cache.rb
@@ -13,25 +13,27 @@ def self.fetch(namespace, key)
       # Check global invalidation
       CacheInvalidator.check_and_clear_if_needed(self)
 
-      @cache ||= {}
-      @cache[namespace] ||= {}
+      # Take a local snapshot so a concurrent `clear` (which replaces @cache)
+      # cannot make us dereference nil mid-fetch. Worst case we write into a
+      # detached hash, which is simply an extra cache miss on the next fetch.
+      cache = (@cache ||= {})
+      namespace_cache = (cache[namespace] ||= {})
 
-      value = @cache[namespace][key]
-      unless value
-        value = yield
-        @cache[namespace][key] = value
-      end
+      value = namespace_cache[key]
+      return value if value
 
-      value
+      namespace_cache[key] = yield
     end
 
     def self.clear_key(key)
       # Invalidate globally for all processes
       CacheInvalidator.invalidate
 
-      # Clear from local cache immediately
-      @cache&.keys&.each do |namespace|
-        @cache[namespace]&.delete(key)
+      # Clear from local cache immediately. Snapshot @cache and its values
+      # so a concurrent `clear` cannot nil it out mid-iteration.
+      cache = @cache
+      cache&.values&.each do |namespace_cache|
+        namespace_cache.delete(key)
       end
     end
   end
diff --git a/lib/split/cache_invalidator.rb b/lib/split/cache_invalidator.rb
index 160cbea6..f0b354df 100644
--- a/lib/split/cache_invalidator.rb
+++ b/lib/split/cache_invalidator.rb
@@ -43,7 +43,9 @@ def reset
 
       private
         def within_check_interval?(now)
-          @last_global_ts_check && (now.to_f - @last_global_ts_check.to_f) < check_interval
+          # Snapshot the ivar so a concurrent `reset` cannot nil it between reads
+          last_check = @last_global_ts_check
+          last_check && (now.to_f - last_check.to_f) < check_interval
         end
 
         def fetch_global_timestamp
@@ -51,7 +53,9 @@ def fetch_global_timestamp
         end
 
         def timestamp_updated?(current_global_ts)
-          @global_cache_ts && current_global_ts > @global_cache_ts
+          # Snapshot the ivar so a concurrent `reset` cannot nil it between reads
+          known_ts = @global_cache_ts
+          known_ts && current_global_ts > known_ts
         end
 
         def update_local_state(current_global_ts, now)
diff --git a/spec/cache_spec.rb b/spec/cache_spec.rb
index fdebbe40..c81c294c 100644
--- a/spec/cache_spec.rb
+++ b/spec/cache_spec.rb
@@ -115,6 +115,28 @@
     end
   end
 
+  describe "thread safety" do
+    before do
+      Split.configuration.cache = true
+      # Disable global invalidation check for this test
+      allow(Split::CacheInvalidator).to receive(:check_and_clear_if_needed)
+    end
+
+    it "tolerates the cache being cleared while fetch is computing a value" do
+      # Simulates `clear` (from another thread, or triggered by global
+      # invalidation) landing between fetch taking its snapshot and writing
+      # the computed value back.
+      result = nil
+      expect {
+        result = Split::Cache.fetch(namespace, key) do
+          Split::Cache.clear
+          :computed
+        end
+      }.not_to raise_error
+      expect(result).to eq(:computed)
+    end
+  end
+
   describe "global timestamp invalidation" do
     before do
       Split.configuration.cache = true