Support min-age in render caching

Well, this blocks #2381277: Make Views use render caching and remove Views' own "output caching" so :(

On this

Discussed with @dawehner.

#2381277: Make Views use render caching and remove Views' own "output caching" isn't blocked by this, so changing this to major.

$this->storage['#cache']['tags'] already contains the relevant cache tags like the node_list cache tag. This means it is already invalidated whenever a node changes, it's not min-age* caching in HEAD either with views output caching.

Which means that switching to normal render caching is no regression compared to HEAD.

* It was never min-age caching. It's just max-age without invalidation. The important difference is that min-age could still be cached without or much higher expiration, but it would ignore (tag-based) invalidation for the configured timeframe.

The discussed workaround to implement it by dropping a part of the cache tags and use cache expiration would have the same flaw. It would force a cache rebuild after the min-age expired.

I'd love to have a real min-age support, but that's kind of a feature and possibly needs to be postponed to 8.1.x. I think for real min-age caching, we also don't need special features like being able to still invalidate some cache tags. You would set that to a very low value to prevent constant invalidations if you have a lot of new/changed content. I'd expect typical values to be between 1-5 minutes. Because if you have a lot of content, then you also want to have it published as soon as possible. And then you can also wait for that time until your views configuration change is visible :)

Fair enough, feature-parity with D7 was lost when making the Time-based plugin use cache tags, I guess, so no reason to block #2381277: Make Views use render caching and remove Views' own "output caching" on this.

Attaching the solution discussed with @dawehner before this was demoted.

Better version

It is borderline featur-y, I agree with berdir on that, so we should have a beta eval before starting work.

The proposed solution to have:

#cache_min_age = [ 'age' => 300, 'allowed_cache_tags' => [...], ]

sounds good to me.

It also means that the renderer will do the filtering _and_ only filter it for setting on the actual cache item, so bubbling is not affected.

---

Train of thought

Here is why this is a good and sane thing that min-age is not bubbled:

a) Making a view min-age does not mean, you want to have that for the whole page.
b) It gets soon really really complicated

Here is a hypothetical example, where it (bubbling) fails - if you were to simply remove the tags instead of having a proper API only for that item:

- Custom coded view in a block
- View is set to min-age: 5 min, max-age: 1 hour -- views checks the time itself
- Contexts are completely removed and hence not bubbled up

- If a cache item is retrieved at the view layer, cache hit, views takes a look at the age and returns it if it is below that age

=>

However the block is cached, too and this is where it gets interesting.

The block is cached for 1 hour, but there is no invalidation information and no min-age information => items are returned after 1 hour just - regardless if invalidated or not ...

So I think any more advanced usage one should experiment in contrib/ first, then consider for 8.1 or 8.2.

====

Train of thought - 2

Even the proposed solution does not work as well.

What needs to happen is that views / render cache uses the cache_tags invalidator service itself to store a second checksum in e.g. #min_age_checksum for a given set of tags.

Then invalidation of all tags works (return invalid item), but invalid items can be distinguished from "really invalid items".

I still think that is a bad idea to introduce special cache tags here, IMHO we should keep this simple and treat all cache tags equal. As you said yourself, this can get quite complicated and it won't be invalidated properly after that time without forcing a max age.

IMHO, if you configure views to minimally cache an item for at least 15 minutes, then it's OK if it also survives changing the view. min-age shouldn't be set to hours IMHO, I'd imagine you'd set it only up to 15minutes or so in common cases, since you don't want to wait hours until the items show up properly?

IMHO, we basically just need something like the attached patch and if do it like this, then this is really easy.

Didn't look into bubbling yet, but AFAIK, we don't bubble up the whole #cache array, just the tags, context and max-age, so this should work fine?

Didn't look into bubbling yet, but AFAIK, we don't bubble up the whole #cache array, just the tags, context and max-age, so this should work fine?

Correct. And very clever :)

This patch looks wonderfully simple, which makes it all the more more convincing. I'd love to have others chime in, but this definitely looks very, very interesting :)

If we don't do the tags dance and no bubbling, etc., then I am 100% +1 to just get the MVP / MLP (I love it! :) in ...

In my book this is RTBC, also only internal changes.

Just some food for thought (but its up to the site developer to determine that, so not for core to solve):

- a comment on a node gets min-age = 15 min, max-age is not set and hence implicitly Cache::PERMANENT
- the node has explicit max-age = Cache::PERMANENT

The comment gets invalidated => the node gets invalidated, too.

One minute afterwards:

Render Chain:

- node => cache miss
- comment => cache miss, but min-age = 15 min is set => using old (invalidated) comment
- comment gets returned
- Node gets stored permanently with the wrong comment

=> Rule of thumb (for those implementing min-age):

min-age <= max-age on that same element

We could of course enforce it here, too as I can't see how it could ever be wanted to set max-age to something else.

So I would propose:

If min-age is set and max-age is not explicitly set, to set max-age to either min-age OR to the 'remaining validity' period.

The 'remaining validity period' is obviously:

$min_age - (REQUEST_TIME - $cache->created)

which is when this item needs to be expired again, which would be easy to add.

Thoughts?

Definitely, we need to update docs.

We also need to answer the following:

How exactly are you actually going to use this to use min-age on a block or view? It doesn't bubble up, and a block plugin currently isn't able to influence the #cache array on the block render array directly. Do we need to get getMinAge() methods? Possibly as an optional interface?

A view would be able to take care of this if it was available, and applied to CacheableMetadata. It could then trivially get this from the cache plugin, as it does currently for max age (and friends). Blocks, NFI :)

OMG its a return of the dreaded #730060: Replace CACHE_TEMPORARY, cache_clear_all() and minimum cache lifetime with cache tags support. Just kidding - this makes sense.

