Problem/Motivation

I wanted to update https://www.drupal.org/node/1884814, but couldn't because it's not at all clear why #2231595: Add a cache backend that checks an inconsistent cache, then falls back to a consistent cache backend didn't use BackendChain (it's not documented in the code, nor explained in the issue, despite having been asked twice).

Or perhaps it makes more sense to remove BackendChain?

Currently, both seem to solve the same problem. That's bad. Let's fix that.

Proposed resolution

Remaining tasks

User interface changes

None.

API changes

TBD

Data model changes

None.

CommentFileSizeAuthor
#4 2541432-3.patch2.07 KBwim leers
#2 2541432-2.patch2.07 KBwim leers

Comments

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Follow-up for #2231595: Either remove BackendChain or document why ChainedFastBackend cannot use BackendChain » Follow-up for #2231595: Document why ChainedFastBackend cannot use BackendChain

catch confirmed in IRC the reason is what I wrote at #2231595-8: Add a cache backend that checks an inconsistent cache, then falls back to a consistent cache backend.4:

+++ b/core/lib/Drupal/Core/Cache/CacheFactory.php
--- /dev/null
+++ b/core/lib/Drupal/Core/Cache/ChainedConsistentAndInconsistentBackend.php

Why doesn't this subclass \Drupal\Core\Cache\BackendChain? I think it's because BackendChain "stupidly" chains cache back-ends, not allowing for inconsistent cache back-ends, only allowing for consistent-but-automatically-discarding-cache-entries cache back-ends (i.e. LRU/MRU/…)?

If so, it'd be great if this were documented on ChainedConsistentAndInconsistentBackend.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
Issue tags: +Documentation
StatusFileSize
new 2541432-2.patch2.07 KB
dawehner’s picture

dawehner
Primary language German
commented

Can we rename the classes to be more helpful?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
StatusFileSize
new 2541432-3.patch2.07 KB

s/back-end/backend/

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

@dawehner Yes, but that's more bikesheddable + a lot more work invasive, and we need these docs anyway, so I'd prefer renaming in another issue.

fabianx’s picture

fabianx commented
Status: Needs review » Reviewed & tested by the community

RTBC, looks good to me.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Priority: Major » Normal
dawehner’s picture

dawehner
Primary language German
commented

So what about creating an issue to actually improve the naming? A proper naming is much more worth than some docs.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#2541864: Rename ChainedFastBackend to make it clear that it's about chaining a consistent and inconsistent backend, unlike BackendChain

As you requested: #2541864: Rename ChainedFastBackend to make it clear that it's about chaining a consistent and inconsistent backend, unlike BackendChain.

Anonymous’s picture

Anonymous (not verified) commented

+1 to the doc additions.

re. #8 - i'd be happy to figure out a better name. i seem to recall it was not fun to name the first time around.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Docs are unfrozen in beta. Committed 74c4534 and pushed to 8.0.x. Thanks!

  • alexpott committed 74c4534 on 8.0.x 
    Issue #2541432 by Wim Leers: Follow-up for #2231595: Document why...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.