Keyed sticky prefetch cache, lock-free evaluation, tracking retry

[madhuchavva]

Follow-up to #128 (stacked on its branch). Branch was rebuilt after external review; see the findings comment on #127.

Summary

Removes the eval-wide lock, makes concurrent sticky-bucket fetching cancellation-safe, and replaces the originally-proposed indefinite LRU cache with JS-parity per-eval fetching plus an explicit opt-in TTL cache. Also fixes the async-tracking retry asymmetry. Net effect on the checked-in benchmark: distinct-user throughput with an async sticky service goes from ~342 rps to ~19,900 rps (~58x) with p50 request latency dropping from 310 ms to 4.3 ms — with bounded (or zero) staleness, unlike the earlier unbounded-cache draft.

What's included

• Lock-free evaluation via immutable snapshot swap — _eval_lock (the shared _context_lock) serialized every CDN-mode evaluation, including across the sticky prefetch await. _feature_update_callback now builds a NEW GlobalContext and swaps the reference atomically; each evaluation captures the current snapshot once and runs without locks:

  # before (every eval, CDN mode)
  async with self._eval_lock():
      context = await self.create_evaluation_context(user_context)
      result = core_eval_feature(...)
  
  # after
  context = await self.create_evaluation_context(user_context)   # captures snapshot ref
  result = core_eval_feature(...)

  In-flight evaluations finish against their captured snapshot; the next evaluation sees the new one (tested by swapping features mid-eval).

• Cancellation-safe per-key coalesced sticky fetch — concurrent evals with identical attributes share one inflight fetch; distinct attributes fetch in parallel (previously a single global lock serialized all fetches). Waiters await asyncio.shield(inflight): a cancelled waiter can no longer propagate its cancellation into the shared future (which made the OWNER's successful fetch die with InvalidStateError — reproduced, now regression-tested). Owner cancellation semantics are defined: the shared future is cancelled and waiters retry, one becoming the new owner (also tested).

  # waiter side
  try:
      return await asyncio.shield(inflight)
  except asyncio.CancelledError:
      if not inflight.cancelled():
          raise          # we were cancelled; owner fetch continues
      continue           # owner was cancelled; retry, maybe become owner

• Cache policy: per-eval fetch by default, opt-in TTL cache — the earlier draft of this PR cached fetched assignments indefinitely per attributes dict (LRU-bounded), which meant assignments written by another worker could stay invisible forever in a low-cardinality service, and its claimed JS parity was wrong: the JS SDK's server-side GrowthBookClient.applyStickyBuckets fetches assignments fresh for each supplied context. Default behavior now matches that: every evaluation fetches (coalesced when concurrent), so cross-worker writes are visible on the next eval. For deployments that prefer fewer service round-trips, Options.sticky_bucket_cache_ttl (seconds, default 0 = disabled) enables a bounded-staleness cache, LRU-limited by Options.sticky_bucket_cache_size; cache hits still re-apply this process's own writes from Async sticky bucket service support for GrowthBookClient #128's authoritative doc map.

• Validated cache sizing — sticky_bucket_cache_size <= 0 (or ttl <= 0) now cleanly disables caching; previously a negative size crashed evaluation with KeyError from popitem() on an empty cache (reproduced, regression-tested). This matches the existing remote_eval_cache_size convention (negative = cache holds nothing).

• Async tracking callbacks retried on failure — Async sticky bucket service support for GrowthBookClient #128 marked an experiment as tracked when the async on_experiment_viewed coroutine was scheduled; if it later failed, the impression was lost forever (a failing sync callback is retried on the next eval). The dedup key is now un-marked when the scheduled coroutine fails, restoring parity.

Measured impact (tests/scripts/benchmark_async_client.py, 100-way concurrency, 1000 requests, 1 ms service latency, default cache-off policy)

Scenario	#128	This PR	Change
async sticky service, distinct users	342 rps / p50 310 ms	19,874 rps / p50 4.3 ms	~58x
sync sticky service, distinct users	303 rps / p50 333 ms	3,051 rps / p50 32 ms	~10x
async service, hot user	18,059 rps	31,306 rps	~1.7x
no sticky service (control)	~55,000 rps	~67,500 rps	lock removal

Event-loop lag stays sub-7 ms in every scenario. The gains come from removing the locks and parallelizing fetches — NOT from caching (these numbers are with the cache disabled). The remaining sync-vs-async gap (3k vs 19.9k rps) is the default executor's thread-pool ceiling — the concrete case for implementing AbstractAsyncStickyBucketService on network-backed stores.

Behavior notes for review

• Default sticky fetch volume increases vs Async sticky bucket service support for GrowthBookClient #128 (one service call per evaluation instead of single-slot caching). This is the JS-parity correctness choice; the TTL cache is the explicit opt-out. A batched get_all_assignments override (single Redis MGET) keeps the per-eval cost to one round-trip.
• With the TTL cache enabled, cross-worker writes are masked for at most sticky_bucket_cache_ttl seconds — bounded, documented staleness rather than the earlier draft's indefinite masking.

Test plan

• Full suite: pytest tests/ -q --ignore=tests/scripts — 816 passed (809 in Async sticky bucket service support for GrowthBookClient #128 + 7 new)
• Cancellation both directions: cancelled waiter → owner completes normally; cancelled owner → waiter retries and succeeds
• Distinct users' fetches provably overlap (gated-fetch test that fails under any global lock)
• Feature update mid-eval: in-flight eval finishes on its captured snapshot; next eval sees the update
• Per-eval fetch default; TTL cache opt-in hit/LRU-eviction; non-positive size/ttl disables caching without crashing
• Failed async tracking callback retried on next eval, deduped after success
• External reviewer's probes re-run against this branch: lost-update NO LOSS, waiter-cancel NOT REPRODUCED, negative-size NOT REPRODUCED
• mypy growthbook/growthbook*.py tests/typing_probe.py --implicit-optional — clean

Out of scope (deliberately)

• Dedicated bounded executor pool for sync services (the 3k rps ceiling above); revisit on demand.
• Save retry policy for failed sticky writes (still log-only, matching JS).