ChainedFastBackend invalidates all items when cache tags are invalidated

Testing the patch on default umami installation frontend as admin with disabled render caching, I'm seeing 6 extra queries to the cachetags table (142 vs 148 in total). Time difference of that is too small is way too small to reliably profile, but it's still something to consider.

The specific tags are, in order of appearance:
* routes (non admin routes)
* entity_types (also entity_bundles and lots of entity_field_info later, but all of them would be needed anyway sooner or later, so doesn't really count)
* element_info_build
* theme_registry
* breakpoints
* local_tasks
* library_info

That's 7 with entity_types. I think we had discussions before about whether or not we should use cache tags for some kind of plugins/caches. I'd suggest opening a follow-up that we remove cache tags for finite and known variations (langcodes, themes), that would remove library_info, element_info_build and theme_registry. breakpoints is a bit trickier areay, groups there are more dynamic, but we also have an API to fetch them (\Drupal\breakpoint\BreakpointManager::getGroups). Alternatively, we could merge these into one, it's rare that they are invalidated and invalidating a few extra thingss might be worth saving 2-3 queries on every request. Not sure how much cache tags are an API though.

That said, IMHO it's fine to move ahead with this, setting to needs work for the documentation updates.

I couldn't find an existing issue for removing cache tags from plugin managers, although definitely remember talking about it somewhere, so opened a new one for that 📌 Manually clear cache keys from plugin managers with finite variations instead of using cache tags Fixed .

I just found this old issue, where most of us had similar discussions 6+ years ago :)

📌 ChainedFastBackend should not invalidate the whole fastBackend when doing a Set() Needs work

I seem to have derailed this with the request for documentation, here's a proposed sentence for that. I also created https://www.drupal.org/node/3344524 → .

I think it should throw deprecation if no interface support for backend implemented

There is no interface that we could check, there is no explicit API or anything, we always pass in cache tags, it's up to the implementation to decide how to handle that. We discussed that above, introducing something would IMHO just cause existing implementations to break, I'm not aware of a single cache implementation that would be affected by this. This is only about the fast backend, I'm not sure if there even is an alternative to APCu out there, I've certainly never used something else.

Redis module now supports Relay and that comes with a built-in APCu-like cache, but then you want to replace ChainedFastBackend completely, not just the fast backend.

+++ b/core/lib/Drupal/Core/Cache/ChainedFastBackend.php
@@ -244,7 +237,9 @@ public function invalidateTags(array $tags) {
-    $this->markAsOutdated();
+    if ($this->fastBackend instanceof CacheTagsInvalidatorInterface) {
+      $this->fastBackend->invalidateTags($tags);
+    }

If we are concerned about (hypothetical) fast backends that don't implement tags can't we just say

    if ($this->fastBackend instanceof CacheTagsInvalidatorInterface) {
      $this->fastBackend->invalidateTags($tags);
    }
    else {
      $this->markAsOutdated();
    }

This whole discussion made me wonder if we were reinventing the wheel, and we do seem to be: Symfony supports database, APCu, filesystem, memory, Redis, Memcache and other backends, and allows chaining too: https://symfony.com/doc/current/components/cache/adapters/chain_adapter....

Symfony also supports cache tags in an apparently cleverer way than we do; you store tags in another cache adapter, but it can be a different storage backend to the one that the actual items are in: https://symfony.com/doc/current/components/cache/cache_invalidation.html

I couldn't find an issue for this, worth opening one to explore?

I think their chain adapter is our more generic BackendChain, I don't see anything there that specifically supports the sync feature of the ChainedFastBackend, but I only had a quick look at it.

The cache tag implementation seems somewhat comparable with cache tag versions that seem similar our checksums. I guess if you use the same tag pool for different adapters, you should get the same behavior as we have, but it will fetch cache tags from the adapter for every bin/pool. For the record, I'm fairly certain we invented that particular wheel quite some time before Symfony did, but that doesn't really matter.

I'm open to exploring to use their cache system, but that seems a like a _big_ issue and I'm sure we'll hit a fair share of roadblocks on the way there. (Like the database transaction topic) And even if we do that, we'd either need to contribute this back or re-implement it as an Adapter, so this is an improvement we want to do either way.

This whole discussion made me wonder if we were reinventing the wheel

I feel it's necessary to point out that it's Symfony that re-invented the wheel, we've had cache tags support since 2012, theirs was added in about 2016. We tried to get cache tag support into the PSR cache spec when it was in progress, and that was rejected at the time - looks like they've figured out a way around that though.

Question before reviewing (attempting) will this require tests?

We didn't have tests before about the explicit internal behavior of ChainedFastBackend, or we would have needed to adapt them. We could add them, but I'm not sure if there is a benefit to that.

We have plenty of test coverage that test that ChainedFastBackend overall works as expected (blackbox testing) as basically every functional/browser test is using it. As evident by the 100 fails in the first inconsistent patch.

I think this is ready - should be 10.1-only with a change notice, but shouldn't be disruptive and should be a good performance improvement on real sites.

The last submitted patch, 23: 3334489-22.patch, failed testing. View results →

Random failure

> Various plugin manager related cache tags removed
Why was this mixed in here?

With all due honor for the work done here, setting NW for the reasons elaborated in #2241377-124: [meta] Profile/rationalise cache tags →

That is not mixed in here, that is a related draft change record that just mentions this issue as related, as it is the reason for doing it.

The last submitted patch, 23: 3334489-22.patch, failed testing. View results →

Random test failure.

The last submitted patch, 23: 3334489-22.patch, failed testing. View results →

Another random test fail I assume, in 1) Drupal\FunctionalJavascriptTests\Ajax\ThrobberTest::testProgressThrobberPosition.

The last submitted patch, 23: 3334489-22.patch, failed testing. View results →

random test failure

Technically as a bug fix this is eligible for 10.1.x but I'm kinda wary about doing this now we are past rc just in case we are breaking something subtle; I still find it hard to understand all layers of the cache subsystem. So let's get this into 10.2.x and it can have six months of testing before it gets into real sites.

Committed and pushed 0448b4bcc77 to 11.x (10.2.x). Thanks!

Yeah, makes perfect sense to not backport this. This will also become a clearer improvement if we do 📌 Manually clear cache keys from plugin managers with finite variations instead of using cache tags Fixed as well and that definitely won't go into 10.1.

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

Published CR.