Fence Redis cache fills after invalidation#76
Conversation
Greptile SummaryThis PR adds a Redis-level fencing mechanism to prevent stale cache fills after
Confidence Score: 4/5Safe to merge for Redis and RedisCluster deployments, but Multiplexing-backed caches will have fencing entirely disabled until the numkeys fix lands. The fencing logic in Redis.php and RedisCluster.php is sound, and the wrapper forwarding is correct. However, the Multiplexing adapter's saveFenced() has a one-character off-by-one in the EVAL numkeys declaration that causes a Lua runtime error on every invocation — meaning any production deployment that uses the Multiplexing adapter will silently fall through to false on every post-miss save, defeating the entire fencing guarantee for that adapter. src/Cache/Adapter/Redis/Multiplexing.php — the saveFenced() method needs numkeys changed from '3' to '4' before this is safe for Multiplexing-backed deployments. Important Files Changed
Reviews (4): Last reviewed commit: "Handle Redis purge and flush fences" | Re-trigger Greptile |
|
Follow-up on the two Greptile notes that did not result in code changes:
|
|
Follow-up on the latest Greptile notes that are intentionally not changing this PR:
|
|
Update after commit 091d544: the previous note saying the flush and purge-token Greptile P1s were intentionally out of scope is now obsolete. Both are handled in this PR now: Redis-family flushes write a short-lived flush fence, and successful same-instance purges clear matching pending fill tokens before the next save. |
| $result = $this->command([ | ||
| 'EVAL', | ||
| $script, | ||
| '3', | ||
| $key, | ||
| $this->getTombstoneKey($key, $hash), | ||
| $this->getTombstoneKey($key, '*'), | ||
| $this->getFlushKey(), | ||
| $hash, | ||
| $token->value, | ||
| $value, | ||
| (string) self::TOKEN_TTL, | ||
| (string) \time(), | ||
| ]); |
There was a problem hiding this comment.
saveFenced passes numkeys='3' to EVAL but the Lua script reads KEYS[4] for the flush key. In Redis EVAL, elements beyond numkeys are treated as ARGV, so getFlushKey() lands in ARGV[1] instead of KEYS[4], and KEYS[4] is nil. Calling redis.call('GET', nil) raises a Lua error, which causes EVAL to fail on every fenced-save call through the Multiplexing adapter — silently breaking all post-miss saves. Compare with Redis::saveFenced(), which correctly passes numkeys=4.
| $result = $this->command([ | |
| 'EVAL', | |
| $script, | |
| '3', | |
| $key, | |
| $this->getTombstoneKey($key, $hash), | |
| $this->getTombstoneKey($key, '*'), | |
| $this->getFlushKey(), | |
| $hash, | |
| $token->value, | |
| $value, | |
| (string) self::TOKEN_TTL, | |
| (string) \time(), | |
| ]); | |
| $result = $this->command([ | |
| 'EVAL', | |
| $script, | |
| '4', | |
| $key, | |
| $this->getTombstoneKey($key, $hash), | |
| $this->getTombstoneKey($key, '*'), | |
| $this->getFlushKey(), | |
| $hash, | |
| $token->value, | |
| $value, | |
| (string) self::TOKEN_TTL, | |
| (string) \time(), | |
| ]); |
| $result = $this->command([ | ||
| 'EVAL', | ||
| $script, | ||
| '3', | ||
| $key, | ||
| $this->getTombstoneKey($key, $hash), | ||
| $this->getTombstoneKey($key, '*'), | ||
| $this->getFlushKey(), | ||
| $hash, | ||
| $token->value, | ||
| $value, | ||
| (string) self::TOKEN_TTL, | ||
| (string) \time(), | ||
| ]); |
There was a problem hiding this comment.
The
EVAL call declares numkeys='3', but the Lua script reads KEYS[4] to fetch the flush marker (local lastFlush = redis.call('GET', KEYS[4])). With only 3 keys declared, KEYS[4] is nil in Lua, and redis.call('GET', nil) raises a Lua runtime error. As a result, every call to saveFenced() on the Multiplexing adapter fails unconditionally — all post-miss fills go to the false branch — silently disabling fencing for this adapter. The standalone Redis adapter correctly passes 4 here, and the same fix is needed for Multiplexing.
| $result = $this->command([ | |
| 'EVAL', | |
| $script, | |
| '3', | |
| $key, | |
| $this->getTombstoneKey($key, $hash), | |
| $this->getTombstoneKey($key, '*'), | |
| $this->getFlushKey(), | |
| $hash, | |
| $token->value, | |
| $value, | |
| (string) self::TOKEN_TTL, | |
| (string) \time(), | |
| ]); | |
| $result = $this->command([ | |
| 'EVAL', | |
| $script, | |
| '4', | |
| $key, | |
| $this->getTombstoneKey($key, $hash), | |
| $this->getTombstoneKey($key, '*'), | |
| $this->getFlushKey(), | |
| $hash, | |
| $token->value, | |
| $value, | |
| (string) self::TOKEN_TTL, | |
| (string) \time(), | |
| ]); |
What
Fixes stale Redis cache fills after invalidation by fencing Redis-backed cache misses internally.
The problematic sequence is:
load()and misses.purge()orflush().save().This PR keeps the public cache API unchanged. Callers still use
load(),save(),purge(), andflush()normally; the cache layer tracks an internal token for Redis miss fills and Redis only accepts the follow-up save when that token is still valid.Why this is narrow
This intentionally keeps the fix Redis-focused:
Redis,RedisCluster, andRedis\Multiplexing.Adapterremains unchanged; the new behavior is behind a smallFeature\FencedFillcapability.Pool,Sharding, andCircuitBreakeronly forwardFencedFillwhen their inner adapter supports it, so Redis-backed wrapper deployments keep the same protection.Changes
TokenandFeature\FencedFilltypes.Cacheto remember fill tokens only when the top-level adapter supportsFencedFill.purge(), including all known hashes for key-level purge.flush()implementations so pre-flush absent tokens cannot save after a flush.Redis behavior
load()still returnsfalseon misses.Cache::load()receives an internal token on Redis misses and returnsfalseto callers.Cache::save()consumes the matching token and callssaveFenced()only for Redis fenced fills.TOKEN_TTLseconds.Greptile follow-up
Latest Greptile P1s are now handled:
Redis::flush()andRedis\Multiplexing::flush()now performFLUSHDBand marker write in one Lua script.RedisCluster::flush()writes a marker after flushing masters andsaveFenced()checks that marker before and after the per-key fenced save.Cache::purge()now clears pending local fill tokens for the purged key/hash, or all known hashes for key-level purge.Earlier Greptile notes:
FencedFillthroughPool,Sharding, andCircuitBreakerwhen supported by the inner adapter.TOKEN_TTLduring fenced saves.flush()increments the shared token generation, so old coroutine-context tokens are no longer looked up by later saves.>=8.3incomposer.json.Validation
Passed locally:
php -lon all touched PHP filesvendor/bin/phpunit --configuration phpunit.xml tests/Cache/Unitvendor/bin/phpstan analyse --level max src tests --memory-limit 2Gvendor/bin/pint --test ...on touched filesRedisflush fencing:FLUSHDBis accepted insideEVALCould not run the repo Redis E2E files from this host because the expected Docker DNS names are not resolvable here:
redisredis-clusterThe Redis E2E attempt failed in setup/connection before assertions ran. Pint passes, but PHP 8.5 emits deprecation noise from Pint’s bundled dependencies.
Risk
Risk is limited to Redis-backed cache behavior and wrappers around Redis-backed adapters. The main compatibility surface is that Redis
load(), fencedsave(),purge(), and standaloneflush()now execute Lua scripts, and Redis invalidations keep short-lived tombstone/flush markers forTOKEN_TTLseconds.