Problem/Motivation

For the https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Cache%21C... it is automatically detected if apc_fetch is available and then the apcu backend used.

This leads to stale caches when the cache is:

a) cleared via cli
b) apc.enable_cli = 0 (hence apc_fetch not available as function)

because then only the consistent backend is cleared, but the timestamp of the APC counter is not updated.

The caches will remain stale until any write on the cache fast chained backend.

Proposed resolution

We need to do a little more extensive check and use a Null or Memory Backend as fast chained cache when:

- ini_get(apc.enabled) == 1 AND apc_fetch is not available.

Therefore proposed code would be:

  // Auto-Detect a possible fast backend.
  if (!isset($fast_service_name)) {
    if (function_exists('apc_fetch')) {
       $fast_service_name = 'cache.backend.apcu';
    }
    // If APC is enabled, but not usable here, this means the non-cli
    // request uses the apcu backend with a high probability, therefore ensure
    // that timestamps are updated.
    else if (ini_get('apc.enabled') == 1) {
       $fast_service_name = 'cache.backend.memory';
    }
  }

That should solve this nicely.

This kind of detection has been used in a different way in Zend Framework:

https://github.com/krakjoe/apcu/issues/43#issuecomment-24978761

Remaining tasks

- Discuss
- Write patch

User interface changes

- None

API changes

- None

CommentFileSizeAuthor
#4 0001-Issue-2427861-Make-FastChainedBackend-compatible-and.patch1.44 KBfgm

Comments

fabianx’s picture

fabianx commented
Issue summary: View changes
berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Interesting idea.

I haven't had problems with that so far, not sure if that is because I have it enabled or because a real request quickly does set data that leads to an invalidate.

This won't actually help with the installer problem, because it still leads to a lot of invalidate calls. I'm wondering if we can optimize that too, maybe in a different issue. Similar to cache tags, so that we don't send an invalidate repeatedly, until a get() happens again, or something like that.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

another note, based on my comment in #2423591: Optimize config entity import.

The costly part is the markAsOutdated(), using memory backend would still result in all those queries.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Status: Active » Needs review
StatusFileSize
new 0001-Issue-2427861-Make-FastChainedBackend-compatible-and.patch1.44 KB

Rolled patch as suggested. Not sure the test bots cover such cases, though.

fabianx’s picture

fabianx commented

Looks great to me!

fabianx’s picture

fabianx commented

Berdir: Can you review / RTBC that one?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +D8 Accelerate Dev Days

Thanks for the patch, @fgm! :)

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

This makes sense to me as well.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Assigned: Unassigned » catch

Looks good to me, sending to catch.

effulgentsia’s picture

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

I'm not so sure about this:

  1. cache.backend.memory is only defined in development.services.yml, but CLI can be used on production sites too.
  2. Would it make more sense to make ChainedFastBackend capable of being constructed with $fast_backend = NULL (not NullBackend), to cover other code flows where a single site can sometimes have the fast backend available and sometimes not, and therefore, also need that timestamp updated?
fabianx’s picture

fabianx commented
Assigned: catch » Unassigned
Status: Needs review » Needs work

1. Oh it was moved back again? It was at some point in core.services.yml ...
2. Possibly - however it might be a larger change ...

2) might indeed be cleaner - unless there is any use case where we do not want timestamps to be updated.

@Berdir: Can you think of any such use case?

gapple’s picture

gapple
he/they
Primary language English
commented
Issue tags: +Triaged at DrupalCon Los Angeles 2015

I don't think enable_cli=0 will actually cause any issues as function_exists('apc_fetch') still returns true as long as the APCu extension exists, even if not enabled. As a result, the ChainedFastBackendFactory still returns a ChainedFastBackend and updates to lastWriteTimestamp are still written to the consistent backend.
Since APC is disabled cache.backend.apcu essentially acts as a null cache for CLI requests.

PHP 5.6 + APC 4.0.7

$ php -a
Interactive shell

php > var_dump(ini_get('apc.enabled'));
string(1) "1"
php > var_dump(ini_get('apc.enabled_cli'));
bool(false)
php > var_dump(function_exists('apc_fetch'));
bool(true)
php > var_dump(apc_store('test', 'value'));
bool(false)
php > var_dump(apc_fetch('test'));
bool(false)
xjm’s picture

xjm
she/her
Primary language English
commented

(Saving proposed issue credit for discussion and triage participants at LA.)

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

cache.backend.memory is only defined in development.services.yml, but CLI can be used on production sites too.

Got added back to core.services.yml in #2501117: Add static caching for PermissionsHashGenerator::generate().

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

fabianx’s picture

fabianx commented
Status: Needs work » Fixed

Per #12 this issue seems not to be a problem as if its disabled it acts as a NULL backend, which is fine.

Status: Fixed » Closed (fixed)

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