-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Cache] Respect $save option in all adapters #46699
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When $save is passed as the second option to the callback it should be respected, even in the ephemeral array adapter.
Hey! I think @sbelyshkin has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
@jrjohnson I don't know all the details of your case but from my perspective PSR-6 methods is the best choice for your scenario. $hitsAndMisses = $this->cache->getItems($ids); // produces MGET id1, id2,... for Redis
$hits = $misses = [];
foreach($hitsAndMisses as $cacheItem) {
if ($cacheItem->isHit()) {
$hits[$cacheItem->getKey()] = $cacheItem->get();
} else {
$misses[$cacheItem->getKey()] = $cacheItem;
}
}
$hydrated = $this->hydrate(array_keys($misses)); //very expensive to do individually
foreach($hydrated as $obj) {
$this->cache->saveDeffered($misses[$obj->id]->set($obj));
}
$this->cache->commit(); // sends all SET commands in a pipeline and reads a single response
return array_values([...$hits, ...$hydrated]);
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch @jrjohnson
I just added the commit to fix ChainAdapter.
Thank you @jrjohnson. |
Happy to help! Thanks @sbelyshkin, for the info. I need to take advantage of tags and I don't think I can do that with the PSR6 cache. I'm going to refactor our reads to use those methods though and save some significant back and forth! |
I was working with a cache chain which included the array adapter. When we have a cache miss we need to consolidate all of the misses so we can do a single DB query for all values. So first we look in the cache, then and then we can pull the data as a big group, hydrate the results, and then cache them. Because we search for these values a bunch of times in the same request I chained the array and Redis adapter together.
Looks like:
Unfortunately neither the
ArrayAdapter
nor theChainAdapter
respect the $save reference. I was able to fix theArrayAdapter
in this PR, but I'm at a loss for how to do the same for theChainAdapter
, but I was able to create a generic failing test.