We have exactly one valid use case for invalidating a specific item in core vs delete (CacheCollector), but I don't see a use case for invalidateAll(). There is \Drupal\system\Entity\Menu::save(), but IMHO, it's mostly a naming problem because you invalidate the cache. But really you should use deleteAll() in that case IMHO, which actually deletes the entries (in the database implementation. redis is another special thing).
Quite a lot of calls in contrib (https://git.drupalcode.org/search?group_id=2&scope=blobs&search=%22-%3Ei...), most of those are just flat out wrong (cache.render invalidate all should really invalidate the rendered cache tag, that won't work for page cache and dynamic page cache)
Reason: It's an expensive thing for redis and I guess also memcache to support, because redis implements it as an extra cache tag. See #3498940: Optimize bin cache tags and last write timetamp as a related issue.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
berdirCreated a basic merge request, will need an update with the change record if others agree.
Comment #4
smustgrave commentedCan we add a CR for the deprecations and update the messages to use that link vs the issue. Since there is a replacement I'm a big fan of before/after snippets
Comment #5
berdirI tend to start off without to get early feedback. But I'm also aware that helps with explaining the reason, created one now.
Comment #6
smustgrave commentedPer slack this is fine to remove.
Just fyi wasn't against it or anything just didn't think I could make the call around it but @catch was kind enough to approve.
Verified the CR reads well, thanks for the before/after snippets.
Don't see anything else to review.
Comment #7
oily commentedI agree with #6 that the CR is quite readable. But perhaps readable in the way that Hayden would perfectly understand Mozart's explanation of a passage.
The CR is IMO a little hard to grasp for the average coder/'musician'.
I have edited it and tried to put it in terms that I understand which has the advantage of showing what I am not grasping. Using the supermarket shelf analogy, I am unsure about what happens when the item is deleted (incinerated). It seems that with Redis etc a label is placed on the shelf stating 'this item has been incinerated'? Not sure about Drupal standard caching, though.
Comment #8
berdirI'm not sure how detailed the change record needs to explain invalidate vs delete. This doesn't change the concept of invalidation or remove it, it only removes the ability to invalidate a whole bin. deleteAll() in redis has exactly the same problem as invalidateAll(), it also doesn't actually delete anything and just sets a timestamp. With redis, deleteAll() doesn't mean that cache items cease to exist.
That said, I wrote a long blog post about the background and details around some of my recent performance improvements: https://www.md-systems.ch/en/blog/2025-01-26/redis-startup-performance-i...
Comment #9
oily commented@berdir Ah, interesting it's even more complicated than i thought. Perhaps including the link would help.
That seems a really important point at least for my personal understanding.
Okay but this issue does not make any difference to that Redis problem? It is a Redis problem that is much harder to deal with? Even though the nature of the problem is similar.
I think what you are saying let's say speaking like a supermarket manager, drupal/ this shop/ when a batch of items is rotten we do not mark them as 'past sell by date' and leave them on the shelf. We incinerate them and replace the batch. So we have no need for invalidateAll()
Comment #10
berdirThe difference is that deleteAll() is needed. We need a way to completely delete all entries in a bin. What we IMHO don't need is two different ways of doing that. As I wrote in my blog post, you should only use invalidate* methods when you have a specific use case and might want to later on fetch invalidated items again. For anything else, use delete*() methods.
Then redis only needs to handle one all method and not two that vary and therefore require a different implementation and different check.
Comment #11
catchNeeds a rebase.
Comment #12
catchgitlab MR rebase was enough.
Committed/pushed to 11.x, thanks!