Cache tag support

subscribing, and noting that the conversation above is flipped - lines starting with 'catch:' are from me, lines starting with 'sdboyer:' are from catch.

edit: this is now fixed

Subscribe, this approach looks very promising! If D7 going more entity-related so this could go. Right now this could be done in contrib like clearing cached views on some events.

This is a very interesting approach, and I think I like it. One interesting addendum to this approach is that hook_cache_info could be used to create schema for cache tables, so that modules wouldn't have to handle that. Right now that's a trainwreck waiting to happen if the cache table schemas ever change again.

Subscribe. Lets release D8 right after this gets in :)

I think we want the ability to fire multiple callbacks for an event like: cache_invalidate('entity', 'node', 'insert', $nid). Perhaps a model like FAPI submit handlers is what we want here.

I think better use arrays

cache_invalidate ( array( $bin1 => array($tags1), $bin2 => array($tags2) ) ); 

Subscribing. Looks very cool.

There's a great discussion - Drupal Cache Roadmap for D8 http://groups.drupal.org/node/75823

I just found this, and the two conversations are separate, but any decisions made here absolutely affect the D8 cache API roadmap. Let's pull these two conversations together and i'll update the wiki page to reflect the hooks and cache API features we're talking about adding.

This naming sounds a bit cryptic currently, my only suggestion would be to spend some time to make sure we have clear naming conventions.

+1

subscribing

Note that we badly need a quick fix for D7, without an API change. I've opened a separate issue and submitted a stupid-simple proposal in a patch: #918538: Decouple cache tags from cache bins

Subscribing. Would also find this very useful in several contribs. Maybe we can experiment in contrib for D7?

+1 to experimenting in contrib. I was actually noodling around with some ideas the other day, thinking about a contrib module for them...

subscribing

subscribe as I'm always hacking around drupal's cache clear all in my various modules. I've been quite successful at it I might add.

D8 is open for business. Anyone willing to flesh this out a bit more? If so, assign this issue to yourself.

This is a hard problem for sure. I do think a new table like {cache_trigger} is the only way to track exactly cache items and events which invalidate them.

I recall that mysql is (was?) really inefficient with subqueries like DELETE FROM {cache_foo} WHERE cid IN (SELECT cid FROM {cache_trigger} WHERE trigger IN (:triggers));. Needs investigating.

Also, I think that nearly every cache item is going to record triggers which will extremely rarely ever activate. Stuff like language and view mode. We still have to multi-insert these over and over again. I guess I should not sweat it but it is a bit of a bummer for slower cache backends like DB.

I can't think of a better name than trigger right now. Another way to think of these are 'the list of potential changes which would cause a given cache item to become invalid and thus get deleted'. i kinda like 'causes of death' but thats a bit too cheerful :)

The proposal is in the right direction, but limiting this whole thing to be a workaround to improve our cache invalidation doesn't make sense to me. If you remove the word "cache" from the last follow-ups, then we are at the actual and essential point: We need a system that is capable of tracking what is being involved where.

Caching and cache invalidation is just one use-case for this information. The very same information is required (and badly missing) for countless of other situations and operations. Be it module updates that need to figure out what is affected by a certain change and needs to be updated, or be it administrative lookups/audits that need to investigate what is possibly going to be affected by a configuration change. All of this (and more) requires the same kind of internal and reliable data source.

Therefore, it would be a huge gain for Drupal at glance if we'd slightly shift the focus and try to solve the what is being involved where problem, which apparently is the heart and primary challenge of the cache invalidation issue. If that info would exist, proper cache invalidation would be a lot easier.

For starters, we already have a simple/suboptimal involvement tracker with the file usage API.

Agreed with @sun that we need to focus on involved bricks of system. If the butler approach with context will be realized by this way we get context + actual render (or usage of cached item)

Cache tags has a long story but the main trouble with memcache that tags periodically are stale from cache because of nature of memcache (LRU)

Another way I'd like to point into issue #1120928: Move "history" table into separate History module

Core already have node-history table so we can interpolate this approach to cache:
- extend history support for core entities
- run cache clean depending on historical usage of entity within context of block, view and whatever...

I still have to do some catch-up reading on this issue, but I wanted to mention this in light of @sun's "what is being involved where" comment.

The FlexiCache project provides something like this. You can associate users, nodes, comments, etc. with cache entries via a metadata database. Then clear the cache via those entity IDs, rather than cache IDs or "general wipes". This system sits on top of the traditional cache API as a wrapper, so it's an API addition rather than a total shift.

The main thing holding this back is that the current implementation depends on MongoDB as a metadata backend. That makes it easy to create indexed arrays and query with arrays. I have FlexiCache running on a fairly large site right now, and it has improved our cache hit rate (particularly for block cache) by a *huge* amount.

I read #30 now and it basically describes FlexiCache. The key idea is that wildcards are very clumsy to query on, and need to be phased out in favor of a more structured (and query-able) array of metadata.

In catch's example, cache_clear_realm(array('entity' => $taxonomy_term)); would be equivalent to the Flexicache syntax: flexicache_clear_all(array('terms' => $tid)). Instead of "realms" I called them "properties" (although I tend to think of them as tags).

