Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Discovered in #2429617: Make D8 2x as fast: Dynamic Page Cache: context-dependent page caching (for *all* users!). The redirecting cache item would be created in the default cache bin (render), the actual render cache item would be stored in the specified cache bin. This the of course led to the cache redirect cache item not being found, and thus resulting in a cache miss on every request.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | cache_redirect_bin-2493047-7-test-only.patch | 5.64 KB | wim leers |
| #7 | cache_redirect_bin-2493047-7.patch | 6.67 KB | wim leers |
Comments
Comment #1
wim leersComment #2
fabianx commentedRTBC, this tests via unit tests that the 'bin' is set to 'render' in the redirect cache item, hence enough test coverage exists.
Comment #3
fabianx commentedNo, it has enough tests ... - as said already.
Comment #4
xjmThanks! More cache fixing goodness.
I'm not clear on why this doesn't need tests though. @Fabianx, how is there enough test coverage if there was a cache miss on every request under... the circumstance that is not quite clear to me... and nothing was failing in HEAD? That sounds like we definitely need more test coverage; the unit tests definitely aren't enough with a bug of that magnitude? Refer to https://www.drupal.org/core-gates#testing
So let's add test coverage, or at least a really thorough justification of how it is we don't need it...
Comment #5
fabianx commentedRelated #2463581: #cache_redirect cache items should have an 'expire' timestamp that matches the merged max-age - maybe we want to incorporate that one in here?
Comment #6
wim leersI'm not sure about #2463581: #cache_redirect cache items should have an 'expire' timestamp that matches the merged max-age. I'm posting a comment there that explains what I still find unclear. So let's keep that as a separate issue.
EDIT: see #2463581-3: #cache_redirect cache items should have an 'expire' timestamp that matches the merged max-age.
Comment #7
wim leersComment #8
fabianx commentedRTBC, looks great, thought test-bot will send this to NW as test-only patch was attached last ...
Comment #10
wim leersRTBC per #8.
Comment #11
alexpottThis issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed d2ebb7a and pushed to 8.0.x. Thanks!
Comment #13
wim leersComment #14
wim leers