Mitigate stale cache across processes (related to #735)

[snaka]

Related to #735.

Background

When config.cache = true is enabled in a multi-process setup (e.g. Puma cluster mode), each process maintains its own in-memory cache via class instance variables. Mutations made in one process — for example setting a winner from the Dashboard — are not propagated to other processes. As a result, reloads can flip between the new and old state until every worker is restarted.

This PR does not eliminate the underlying single-process design, but it bounds the staleness window so the inconsistency naturally resolves within a few seconds without a process restart.

Solution

A single Redis key (split:cache:global_ts) acts as a shared invalidation signal across processes. Split::Cache.clear_key — already called from Experiment#reset and Experiment#reset_winner, and now also from Experiment#winner= — bumps this timestamp; other processes detect the change on their next throttled read and drop their snapshot.

When enabled, this means cross-process propagation works for the Dashboard "Use as winner", "Reopen", and "Reset" actions.

The mechanism is opt-in (controlled by cache_global_ts_check_interval), so existing applications see no change in caching behavior.

Object relationships

flowchart LR
    U(("User"))

    subgraph Rails["Rails"]
        Ctrl["Controller"]
    end

    subgraph SplitGem["Split"]
        direction TB
        IMC["In-memory cache<br/>(@cache)"]
        CI["CacheInvalidator<br/>(@global_cache_ts,<br/>@last_global_ts_check)"]
    end

    subgraph RedisDB["Redis"]
        direction TB
        RC["experiment_winner<br/>(authoritative data)"]
        GT["global_ts<br/>(invalidation signal)"]
    end

    U -- "HTTP" --> Ctrl
    Ctrl -- "ab_test / winner=" --> IMC
    IMC -- "check before serve" --> CI
    IMC -- "HGET on miss" --> RC
    IMC -- "SET on clear_key" --> GT
    CI -- "GET (throttled)" --> GT

Loading

• In-memory cache lives per process (Puma worker). Serves cached values; falls back to Redis on miss.
• CacheInvalidator holds the last-seen global_ts and is consulted before each cache read. When enabled, it re-reads from Redis at most once per cache_global_ts_check_interval seconds, which also bounds the cross-process staleness window.

Configuration

Split.configure do |config|
  config.cache = true

  # Optional. Default: nil (mechanism disabled).
  # When set, processes re-read split:cache:global_ts at most once per N seconds
  # and drop their in-memory cache if it has been bumped by another process.
  # The value is the upper bound on cross-process staleness (in seconds).
  config.cache_global_ts_check_interval = 10
end

Backwards compatibility

No breaking changes. The new mechanism is opt-in; config.cache = true alone keeps the previous behavior. config.cache = false continues to bypass the cache entirely.

Note

The straightforward workaround for the staleness issue — setting config.cache = false — eliminates the inconsistency but also gives up the performance benefit caching provides on hot paths. For high-traffic applications this is a costly trade-off, and flipping it off in production carries its own operational risk. This PR aims to provide an opt-in mitigation that keeps the cache enabled while bounding cross-process staleness, so users can address the symptom of #735 without disabling caching wholesale.

[vladr]

Building on top of "broken" Split::Cache behavior: if yield returns a falsy value, fetch never caches it.

Namespace	When block is falsy
:experiment_winner	nil when Redis has no winner (hget miss) — explicit else nil
:experiment_start_times	nil when no start time in Redis
:experiments	Missing experiment uses return unless exists? inside the block → non-local return from find, so fetch never stores anything.

[snaka]

Confirmed — all three cases behave as you describe (the non-local return in Experiment.find included). This is pre-existing behavior on main that this PR does not change; I have reverted the cosmetic rewrite of fetch's tail (144b194) so the diff no longer touches those lines. Fixing falsy caching interacts with the invalidation design — today the nil winner is effectively uncached, so "no winner → winner set" is never stale cross-process; caching nil would widen the staleness window this PR bounds — so I would like to address it in a follow-up PR. I can open an issue to track it.

[snaka]

Opened #762 to track this.

[vladr]

Building on top of "broken" Split::Cache behavior: clear is not thread-safe, can cause undefined method '[]' for nil (NoMethodError) to be raised from fetch executing in a different thread (e.g. if clear sets @cache = nil in thread A after fetch performed @cache ||= {} but before @cache[namespace][key] in thread B).

[snaka]

Fixed in 144b194. fetch and clear_key now take a local snapshot of @cache up front, so a concurrent clear can no longer cause the NoMethodError — at worst a racing thread writes into a detached hash, which is just one extra cache miss. Applied the same pattern to the ivars read twice in CacheInvalidator's predicates. Added a deterministic regression spec (clear landing mid-fetch), which raises NoMethodError on the previous code.