FlexiCache has been running in production for 4 months on a social network, and although the garbage collection aspect could be improved a bit (there are 400k+ metadata objects), the concept is working in practice, our cache lifetimes are much better, and I have confidence that this would be a really good core improvement. The only challenge is making this work in SQL without subselect chaos raining down on everyone :)

@catch: I think this is a great topic and am looking forward to sample code for review.

catch pointed me here for reference. The tags/triggers idea sounds logical to me, and the fact that the other major frameworks haven't been able to do any better means we're probably on the right track. :-) By and large I agree, at least in concept, with what catch is describing. I've not looked at any implementation yet.

As far as butler/context/WSCCI, I agree that should not be a blocker here. Rather, the cache system itself should be able to handle "tags", and then we should, later, try to figure out a good way to automate what the right tags are to use based on context data. For blocks, one thing I've discussed with David Strauss and Sam Boyer is having blocks declare the context keys that they will need. Deriving that information dynamically will be impossible. So if a given block declares that it depends on a node and the language, then you can reliably cache it based on nid and language, and use nid and language as Varnish cache keys, etc. Then we could cache blocks using what catch describes automagically, using that same meta information.

Perhaps we could expand that concept? That is, are there other "cachable thingies" that we could conceivably declare up front vary based on context keys X, Y, and Z? If so, then we could automate their caching based on those keys. Views queries, for instance, I once discussed with Earl as being something we could conceivably cache if we can reliably cache "plugins", or some subset of plugins. A views query object varies based on the arguments fed to it. If there are no arguments, great. If there are, then it varies based just on that. (OK, that's an over-simplification, but you hopefully get the idea.)

If we could define "thing thingie is cachable", then we could declare up front "and these are the keys that matter", and then automatically cache based on that data with no further user involvement. Then we just need to make sure that lots of thingies meet whatever criteria that is.

subscribe

Subscribing...

Some interesting Varnish-related ideas at http://pooteeweet.org/blog/0/1979#m1979

@moshe: Very interesting. So in a Drupal context, that would entail things like adding an html header that showed which nids were in a given View, so that the Views-generated page could be invalidated if any of those nodes had been updated?

I don't know if there's anything core could do to support something like that though - seems like it would be best in a contrib module.

In boost I use the boost_cache_relationships table to keep track of what nodes show up on the pages. Works well, but is a little DB intensive as a page with 10 nodes on it, creates 10 entries in the database; boost_schema(). This is the reason you see people running with Boosted Varnish. Its the same setup we are using at my work; we run don't even use the .html files boost creates unless we have a "bad event" (happens about 2x a year).

I support this. If your driver cant deal with an unstructured fugly DELETE statement then... well do nothing as the code defaults to no DML JOIN anyways :) protected $joinDMLSupport = FALSE;

+++ b/includes/cache.incundefined
@@ -304,6 +310,15 @@ interface DrupalCacheInterface {
+   *   An array of tags grouped by namespace. The array follows the same format
+   *   as that used to in DrupalCacheInterface::set().
+   */
+  function deleteTags($tags);

Don't we have to override this method too in DrupalFakeCache? Will probably not be called, but might still be a good idea...

+++ b/includes/cache.incundefined
@@ -485,8 +515,42 @@ class DrupalDatabaseCache implements DrupalCacheInterface {
+
   function flush() {
     db_truncate($this->bin)->execute();
+    db_truncate($this->bin . '_tags')->execute();

What happens if we call flush() (or deleteTags()) on a cache bin that does not have a tags table defined? Should we check with a tableExists()?

Or alternatively, could we even only have a single cache_tags table which would have an additional 'bin' column? No idea how much slower that would be...

EDIT: Crosspost with catch :)

#73: cache_tags_4.patch queued for re-testing.

#73: cache_tags_4.patch queued for re-testing.

Looks very promising to me. I think that's the right direction.

Although you clearly showed the disadvantages, we hopefully will have much less clear-everything cases in future. This will allow much better long-running performance. It's just very important to eliminate all those clear-all cases in regular code to make the cache perform better for future.

EDIT: If a caching system will support tagging natively... Will we be able to cover that with best performance? :-)

I haven't figured out the exact mechanism, but this could probably help improve the cache efficiency of #152901: Caching (per user) for authenticated users quite a bit.

I'm starting to work on FlexiCache 2.x (http://drupal.org/project/flexicache) which will store the cache data in Mongo, have memcache-like version flushing for tags and wildcards, and no longer require a core patch (I hope). I need to look at the SQL patches here and take ideas from that, and possibly have an approach that will work for both SQL and noSQL.

Untagging.

Looking good.

1. Is varchar 64 enough room for the tag value?
2. I'm not clear on why block.install and all the others hard code DatabaseCache like so: '$cache = new DrupalDatabaseCache('block');'. Couldn't they choose to use non-DB cache backend? Block is not a particularly special module anymore.

A UUID is a 16-byte (128-bit) number. But most database storage engines can manipulate with UUID by COLUMN definition.

I think each cache definition(_info) could define some limits (native_UUID = TRUE, key_limit = 64, chunk size = 1M) also I lke to remember about cacherouter module - each router have it's own key generation override

#73: cache_tags_4.patch queued for re-testing.

Quick re-roll after #1275684: Image module creates a cache bin then never uses it

For the sake of experimentation, I'm introducing a couple of alternate implementations.

The concept is to do simple invalidation of tags instead of synchronously deleting all the data on a cache clear. This shifts some weight to cache get/set, since the get needs to check against the current "version" of a tag, and the set may need to get/attach the current "version" to the data record. However, I think we still win in the end.

Having run a Mongo-based cache tags system in production for most of 2011, cache clears have been the biggest bottleneck, as I was using a "select all from these tags" -> "delete all that stuff" combination which frequently ends up in unhappy cursors and timed out requests once there is a good deal of tagging going on. Shifting to a lightweight invalidation scheme I think is the way to avoid that. It will also make cache tags very do-able in simple key-value stores, Mongo, Redis, whatever :)