#14 I want an alterBuild() function for BlockBase since ages, which would easily solve that ...

Or use hook_block_build_alter()

Min-Age should never be bubbled but just applied at a render item.

This should indeed be supported directly inside the renderer.

I would also suggest to bring back a kind of #post_render_cache callback, which is run when the item is loaded from the cache (so it could still be a cache miss e.g.).

And I am right that when using min-age, max-age should at least be set to min(max-age, min-age).

Tried to get a proof of concept working using a views block, but it's tricky. While the simplified example works fine, reality is that #cache is the result of merging through CacheableMetadata and that drops min-age. Not quite sure yet how to do it, but we still might need to make that class aware of min-age, so we can internally store it and set it again, without merging it from other places.

This now works, but only by actually looking at min-age of the incoming $elements, also had to set it to make cache redirect work.

No automated test yet, you can test by putting the recent content views block into the sidebar and changing it to do time based caching.

Note that, since min-age does not and must not bubble up, the whole page will still be invalidated when saving a node. We can only keep using the views block itself.

The existing tests will likely fail as I didn't update them. This isn't really the way forward, just wanted to try this out.

This isn't really the way forward, just wanted to try this out.

This is why I didn't review it, but perhaps I should?

Removing outdated tag.

@dawehner: I don't really follow that. lazy_builder makes sense for things that can *not* be cached or have more variations. This is the opposite, something that likely can be cached longer than other parts. So either it's just part of the whole page, or it will still be returned from cache if the page cache hit is a MISS. Just like any other entity/block, just possibly for a longer time.

> This is why I didn't review it, but perhaps I should?

You're welcome to, because I don't really have an better ideas than what I did there ;)

I talked with @dawehner about his idea and understand it now.

As he said, the expectation is that min-age is used when the block/element is slow to build *and, most importantly* it contains a frequently invalidated tag (e.g. node_list).

So the generic idea is that we trade the overhead of an additional fast cache get on dynamic page cache hits to avoid having that frequently invalidated cache tag "pollute" all our dynamic page cache entries. Because right now, with this patch, if you have a "latest news" block in your sidebar on all pages, then all your dynamic page cache entries still have the node_list cache tag. And if you save a node, then they are all invalidated. And if you request such a page again, we would still return the latest news block from cache but would have to rebuild possibly large parts of the remaining page (like the main content, if it is not a rendered node).

If it's a lazy-builder placeholder in the dynamic page cache (and nothing else adds the node_list cache tag), then we still have a cache hit on that, still return the views block from cache (for another 5min or so) and are done.

That only works for DPC, we still have the node_list cache tag on the internal page cache, but then rebuilding that would work like this: X-Drupal-Cache: MISS -> X-Dynamic-Page-Cache: HIT -> views block HIT => store the result in page cache again.

We can already achieve that by explicitly setting #create_placeholder => TRUE e.g. on all views blocks or we can indeed do it for all \Drupal\Core\Render\PlaceholderGenerator::shouldAutomaticallyPlaceholder and do it for all placeholder-able parts with a min-age entry. Or we would add node_list to the main #cache tags and make that an auto-placeholder cache tag.

#2935804: Add cache context exclusion list to RenderCache::set() (skip render caching at least *some* lazy-builder elements) is kind of the opposite idea and we need to make sure those two don't conflict.

A better name for min-age btw. is stale-while-revalidate, which is way more clearer than min-age (what does that even mean).

> what does that even mean

Well, it does *not* mean stale-while-revalidate for starters :)

This is not an implementation for return one stale response and recalculate in background. This is the opposite of max-age, this keeps the cache *at least* for the configured amount of time, guaranteed *. If you set it to 10min, then it will return the same cached response for 10 minutes, even if the node_list cache tag is invalidated after after one minute.

stale-while-revalidate could be useful too, but so is this. stale-while-validate ensures faster responses but in the end results in the same amount of server load/rebuilds as not having it.

* only as guaranteed as the cache backend is of course, but Drupal will do nothing to actively replace it or to stop using it.

Indeed, but a big problem is how those 10 min go down into lower layers (e.g. DPC).

e.g. consider a view with min-age within an entity within a block within a page.

The cache hierarchy is:

page > block > entity > view > ...

If the view itself can be cached for 10 more min that is fine, but the entity and block and page must not be cached for 10 more min, which is why I am saying this is closer to stale-while-revalidate semantics overall (without the revalidate part for obvious reasons).

e.g. while a lower item is stale the other cache page parts must not be re-cached or at least max-age needs to be set to remaining TTL on the min-age, which severely limits the useful-ness of that whole thing.

And current page_cache would even re-cache the stale item permanently as it does not take max-age into account ...

For obvious reasons a min-age could be defined globally, but then the problem is that you still have to synchronize the different cache entries and re-caching stale items is very tricky.

Yes that's true, the other caches that contain this part would still be invalidated. But that's exactly why @dawehner suggested to auto-placeholder those elements, because of the assumption that those elements likely are going to be invalidated more frequently, then the parents can actually be cached even longer.

See #27-30. But maybe the auto-placeholdering should more explicitly be done through the node_list cache tag as you suggested in the other issue which would also solve that use case.

e.g. while a lower item is stale the other cache page parts must not be re-cached or at least max-age needs to be set to remaining TTL on the min-age, which severely limits the useful-ness of that whole thing.

And current page_cache would even re-cache the stale item permanently as it does not take max-age into account ...

The first part is indeed interesting, if the element would *not* be auto-placeholdered, then yes, the parent might be cached without limit and unless another invalidation happens in that time, it wouldn't be invalidated. But autoplaceholdering would solve that too?

And yes, page_cache could also be tricky, also since it doesn't support any form of auto-placeholdering..

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.