Problem/Motivation

Child issue of #2497243: Replace Symfony container with a Drupal one, stored in cache.

The chained fast cache backend currently invalidates caches using the cache itself. This means if multiple webheads with local fast backends are used, invalidations are not consistent across all webheads - in case NTP is not used. (The timestamp solution works, but an atomic counter that cache tags provide is better).

Proposed resolution

Use the cache tag invalidation provider to track cache invalidations.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#25 chainedfastbackend-invalidations-2508417-25.patch14.38 KBjhedstrom
#25 interdiff.txt548 bytesjhedstrom
#9 chainedfastbackend-invalidations-2508417-09.patch14.26 KBjhedstrom
#9 interdiff.txt1.78 KBjhedstrom
#1 chainedfastbackend-invalidations-2508417-01.patch17.12 KBjhedstrom
#4 interdiff.txt13.45 KBjhedstrom
#4 chainedfastbackend-invalidations-2508417-04.patch14.38 KBjhedstrom
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Status: Active » Needs review
FileSize
chainedfastbackend-invalidations-2508417-01.patch17.12 KB

Status: Needs review » Needs work

The last submitted patch, 1: chainedfastbackend-invalidations-2508417-01.patch, failed testing.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented
Issue summary: View changes 
  1. +++ b/core/lib/Drupal/Core/Cache/ChainedFastBackend.php
@@ -68,11 +68,11 @@ class ChainedFastBackend implements CacheBackendInterface, CacheTagsInvalidatorI
+  protected $checksumProvider;

