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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers’s picture

Issue summary: View changes
Berdir’s picture

Note 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.

Berdir’s picture

Status: Active » Needs review
FileSize
3.31 KB

Basically 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.

Anonymous’s picture

Status: Needs review » Reviewed & tested by the community

discussed this with berdir, agree with the patch.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Agreed with removing it.

Committed/pushed to 8.0.x, thanks!

  • catch committed 723ac95 on 8.0.x
    Issue #2392235 by Berdir: ChainedFastBackend shouldn't write cache tags...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.