Problem/Motivation
Storing cache tags on the fast back-end is useless: any change results in an invalidation of the whole fast cache back-end anyway.
Worse: it's costly, because retrieving cache tag metadata also costs time and hence performance.
And finally: it also costs some room, hence for cache back-ends like APCu, it leaves less room for actual cache data.
Proposed resolution
In ::setMultiple()
:
+ // Don't write the cache tags to the fast backend as any cache tag
+ // invalidation results in an invalidation of the whole fast backend.
+ foreach ($items as &$item) {
+ unset($item['tags']);
+ }
(Copied from #2386161-14: ChainedFastBackend doesn't set 'expires' and 'tags' when writing to fastBackend.)
Remaining tasks
Put the above in a patch, and update the generic cache back-end tests to allow for tags to not be stored in the fast back-end.
User interface changes
None.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
#3 | chained-fast-backend-tags-2392235-3.patch | 3.31 KB | Berdir |
Comments
Comment #1
Wim LeersComment #2
BerdirNote that the snippet is just setMultiple(), set() and get() (for the write-through) also need to be updated.
Essentially, the decision here is if callers can expect $item->tags to be there or not.
The generic cache backend test right now ensures that we do, and making that changes breaks that test, so we need to remove that assertion.
Comment #3
BerdirBasically this patch.
Just have to decide that's OK.
It doesn't look like we even document on CacheBackendInterface what properties a cache item has.
This would also make it less problematic for #918538: Decouple cache tags from cache bins to switch to the generic, by default (for now) database-powered cache tag storage in ApcuBackend, as we don't actually use it for the chained fast backend, which is probably almost the only time you can use it reliable, as you need that even on a single server so that drush works.
Comment #4
Anonymous (not verified) CreditAttribution: Anonymous commenteddiscussed this with berdir, agree with the patch.
Comment #5
catchAgreed with removing it.
Committed/pushed to 8.0.x, thanks!