Expose cache tags to reverse proxies: X-Drupal-Cache-Tags header (by just slightly changing an existing header)

Could be a stupid question, but could we hash it to save information leaking, might end up smaller too.

1. +++ b/core/lib/Drupal/Core/EventSubscriber/HtmlViewSubscriber.php
   @@ -93,6 +93,36 @@ public function onHtmlPage(GetResponseForControllerResultEvent $event) {
   +   * @see \Drupal\Core\Cache\DatabaseBackend::flattenTags()
   

   Is tere any reason we don't simply make that method a public static one?

2. +++ b/core/modules/system/lib/Drupal/system/Tests/Cache/PageCacheTagsIntegrationTest.php
   @@ -66,24 +66,24 @@ function testPageCacheTags() {
        $this->verifyPageCacheTags('node/' . $node_1->id(), array(
   ...
        $this->verifyPageCacheTags('node/' . $node_2->id(), array(
   

   Can't we just remove the identical and replace it with equal now? Is there any reason that the order of cache tags would matter?

At this point, we might as well use this as the ETag, this gives us client-side validation while still allowing more intelligent agents to take additional decisions.

With the current implementation, the Etag would only change if the actual list of tags themselves change (i.e. a page no longer has node 2 on it, but node 3 instead) - since we're (rightly I think) not including the tag invalidation counts in the header.

If we did include the invalidation counts in the headers, using the ETag gets more interesting in terms of client side invalidation though.

For the use case of clearing varnish as originally discussed, the idea has been to directly invalidate via regexp on the headers. In that case you don't need to store tag invalidations, because the cache backend is just purging when a cache tag is updated - nothing local about cache tags needs to be kept at all (for the page cache at least).

I thought this was a list of tag fingerprints. We have this readily available, so we should just use it.

Agreeing with @Wim on the scope here. External caches can just store the header and outgoing calls (e.g. from purge) can instruct Varnish (or a CDN) to wipe regex based on the cache tag string.

Getting this in would be king!

I do understand where this wants to go, and it would be trivial to implement a Varnish ban rule based on the cache tag. What I'm discussing is that (1) this is at odd with standard HTTP caching mechanisms, so non-Drupal-specific caching agents are not going to be able to leverage it, (2) this introduces an inconsistency between client-side invalidation (which relies on ETag only) and intermediate agent invalidation.

Wim Leers asked me to review this patch. The thing that stands out as very odd to me is that there is no encapsulation for this functionality. Logic related to it appears oddly as several places in the codebase.

I thought this was a list of tag fingerprints. We have this readily available, so we should just use it.

This isn't readily available. To make it available we'd have to store either an increment or timestamp of the last time any particular cache tag is invalidated. Just the list of tags isn't enough to know if an item is valid or not.

The database backend does store that information, but it's an implementation detail and not imposed by the API. Right now it would be valid to have a 'varnish' backend for the page cache that does nothing when items are stored (i.e. similar to pressflow/7.x external cache), and invalidates via purge when items are cleared. Even using the 'internal' page cache, something like redis could do direct invalidation of cache items rather than tracking information about tag invalidations and comparing on read.

@catch: That sounds like a good reason to revise the API. I think #918538: Decouple cache tags from cache bins is moving in this direction, but I haven't looked very closely.

@Damien that tries to clear up the relationship between bin storage and tags (and I liked your last suggestion on there), however there are two different and equally valid approaches to tag invalidation, direct clearing vs. comparison on get.

ETags relies on comparison on GET (in fact that's exactly what what an ETag is for), but I don't think we want to enforce one or the other approach on the tag storage.

One possibility would be to have two different interfaces so that implementations can indicate which they do, and then use it for the ETag or not depending on the answer.

Hm, the flattenTags implementation so far was an implementation detail, a different backend implementation might want to use a different one. But I guess they can still decide to do so.

Also this adds a cross-dependency between the implementations and the Cache class. It's a lot better than adding it to the interface and enforcing the format but still feels a bit odd. What about putting that in a trait?

+++ b/core/includes/common.inc
@@ -3241,7 +3241,7 @@ function drupal_page_set_cache(Response $response, Request $request) {
       ),
-      'tags' => array('content' => TRUE) + drupal_cache_tags_page_get($response),
+      'tags' => explode(' ', $response->headers->get('X-Drupal-Cache-Tags')),
       'expire' => Cache::PERMANENT,

I don't think that's correct?

This will then look like array('node:1', 'node:2') but what we want is array(array('node' => array(1, 2)?

It will probably work right now as the flatten in the database backend does essentially the same, but as you said yourself, we shouldn't rely on that.

+++ b/core/lib/Drupal/Core/EventSubscriber/HtmlViewSubscriber.php
@@ -77,7 +77,7 @@ public function onHtmlPage(GetResponseForControllerResultEvent $event) {
+        $response->headers->set('X-Drupal-Cache-Tags', $this->convertCacheTagsToHeader($tags));

Should this be static::convertTagsToHeader() now?

+++ b/core/lib/Drupal/Core/EventSubscriber/HtmlViewSubscriber.php
@@ -129,4 +129,36 @@ protected function flattenCacheTags(array $tags) {
 
+  /**
+   * 'Flattens' a tags array into an array of strings.
+   *

No it doesn't :)

This overly looks good.

+++ b/core/includes/common.inc
@@ -3241,7 +3241,7 @@ function drupal_page_set_cache(Response $response, Request $request) {
+      'tags' => Drupal\Core\EventSubscriber\HtmlViewSubscriber::convertHeaderToCacheTags($response->headers->get('X-Drupal-Cache-Tags')),

I wonder why we can't just use an use statement instead. This would be a bit nicer to read here.

Not from my side.

#31 I know it's not necessary, but $this->convertCacheTagsToHeader($tags)); made me go to check if the method was static or not (since the reverse is called statically elsewhere), whereas I'd otherwise have saved the check. I can't really find anything else to complain about though :)

You could argue that helper methods should not live on subscribers, but I don't care.

Yes, I prefer this as well.

Looks good to me.

Header HEAD:
cache_tags:a:9:{s:9:"node_view";b:1;s:4:"node";a:1:{i:1;s:1:"1";}s:4:"user";a:1:{i:1;s:1:"1";}s:4:"file";a:1:{i:1;s:1:"1";}s:9:"file_view";b:1;s:13:"taxonomy_term";a:5:{i:1;s:1:"1";i:2;s:1:"2";i:3;s:1:"3";i:4;s:1:"4";i:5;s:1:"5";}s:18:"taxonomy_term_view";b:1;s:13:"filter_format";s:10:"basic_html";s:4:"menu";a:5:{s:5:"tools";s:5:"tools";s:6:"footer";s:6:"footer";s:4:"main";s:4:"main";s:7:"account";s:7:"account";s:5:"admin";s:5:"admin";}}

Header with patch:
X-Drupal-Cache-Tags:node_view:1 node:1 user:1 file:1 file_view:1 taxonomy_term:1 taxonomy_term:2 taxonomy_term:3 taxonomy_term:4 taxonomy_term:5 taxonomy_term_view:1 filter_format:basic_html menu:tools menu:footer menu:main menu:account menu:admin content:1

Much better :)

Very exciting that this just went RTBC. I just validated that this will work 100% when using Varnish as external cache for instance, I will soon build a proof of concept setup based on my work on the purge module for D8.

As can be seen here: https://www.varnish-software.com/static/book/Cache_invalidation.html#ban...

It becomes very trivial to write a VCL that accepts HTTP BAN requests based on X-Drupal-Cache-Tags, e.g.: ban req.http.host ~ "example.com" && obj.http.X-Drupal-Cache-Tags ~ /<REGEX based on req.http.X-Ban-Tag goes here>/

Varnish'es BAN lurker will then simply iterate the heap with cached items, match each header against the added BAN rule and remove the items as requested. The next time the item is requested Varnish will go back to the backend (Drupal) and restore a fresher copy of the page in cache. Thus changing any cache tag in Drupal can result in massive (and efficient) BAN's on the server side and thus very effective external cache invalidation.

Big big big wins here, very nice.

Created #2225765: Consider using cache tags (and cache contexts) for the ETag as follow-up.

Committed/pushed to 8.x, thanks!