RE: the two approaches, I prefer the counter-based approach because it avoids weirdness when time sync is slightly off between servers. The timestamp approach has faster cache_set() though, since it doesn't need to query for the latest tag checksum.

Of course, benchmarks should be done. I will try to get some up for baseline vs. catch's, baseline vs. each approach here, and breakdowns of performance loss or gain for each cache operation. The benchmarks need to operate on a large dataset to be realistic; I may be able to export my production cache tags from Mongo to play with.

Oops, fixed cache-install invalidate() method name.

carlos8f if the decision is not clear, why not having both strategies available but configurable?
I'm not sure if we'll find the perfect decision here that matches best for all cases.. Are you sure we will?

The DB schema in my patch is very different from catch's, that would make switching between the two difficult. Plus it would be a lot of baggage to carry around multiple implementations. I am just trying to play devil's advocate and see if there's interest in other approaches.

I ran some benchmarks:

#!/usr/bin/php
<?php

timer_start('set');
for ($i = 0; $i < 10000; $i++) {
  cache_set("test-$i", NULL, 'cache', CACHE_PERMANENT, array('user' => array(1, 2, 3, 4), 'node' => array(1, 2, 3, 4)));
}
timer_stop('set');

timer_start('get');
for ($i = 0; $i < 10000; $i++) {
  cache_get("test-$i");
}
timer_stop('get');

timer_start('clear 1');
cache_clear_tags(array('node' => array(3)), 'cache');
timer_stop('clear 1');

timer_start('clear multi');
cache_clear_tags(array('user' => array(1, 2, 3, 4), 'node' => array(1, 2, 4)), 'cache');
//cache_clear_all('test', 'cache', TRUE);
timer_stop('clear multi');

unset($GLOBALS['timers']['page']);
var_dump($GLOBALS['timers']);

I ran this 3 times for each patch, and took the middle result for each stat.

(numbers expressed in milliseconds)

Although the approaches are only semi-comparable, we can see some of the theoreticals expressed in numbers here. Subquery/JOIN-based tag clearing is clearly much more expensive than simple wildcard clearing. Curiously, the subquery was always faster than the JOIN on my system. There is also significant overhead in multi-inserting tags for each cid, whereas my method keeps that low by storing them in a serialized field and having one row per tag.

The overhead in querying/computing the checksum is actually lower than I thought, so I'm going to discontinue the timestamp-based approach in favor of the checksum. The performance of checksum-based tags is only *slightly* worse than no tags at all, so it looks very promising if we can work out garbage collection.

It's also worth noting that ideally tags can be cleared across multiple bins. My patch stores tags separately from cid's or bins, making that trivial. I need to tweak the patch to implement that though, since the cache class is so coupled with the bin currently.

I have made some fixes and tweaks to catch's patch and mine to do the tests, so I am posting that also. The main bug in catch's patch was that it only cleaned up tag records in non-JOIN mode (probably skewing benchmarks in the JOIN favor), and the null cache needed fixing.

Followed up on the cross-bin clearing, this version abstracts cache tag stuff into its own class.* That allows cache tag storage to be extendable and still able to expire across multiple bins.

Cleared out some of the cruft from previous patches, and expanded on the tests to include multi-bin expiration. No longer includes #1167144: Make cache backends responsible for their own storage.

* needs docs :)

I've started a contrib project to follow this issue: Cache Tags

I also have a working Mongo cache backend with tag support committed there, and it's fast! Benchmark using #93 method:

set() - 0.068ms
get() - 0.16ms
expire_tagged() single - 0.06ms
expire_tagged() multi - 0.26ms

Whew, blazing!

Updated patch, renamed cache_expire_tagged() to cache_invalidate().

Deployed the 6.x Mongo version of Cache Tags yesterday to a production site, and so far there have been 500k entries across 18 bins in the cache, 38k tags and 546k tag invalidations. Server load is very low, transfer rate is about 36Mb/s, and the site's overall speed is much improved. It's working so smoothly, I am a little shocked :)