@@ -84,11 +84,11 @@ class ChainedFastBackend implements CacheBackendInterface, CacheTagsInvalidatorI
+  public function __construct(CacheBackendInterface $consistent_backend, CacheBackendInterface $fast_backend, $bin, CacheTagsChecksumInterface $checksum_provider) {
...
+    $this->checksumProvider = $checksum_provider;

    The fast backend itself needs to take care of that, no need to do checksumming here. 

  2. +++ b/core/lib/Drupal/Core/Cache/ChainedFastBackend.php
@@ -107,50 +107,41 @@ public function getMultiple(&$cids, $allow_invalid = FALSE) {
+    // Even if items were successfully fetched from the fast backend, they are
+    // potentially invalid if their cache tag has been invalidated since the bin
+    // was written to in the consistent backend, so only keep ones that aren't.

    This should never happen, unless allowInvalid is TRUE, but then the data is okay to be invalid regardless of hold old or outdated or inconsistent it is. 

  3. +++ b/core/lib/Drupal/Core/Cache/ChainedFastBackend.php
@@ -173,11 +164,14 @@ public function getMultiple(&$cids, $allow_invalid = FALSE) {
+    // Append the chained fast tag.
+    $tags[] = self::CACHE_TAG_PREFIX . $this->bin;
     $this->consistentBackend->set($cid, $data, $expire, $tags);

    This should never be added to the consistentBackend(). 

  4. +++ b/core/lib/Drupal/Core/Cache/ChainedFastBackend.php
@@ -237,6 +233,17 @@ public function invalidateMultiple(array $cids) {
+    // Do not invalidate the fast chained tag or looping results.
+    // @see self::markAsOutdated()
+    foreach ($tags as $key => $tag) {
+      if (strpos($tag, self::CACHE_TAG_PREFIX) === 0) {
+        unset($tags[$key]);
+      }
+    }
+    if (empty($tags)) {
+      return;
+    }
+
     if ($this->consistentBackend instanceof CacheTagsInvalidatorInterface) {
       $this->consistentBackend->invalidateTags($tags);
     }

    This should all not be needed, the consistentBackend should already get its cache tags invalidated, the same for the fast backend.

    Can remove the whole interface.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Status: Needs work » Needs review
FileSize
chainedfastbackend-invalidations-2508417-04.patch14.38 KB
interdiff.txt13.45 KB

This is drastically simplified when moving away from the timestamp/write-based invalidation.

Removing the cache tags invalidator interface requires that cache tags *are* sent to the fast backend.

The last submitted patch, 4: chainedfastbackend-invalidations-2508417-04.patch, failed testing.

jhedstrom queued 4: chainedfastbackend-invalidations-2508417-04.patch for re-testing.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented 
+++ b/core/lib/Drupal/Core/Cache/ChainedFastBackend.php
@@ -174,10 +133,10 @@ public function getMultiple(&$cids, $allow_invalid = FALSE) {
-    $this->markAsOutdated();

@@ -185,11 +144,10 @@ public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = array
-    $this->markAsOutdated();

These two are actually still needed ...

The last submitted patch, 4: chainedfastbackend-invalidations-2508417-04.patch, failed testing.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
FileSize
interdiff.txt1.78 KB
chainedfastbackend-invalidations-2508417-09.patch14.26 KB

This should address #7. Not sure why the CKEditor test is failing in the above patch though (it passes locally).

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Assigned: jhedstrom » Unassigned

Status: Needs review » Needs work

The last submitted patch, 9: chainedfastbackend-invalidations-2508417-09.patch, failed testing.

Status: Needs work » Needs review

jhedstrom queued 9: chainedfastbackend-invalidations-2508417-09.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 9: chainedfastbackend-invalidations-2508417-09.patch, failed testing.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented 
+++ b/core/lib/Drupal/Core/Cache/ChainedFastBackend.php
@@ -118,10 +118,10 @@ public function getMultiple(&$cids, $allow_invalid = FALSE) {
-        // Only write the chained fast cache tag to the fast backend since any
-        // other cache tag invalidation results in an invalidation of the whole
-        // fast backend.
-        $this->fastBackend->set($item->cid, $item->data, $item->expire, [self::CACHE_TAG_PREFIX . $this->bin]);
+        // Append the bin invalidation tag.
+        $tags = $item->tags + [self::CACHE_TAG_PREFIX . $this->bin];
+
+        $this->fastBackend->set($item->cid, $item->data, $item->expire, $tags);

No, this chunk is wrong and was correct before ...

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented

#14 is wrong ... nvm me :)

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented

I cannot reproduce the test fails in #9 locally on either php54 or php55.

Status: Needs work » Needs review

Fabianx queued 9: chainedfastbackend-invalidations-2508417-09.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 9: chainedfastbackend-invalidations-2508417-09.patch, failed testing.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Assigned: Unassigned » jhedstrom

Ah, interesting, these tests do fail on the command line. Digging into that now.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Assigned: jhedstrom » Unassigned

So from what I can tell, all of the failures that involve the "Undefined index: base" exception are due to Views relationship plugins being set to the 'broken' pluginId. In testing Drupal\comment\Tests\Views\CommentRestExportTest locally:

viewsHandlerManager::getHandler() is called with

$item = [
  'field' => 'node',
  'id' => 'node',
  'table' => 'comment_field_data',
  'required' => true
  'plugin_id' => 'standard',
];

The fact that views is looking for a field called node in the comment_field_table seems to indicate something going wrong elsewhere. What's really odd is that all the cache tests for this change are passing, so these failures might indicate some incomplete coverage there.

The Drupal\ckeditor\Tests\CKEditorLoadingTest continues to pass through the UI, but fails from run-tests.sh, and doesn't appear related to the views data issue.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Assigned: Unassigned » jhedstrom
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented

After a bit more digging, I am fairly certain the fails here are due to static cache issues. For instance, on the CKEditorLoadingTest, the mismatch that causes the test to fail is due to new plugins provided by the ckeditor_test module not properly being discovered. When these pass via the UI and fail via the test running script, it indicates a very subtle difference in execution where certain static caches are persisting only on the command line test runner.

The obvious culprit was in DatabaseCacheTagsChecksum::invalidateTags():

        // Only invalidate tags once per request unless they are written again.
        if (isset($this->invalidatedTags[$tag])) {
          continue;
        }

but when I simply removed that logic for debugging purposes, the fail still persists.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Assigned: jhedstrom » Unassigned

Wasn't able to track this down completely. In addition to the snippet above, this bit in DatabaseCacheTagsChecksum::calculateChecksum() is also coming into play in terms of static caches causing invalidated items to look valid:

    $query_tags = array_diff($tags, array_keys($this->tagCache));
    if ($query_tags) {
      $db_tags = array();
      try {
        $db_tags = $this->connection->query('SELECT tag, invalidations FROM {cachetags} WHERE tag IN ( :tags[] )', array(':tags[]' => $query_tags))
          ->fetchAllKeyed();
        $this->tagCache += $db_tags;

The cid for the CKEditor issue is 'ckeditor_plugins' if anybody else picks this up while I'm offline.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Assigned: Unassigned » jhedstrom
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Status: Needs work » Needs review
FileSize
interdiff.txt548 bytes
chainedfastbackend-invalidations-2508417-25.patch14.38 KB

This fixes some of the fails, but not the views handler ones.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented 
+++ b/core/lib/Drupal/Core/Cache/ChainedFastBackend.php
@@ -228,6 +228,8 @@ public function reset() {
     Cache::invalidateTags([self::CACHE_TAG_PREFIX . $this->bin]);
+    // Reset static caches in the checksum service.
+    \Drupal::service('cache_tags.invalidator.checksum')->reset();

Uhm, should invalidateTags not itself invalidate or rather invalidate and update the tag of itself?

That seems like a bug in the cache tag implementation.

Status: Needs review » Needs work

The last submitted patch, 25: chainedfastbackend-invalidations-2508417-25.patch, failed testing.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented

Okay, so there are consistent cache tag implementations and inconsistent cache tag implementations ...

A backend like APCu needs to use a consistent cache tag implementation, that always writes through and does not internally statically cache the tags ...

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Assigned: jhedstrom » Unassigned
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Assigned: Unassigned » jhedstrom
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Assigned: jhedstrom » Unassigned

I don't think using cache tags here is going to work. After discussing with Berdir, the decision to leave the fast cache backend independent of cache tags was explicit, and only sending in a custom bin invalidation tag has other issues with static caching as described above.

This might be won't fix unless there is a good reason to pull in the atomic counter concept.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented

Well, to leave the fast cache backend independent of cache tags for all things, but using just the bin specific tag should be okay (and catch +1ed in general). So I think it is the implementation chosen here that is the problem, not the idea of using a consistent cache tags service in general.

I am also okay to use one atomic counter however - but it also means it needs to be re-implemented by memcache, redis, etc.

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.

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

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

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

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

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

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

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

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

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

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

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

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

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

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.