Repository navigation
Atomic Redis writes, batched tag lookups and Redis in CI - #47
Merged
Merged
Conversation
Concurrent puts of the same URL could interleave their pipelines and leave the record listing a tag whose set no longer held the URL, so the next invalidation of that tag missed it. Wrap put() and forget() in MULTI/EXEC so each write's changes apply together. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
matching() read each tag's set or bucket one at a time, so invalidating a long list of tags cost one round trip per tag. Pipeline the SMEMBERS calls on Redis and use a single many() on other cache stores, in chunks of the existing chunk size. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ryanmitchell
changed the title
Add a Redis service and the phpredis extension, then run the suite a second time with CACHE_TRACKER_TEST_REDIS=1 so the Redis driver is covered end to end. The StoreTest Redis cases also run in the normal pass now instead of being skipped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ryanmitchell
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #46.
RedisStore::put()read the old record, then pipelinedHSET,SADDfor the new tags andSREMfor the removed ones. Pipelines from two connections can interleave, so two concurrent renders of the same URL with different tags could end with the record listing tag Y while the URL had been removed from set Y. The next invalidation of Y then missed the page until it was re-rendered. The realistic case is a background recache running alongside a visitor request.put()andforget()now apply their changes in oneMULTI/EXEC, so each write's record and set changes land together and the newest record always matches the sets. The read of the old record stays outside the transaction. If it's out of date, the worst case is an extra set member, which lookups already filter out.Redis Cluster connections can't run a transaction across slots, so they still send the commands one by one.
Batched tag lookups
matching()also read each tag one at a time, so invalidating a long list of tags (for example, every variant of a product) cost one Redis round trip per tag. It now pipelines theSMEMBERScalls in chunks of 1,000. The generic cache driver does the same with a singlemany()call per chunk. Cluster connections still look tags up one by one.A new test matches 2,500 tags on both drivers.
Redis in CI
The test workflow now starts a Redis 7 service, installs the phpredis extension and runs the suite twice: once as before, and once with
CACHE_TRACKER_TEST_REDIS=1so every test uses the Redis driver. The Redis cases inStoreTestalso run in the first pass now instead of being skipped. Both passes are green on PHP 8.3 and 8.4 (61 tests each).🤖 Generated with Claude Code