Edit: after the weekend we've hit 1.5m cache entries, 124k tags and 2.8m invalidations. Working beautifully.

+++ b/includes/cache.incundefined
@@ -616,3 +639,84 @@ class DrupalDatabaseCache implements DrupalCacheInterface {
+
+interface DrupalCacheTagsInterface {
+  function __construct();
+
+  function invaldate($tags);
+
+  function checksum($tags);
+
+  function isValid($checksum, $tags);

Captain Obvious here, documentation missing :)

Also, having an empty __construct() in the interface seems pointless, if nothing is passed to it then there is no requirement to implement it?

Also, typo: "invaldate"

+++ b/includes/cache.incundefined
@@ -616,3 +639,84 @@ class DrupalDatabaseCache implements DrupalCacheInterface {
+  function flatten($tags) {
+    if (isset($tags[0])) return $tags;
+    $flat_tags = array();
+    foreach ($tags as $namespace => $values) {
+      foreach ($values as $value) {
+        $flat_tags[] = "$namespace:$value";
+      }
+    }
+    return $flat_tags;

Missing documentation too here, and should maybe be protected?

+++ b/includes/cache.incundefined
@@ -616,3 +639,84 @@ class DrupalDatabaseCache implements DrupalCacheInterface {
+class DrupalDatabaseCacheTags extends DrupalCacheTags implements DrupalCacheTagsInterface {
+  protected $tag_cache = array();

You could have the abstract class implement the interface, then you don't need to repeat the implements part.

+++ b/modules/system/system.installundefined
@@ -673,6 +673,18 @@ function system_schema() {
+      'tags' => array(
+        'description' => 'Space-separated list of cache tags for this entry.',
+        'type' => 'text',
+        'size' => 'big',
+        'not null' => FALSE,
+       ),
+      'checksum' => array(
+        'description' => 'The tag invalidation sum when this entry was saved.',
+        'type' => 'int',
+        'not null' => TRUE,
+        'default' => 0,

Not sure if completely understood the schema changes but this is certainly missing update functions and seems to only change the {cache} bin.

I guess we'd want to wait with this stuff until we decided who manages database cache bin creation and when it's done. Because this brings up an interesting point, namely schema updates...

-22 days to next Drupal core point release.

Thanks for the review. All good suggestions.

- I added docs!
- took out empty __construct()
- fixed silly method name typo
- flatten() is now a static function (making it protected prevents DrupalDatabaseCacheTags from calling it)

Haven't dealt with the schema update issue, and we might want to leave that to #1167144: Make cache backends responsible for their own storage. I'm not sure if making a special hook for cache bins is really good, since that is making dependencies on the module API for the cache API. However it seems like it doesn't block this issue since we can just provide a hook_update_N() which alters in new fields for cache_* tables in the mean time.

And...

Added upgrade path, and this time the bot should pick it up.

It won't be exactly an additional query per cache get, but rather, an additional query per tag on cache entries fetched per page load. Individual tag invalidation counts are statically cached, and summed with counts fetched from the db. This is very cheap stuff though, as the query component of a cache get takes less than 1/5 of a millisecond on a humble Mac Mini.

So far in production the system has been scaling very well, and at over 2.8 million tag invalidations, and 40 200+ queries per second, the cache server load is typically at 0.00. I think it works! :)

+++ b/includes/cache.incundefined
@@ -616,3 +639,175 @@ class DrupalDatabaseCache implements DrupalCacheInterface {
+   * ¶

trailing whitespace

Whoops, not needed

@catch I've been thinking the exact same thing regarding Varnish. But I'm not sure per-bin storage will help since you don't necessarily do cache_set($cid, $data, ..., $tags) for the page bin (i.e. Varnish).

So, my thinking was that we add a function called cache_tag_page() that add tags to a X-Cache-Tags HTTP header. It would work like this:

<?php

/**
 * Helper function for setting page cache tags.
 */
function cache_tag_page($tags) {
  foreach ($tags as $tag) {
    drupal_add_http_header('X-Cache-Tags', $tag);
  }
}

/**
 * Implements hook_entity_view().
 */
function mymodule_entity_view($entity, $type) {
  list($id) = entity_extract_ids($type, $entity);
  cache_tag_page(array($type => array($id)));
}

/**
 * Implements hook_entity_update().
 */
function mymodule_entity_update($entity, $type) {
  list($id) = entity_extract_ids($type, $entity);
  cache_invalidate($type => array($id));
}
?>

Then we add a hook to cache_invalidate($tags) and then things could look like this:

<?php

function cache_invalidate($tags) {
  cache_tags()->invalidate($tags);
  // Let other cache backends (i.e. HTTP proxies) to clear their caches based on tags.
  module_invoke_all('cache_invalidate', $tags);
}

/**
 * Implements hook_cache_invalidate().
 */
function varnish_cache_invalidate($tags) {
  foreach ($tags as $tag) {
    _varnish_terminal_run('purge obj.http.X-Cache-Tags ~ ' . $tag);
  }
}
?>

Right, we can't really invoke hooks in the cache API. Our goal is to uncouple dependencies like that in D8, not create more. module_invoke_all() is only really usable after a full bootstrap (unless we do some huge lazy-loading change in D8), and the cache API needs to be available much before that. So we would probably just loop over a list of backend plugins and call their respective invalidate() methods, or else create a lower-level hook system for includes to use amongst themselves.

I rather like the idea of cache_tag_page(), but would be concerned about the tagging getting out of control. Also I would generalize it more to be cache_tag_request(), since in D8 we're trying to move away from page-centric thinking and more towards a REST server.

Looks like this got re-tagged through a crosspost; fixing.

Now that I think of it, tagging the request might be better as a part of the new context initiative. That way the metadata isn't only for the cache layer, but usable as general information about the request.

At this point we're at 'needs work', but not really sure what needs the work.

- 'checksum' is meant to be a signature to track whether something has changed, it happens to be the total invalidation count in this case, but might be something else in a different implementation. I think it's conceptually a checksum then.
- Running parallel backends for varnish headers seems edge-casey, and could be accomplished by one backend proxying to another (or parent::invalidate, etc).
- Haven't heard a good argument against site-wide tag storage and invalidation.
- Tagging the request may have to wait so we can coordinate that with D8 context and re-use metadata.
- Minor code formatting will get fixed with the next re-roll.

Don't forget about ETags.

The invalidation will only work for cache backends that can run cache_tags()->isValid($checksum, $tags) after fetching a cache entry. Varnish will never be able to invalidate cache entries with cache_invalidate as it is. But I agree that a hook is probably not the best solution either.

The best solution would probably be the one catch mentioned -- iterating over cache backends and run ->invalidate($tags) on them. But then we need to register cache backends in a different way i.e. not with magic variables but an array, as catch also mentioned.

i'm much more concerned about keeping the concept in the interface. so, i'll RTBC a patch that still has checksum() in the default sql implementation, but won't support a patch that keeps that in the interface.

I can't see how we can remove checksum() from the interface, without requiring the cache implementation to have special knowledge about the cache tags implementation. isValid() requires a $checksum, so if we remove checksum() we can't have isValid() either. Then we're left with an interface with one method, pointless. Can you illustrate what your concern is?

Sounds like there is a misunderstanding -- the checksum method is much, much faster than clearing tags directly, from benchmarks I've done and real deployment experience. And there is no preference for SQL. In fact, the approach targets NoSQL by keeping things key-value, but it also works great in SQL. I'm already using Cache Tags Mongo version in a large deployment with amazing results, compared to the older cache tags implementation we were running with direct clearing. @dixon_ contributed a wonderful Memcache backend using the checksum interface, and I'm sure it wouldn't be hard to port to APC or Redis either.

A backend that doesn't choose to go with checksums could physically clear data in invalidate(), return 0 in checksum(), and always return TRUE in isValid(). Likely this will be an edge case though, and varnish has special needs so we shouldn't take that as the baseline.

Semi-related note...

Here's a friendly fork of carlos' CacheTags module, which adds support for tagging the page cache. A working Varnish implementation is included. Read more here #1341306: New core patch and support for tagging of the page cache and here #1341322: Support for tagging and invalidating the Varnish cache

Basically what you can do is to register the cache_page bin as an "external bin" in settings.php. Cache invalidation of external bins can't be made in passive fashion (i.e. with a checksum). External bins needs to actively invalidate the cache.

This is not fully relevant to this thread, since we need a better way to solve it in Drupal 8, so please, read more in the issue links above.

Just a re-upload to see if bot will test

Here's a new version that removes the procedural wrappers.

The main motivation behind this is handling reverse proxy backends like varnish or akamai. It's in no way an edge case, so we should handle it without having to work around the current implementation, which seems to be written with mongo in mind.

I'm also not totally convinced that we need checksum/isValid in the interface. I trust it's faster for mysql and mongo, and we should use it there, but I don't know if it's worth the complexity for backends like redis that can probably actively invalidate just as fast. They also won't be used at all for reverse proxy backends.

Just to compare with the numbers above, here's redis with the checksum implementation and with immediate invalidation:

with checksum:

set         :           0.59ms
get         :           0.30ms
expire 1    :           0.27ms
expire 7    :           0.60ms

immediate invalidation (using SUNION and DEL):

set         :           0.38ms
get         :           0.11ms
expire 1    :           36.99ms
expire 7    :           67.54ms

So while deleting the entries at the time of invalidation is more expensive, I don't know that it's enough to warrant the overhead on get/set for this particular implementation.

Here's a new version that changes invalidate() to invalidateTags() and replaces the cache_default_class and cache_class_* variables with a new cache_classes variable so that we can discover the classes easier. Here's how it would be used:

$conf['cache_classes'] = array(
  'cache' => 'DrupalDatabaseCache',
  'page' => 'Redis_Cache_PhpRedis'
);

#138: cache-tags-636454-138.patch queued for re-testing.

Should fix the testing error.

One more time.

function cache_invalidate(Array $tags) { Even if PHP is not really case sensitive, it should be a lowercase "array".

New patch with some of the above comments incorporated.

Fixing code standard violation.

+++ b/core/includes/cache.incundefined
@@ -295,6 +301,70 @@ class DrupalDatabaseCache implements DrupalCacheInterface {
+   * Flatten a tags array into a numeric array suitable for string storage.

FlattenS

+++ b/core/includes/cache.incundefined
@@ -510,3 +590,53 @@ class DrupalDatabaseCache implements DrupalCacheInterface {
+/**
+ * Interface for cache backends that support tags.
+ *

/**
* Defines a common interface cache backends that support tags.
*/

See http://drupal.org/node/1354#classes

+++ b/core/includes/cache.incundefined
@@ -510,3 +590,53 @@ class DrupalDatabaseCache implements DrupalCacheInterface {
+   * Invalidate each tag in the $tags array.

InvalidateS

+++ b/core/includes/cache.incundefined
@@ -510,3 +590,53 @@ class DrupalDatabaseCache implements DrupalCacheInterface {
+ * Invalidate the items associated with given list of tags.

InvalidateS

+++ b/core/includes/cache.incundefined
@@ -510,3 +590,53 @@ class DrupalDatabaseCache implements DrupalCacheInterface {
+ * Get the list of cache backends for this site.

GetS

+++ b/core/modules/system/system.installundefined
@@ -1661,6 +1692,38 @@ function system_update_8002() {
+ * Add the {cache_tags} table.

AddS

+++ b/core/modules/system/system.installundefined
@@ -1661,6 +1692,38 @@ function system_update_8002() {
+ * Modify existing cache tables, adding support for cache tags.

ModifieS

Srry for posting the same "doxygen error" that many times but using modify in stead of modifies is just a habbit we have to forget (except when creating hook documentation).

-4 days to next Drupal core point release.

Tagging novice to reroll with aspilicious' suggested cleanups.

:) I'm going to mark this neesds review again for the code. Btw if someone is going to reroll this don't pay attention to the missing docblocks if they are to hard.

New patch with doc cleanups.

Do the DrupalCacheTagsInterface means a backend that supports tags? If yes, it's possible to make it more explicit 1) by naming, and 2) by interface inheritance:

Could be:

interface DrupalCacheTagsInterface extends DrupalCacheInterface {}

Then the naming sucks (I'm thinking of the cache as PHP 5.3/PSR-0 naming), something like:

namespace Drupal\Cache;

interface TaggableCacheBackendInterface extends CacheBackendInterface {}

But this is a minor note, the patch seems good.

This issue is currently for D8. Is it likely there will be a backport to D7?

I can see this having huge performance potential, especially when used with Varnish :) I also happen to have rather immediate and specific needs for http tagging & and cache clearing with Varnish, which means that I'll just create my own module, for now. If cachetags didn't need a core patch I would use it partially.

Anyway, my module will have a proper UI, context & rules support, and some trick for esi + user specific caching, which should make it very flexible and reuseable. Even so, I'm definitely also awaiting this core feature eagerly!

+++ b/.htaccessundefined
@@ -95,7 +95,7 @@ DirectoryIndex index.php index.html index.htm
-  # RewriteBase /
+  RewriteBase /

Needed?

5 days to next Drupal core point release.

Obviously not.

+++ b/core/includes/cache.inc
@@ -32,16 +32,42 @@ function cache($bin = 'cache') {
+    $cache_backends = variable_get('cache_classes', array('cache' => 'Drupal\Core\Cache\DatabaseBackend'));

Is there a reason this doesn't use cache_get_backends()? It has a different default, but it seems that cache_get_backends() still has the pre-PSR-0 one.

+++ b/core/includes/cache.inc
@@ -32,16 +32,42 @@ function cache($bin = 'cache') {
+    if (in_array('DrupalCacheTagsInterface', class_implements($class))) {

This doesn't exist anymore, the if can simply be removed.

+++ b/core/includes/install.core.inc
@@ -297,7 +297,7 @@ function install_begin_request(&$install_state) {
+  $conf['cache_classes'] = array('cache' => 'Drupal\Core\Cache\InstallBackend');

Same question here: Couldn't this use cache_get_backends()? It's the installer so there is probably little benefit to having this configurable, but still.

+++ b/core/lib/Drupal/Core/Cache/DatabaseBackend.php
@@ -254,6 +269,89 @@ class DatabaseBackend implements CacheBackendInterface {
+  public function invalidateTags($tags) {

This could use a function header. Should be the simple
"Implements CacheInterface::invalidateTags()." though.

+++ b/core/includes/cache.incundefined
@@ -32,16 +32,42 @@ function cache($bin = 'cache') {
+ * @param $tags

Can we add type hinting here? I believe it should be an array.

+++ b/core/includes/cache.incundefined
@@ -32,16 +32,42 @@ function cache($bin = 'cache') {
+/**
+ * Returns a list of cache backends for this site.
+ */

There is a missing @return directive here (ideally with type hinting).

+   * @param integer @checksum
+   *   The initial checksum to compare against.
+   * @param integer @tags
+   *   An array of tags to calculate a checksum for.
+   * @return boolean
+   *   TRUE if the checksums match, FALSE otherwise.

I think the wrong type hint is given here for tag param. Also I think it should be $checksum and $tags. Finally, there needs to be a blank line before the @return directive.

+   * @param $tags
+   *   Associative array of tags.
+   * @return integer
+   *   Sum of all invalidations.

Another blank needed before @return directive and type hint needed for $tags variable. Several other $tags vaiables also could use type hinting too.

Yes, that definitely makes sense. (I had somehow overlooked that we are a setting a different class in install.inc)
I couldn't find anything more to complain about from visual inspection, but have yet to play with it, so not setting RTBC.

The last iteration looks very good to me. But before we RTBC this, we probably should have some tests for this, no?

I'll try to give the last patch a ride very soon, and write some tests for it.

Also noticed that there are no tests yet.
I'll have a go at that, maybe that's a good way to try this out.

Oops, that was a crosspost. (Great minds think alike :))
Alright then @_dixon: first come, first serve. Go for it :)

Wow, another one...

incredible...

+++ b/core/includes/cache.incundefined
@@ -32,16 +32,40 @@ function cache($bin = 'cache') {
+/**
+ * Returns a list of cache backends for this site.
+ */

Doing a final review of this patch, I noticed that this docblock is missing a @return directive.

+   * @param $tags
+   *   Associative array of tags, in the same format that is passed to
+   *   CacheBackendInterface::set().

This is also missing type hinting.

Thanks @catch. The documentation fixes look good.

The one thing I am unsure about is the current practice for the translation of assertion messages. I think the best practice now is to remove all t() calls and use format_string() where necessary. However, I may well be wrong.

Nothing to do with the patch itself, but aren't you worried that the cache tags feature might no be implemented easily with some backends, and that we may end up with bailing out some of them because of it? Aside of that, it's an excellent feature, but I'm still afraid of loosing some great backends (memcache, redis, every one of them that are key/value stores in fact).

I hope so, but I still see some weaknesses, even if the checksum method there are chances of cache stall and memory bloat if the checksum validity check is made on access, some cache entries may not be accessed for a while (note that this is also true for actual cache usage anyway so it doesn't really matter).

Thanks for the update, it seem perfectly doable indeed.

Changed self::tagCache to self::$tagCache, the first one is unfortunately not PHP :)

Let's see if this is better now.

Fixed a remaining $self->tag_cache and initialized self::$tagCache to an empty array to prevent array_merge() errors.

Grr... and now without syntax errors :)

This is looking good to me. There are a few improvements that could be made I think, but could wait for separate issues. I don't think we're going to get much more insight on this until we start using it in core.

While attempting to write a change notification for this, I was reading the docs for CacheBackendInterface and I noticed it still had references to cache_default_class and cache_class_*.

Maybe this should be a follow-up issue, but I'm uncertain of any change notification until the in-code docs match.

For the record, I'm not sure my change is actually correct, especially since it makes 1 line into 3, and its all non-CMI variable_get/set stuff.

Per xjm's suggestion, retitling for now but leaving priority.

That looks correct, but it's kinda silly that we give the example using variable_get/variable_set. No one does that. You use use $conf in settings.php.

As far as I know, CMI has not tried to deal with the settings.php settings, which this would be, so it seems like a good idea to just update the docs using $conf.

The only usage of this in core is $conf['cache_classes'] = array('cache' => 'Drupal\Core\Cache\InstallBackend'); in includes/install.core.inc, so that fits.

However, doesn't this also need a hook_update_N for any existing cache_class_* variables and cache_default_class?

If everyone sets their cache classes in $conf, an update hook wouldn't really do anything, so I'm leaning towards no.

Of course we set them there, it'd impossible not to. The very reason you set an alternative cache backend because you want to avoid a database, right :) ?

Wrote up http://drupal.org/node/1534630 and http://drupal.org/node/1534648, someone who actually knows what is going on, please review them :)

Wasn't aware of the changes related to these variables, confirmed that the new documentation is correct by looking at the code.

Note: RTBC is for the documentation cleanup patch in #198 (wow), so this needs to set back to needs review once that is commited for the change notices unless someone confirms that they are ok in the meantime.

About the change notices:

> "Which would now be"

Maybe something along the lines of "Instead of a separate variable, the cache backend of the default bin "cache" is now used as the fallback, which can be set like this". (Edit: If we'd rename the default cache bin e.g to "default" instead of "cache", this would make even more sense ;))

The wording can probably be improved/simplified, but I think it deserves a sentence that actually explains why it's like that. Looks confusing otherwise IMHO.

Similar for the actual cache tag change notice, I'm not sure if that's common, but cache tags aren't just a replacement for cache_clear_all(), they're a completely new API. So a short introduction along the lines of "Cache Tags can be used to flag certain cache entries which allows to invalidate them based on the used tags later on. For example, invalidate all cache entries that have been tagged with a certain node id if that node is updated."

Also, again not sure about the procedure, but cache_clear_all() already already been dropped for all uses except the no-arguments case that clears page and block cache. So maybe you should use the new $cache->delete* methods? Also, maybe a better example would actually be how you can use it to clear all caches tagged with a node id instead of using a prefix/wildcard.

Example code for that:

// Previously, use some kind of naming scheme to identify caches for a node.
$cache = cache($bin);
$nid = 1;
$cache->set('cache_nid_' . $nid . '_id_one', $some_value);
$cache->set('cache_nid_' . $nid . '_id_two', $some_value);

$cache->deletePrefix('cache_id_' . $nid);

// New. Does not rely on magical cid's, you can use whatever makes sense and keep them short.
$cache = cache($bin);
$nid = 1;
$cache->set('cache_id_one', $some_value, CACHE_PERMANENT, array('node' => array($nid)));
$cache->set('cache_id_two', $some_value, CACHE_PERMANENT, array('node' => array($nid)));

cache_invalidate(array('node' => array($nid)));

I feel something like this explains the advantage of the tags system better.

Berdir, I was just posting my limited understanding of it after an hour of staring at the patch, please feel free to edit the change notice!

Per #203, the change notices need work. They're linked in the bottom of the issue summary.

Updated the change notices. Should be closer to a real world use case now. Back to needs review.

Restoring title

The only thing not consistent is the "cache_tags" table name which has the same pattern as a regular cache bin table. This is confusing but not critical.

Renaming to cachetags will fix but, I know, it's ugly :)

It's not really ugly, both "cache" and "tags" word and readable and short enough for us to name the table "cachetags" without making it less readable. Or we could just change the cache system to be configured to use only explicitely registered bins by modules, and never allow to register the "tags" bin, this should be the most secure and elegant way to do this, plus it would allow us to get rid of the infamous hook_flush_caches() hook implementation for most modules in favor of something like hook_bin_info().

+1 for registering bins OR, more simple, redesign the cache table pattern. Something like: cache_bin_*.

For consistency I would rename also the cache table.

Examples:

• cache_path => cache_bin_path
• cache_filter => cache_bin_filter
• cache => cache_bin_drupal <<< switch the main cache table to a consistent pattern

Yeah, good idea. For the record, there is actually an issue opened for this, it didn't moved since a long time ago, let me try to find it.

#1167144: Make cache backends responsible for their own storage Found it!

So, what is your option? Registering bins or change cache tables pattern?

See #216, there is an issue for registering cache bins. I don't think we need to discuss this here. I recommend to close this issue again.

Closing again... Lets meet in the other issue :)

Didn't want to reopen this... When reading through the DatabaseBackend implementation for invalidateTags() I noticed that there may be a major performance issue. I may be wrong though: #1800768: DatabaseBackend tag invalidation performance

Follow-up: #918538: Decouple cache tags from cache bins

Adding followup issue

I think we should consider backporting this so it can be used in Drupal 7 core (and so it can be more easily used by Drupal 7 contrib modules).

#2598340: variable_set() never clears the page cache is an example of a core issue that could use this.

The Drupal 7 module (https://www.drupal.org/project/cachetags) doesn't have much usage, but I used it for a project once a couple years ago and found it to be absolutely great. It basically looks like something that could be dropped into core almost as is. The only modification it makes to existing core functions is an extra (optional) parameter to cache_set().

I used the D7 cachetags project a few years ago, it worked like a charm. Has most of the patches we need to backport to D7.

I'm sorry, but I can't help but answer this bluntly: Are you kidding me?

Do you have any idea how much work it was to update all the things to have them support cache tags?

It literally took more than a year of me working quasi-full-time on "cache tags for all the things" in Drupal 8. And that was with extensive help of many others. And it required API changes. Lots of them. But I think by "cache tag support" you really mean just "API support" and not "make everything in Drupal 7 support it". If so, please adjust the title, because I just suffered a heart attack in your place :P

Or, much better, open a separate issue instead of reopening an issue that was closed for almost 3 years, and that is already beyond 200 comments.

That being said, even using this "just" for one-tag-per-variable (which is what you suggest at #2598340: variable_set() never clears the page cache) is fraught with problems (see #2598340-2: variable_set() never clears the page cache).

Actually having the API does not mean that everything must use it, but having it in core would be a great help for site developers (not builders) to put in place smart tags based caching policies on very specific business oriented sites. Put that in place and possibly a hook_cache_set_alter($item, $bin) and the developer would have total control over his site cache policy.

I'm saying that because I do work on long-run maintenance on multiple complex D7 sites which are never going to migrate on D8, but will continue to be maintain for a long time.

I don't think this is a good idea at all, for several reasons in fact:

1. D7's architecture hasn't been built around the conceptual idea of cache tags.
2. Backporting the full implementation is unrealistic, extremely much work and will therefore - realistically speaking - never happen. It would boil down to rewriting D7's entire render subsystem.
3. Only backporting the table and invalidateTags method creates a false expectation and more importantly confusion to site builders and developers.

Although D7 ships a DrupalCacheInterface, nothing will stop anyone from providing a module that swaps or wraps the standard backend introducing new methods such as invalidateTags.

But no, I don't think this should be backported - to core - in any form.