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:

$hitsAndMisses = array_map(  fn(mixed $id) => $this->cache->get($id, function (ItemInterface $item, bool &$save) {    $save = false;    return false;  }),  $ids); $hits = array_filter($hitsAndMisses);//cut: compare arrays to get missed Ids$hydrated = $this->hydrate($missedIds); //very expensive to do individually$missedValues = array_map(fn($obj) => $this->cache->get(  $obj->id,  function (ItemInterface $item) use ($obj) {    return $obj;  }), $hydrated);return array_values([...$hits, ...$missedValues]);

Unfortunately neither theArrayAdapter 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.

[Cache] Respect $save option in ArrayAdapter

5e8de1e

When $save is passed as the second option to the callback it should berespected, even in the ephemeral array adapter.

carsonbot added Status: Needs Review Bug Cache labels

Jun 17, 2022

carsonbot added this to the4.4 milestone

Jun 17, 2022

Copy link

carsonbot commentedJun 18, 2022

Hey!

I think@sbelyshkin has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

Copy link

Contributor

sbelyshkin commentedJun 19, 2022

@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.RedisAdapter::getItems() can fetch all the objects with a singleMGET, and thencommit() will send allsaveDeffered()SET operations in a pipeline. In this way you can reduce the round trip time.

$hitsAndMisses =$this->cache->getItems($ids);// produces MGET id1, id2,... for Redis$hits =$misses = [];foreach($hitsAndMissesas$cacheItem) {if ($cacheItem->isHit()) {$hits[$cacheItem->getKey()] =$cacheItem->get();    }else {$misses[$cacheItem->getKey()] =$cacheItem;    }}$hydrated =$this->hydrate(array_keys($misses));//very expensive to do individuallyforeach($hydratedas$obj) {$this->cache->saveDeffered($misses[$obj->id]->set($obj));}$this->cache->commit();// sends all SET commands in a pipeline and reads a single responsereturnarray_values([...$hits, ...$hydrated]);

get() is obviously designed for dealing with single items.
At the same time, I agree that the described behavior ofArrayAdapter looks unexpected, compared to the others.

[Cache] Respect $save option in ChainAdapter

b14aa77

nicolas-grekas approved these changes

Jun 19, 2022

View reviewed changes

Copy link

Member

nicolas-grekas left a 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.

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Jun 19, 2022

Copy link

Member

nicolas-grekas commentedJun 19, 2022

Thank you@jrjohnson.

nicolas-grekas merged commitfa4a7a4 intosymfony:4.4

Jun 19, 2022

Copy link

ContributorAuthor

jrjohnson commentedJun 22, 2022

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!