Issue #3018203 by Berdir, davidwhthomas, slasher13: Support delayed c…

.travis.yml

@@ -24,6 +24,8 @@ env:
     # force composer 1.8+ to use a specific folder as home
     - COMPOSER_HOME="$HOME/.composer/"
 
+    - DRUPAL_TI_DRUSH_VERSION="drush/drush:^9"
+
     # Configuration variables.
     - DRUPAL_TI_MODULE_NAME="redis"
     - DRUPAL_TI_SIMPLETEST_GROUP="redis"
@@ -62,7 +64,7 @@ env:
     - DRUPAL_TI_BEHAT_BROWSER="firefox"
 
     # Set Drupal version in which to run tests.
-    - DRUPAL_TI_CORE_BRANCH="8.7.x"
+    - DRUPAL_TI_CORE_BRANCH="8.8.x"
 
     # PHPUnit specific commandline arguments.
     - DRUPAL_TI_PHPUNIT_ARGS="--verbose --debug"

redis.info.yml

@@ -2,5 +2,5 @@ name: Redis
 description: Provide a module placeholder, for using as dependency for module that needs Redis.
 package: Performance
 type: module
-core: 8.x
+core_version_requirement: ^8.8 || ^9
 configure: redis.admin_display

src/Cache/CacheBase.php

@@ -96,6 +96,13 @@ abstract class CacheBase implements CacheBackendInterface {
    */
   protected $lastDeleteAll = NULL;
 
+  /**
+   * Delayed deletions for deletions during a transaction.
+   *
+   * @var string[]
+   */
+  protected $delayedDeletions = [];
+
   /**
    * Get TTL for CACHE_PERMANENT items.
    *
@@ -144,6 +151,49 @@ public function delete($cid) {
     $this->deleteMultiple([$cid]);
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function deleteMultiple(array $cids) {
+    $in_transaction = \Drupal::database()->inTransaction();
+    if ($in_transaction) {
+      if (empty($this->delayedDeletions)) {
+        \Drupal::database()->addRootTransactionEndCallback([$this, 'postRootTransactionCommit']);
+      }
+      $this->delayedDeletions = array_unique(array_merge($this->delayedDeletions, $cids));
+    }
+    else {
+      $this->doDeleteMultiple($cids);
+    }
+  }
+
+  /**
+   * Execute the deletion.
+   *
+   * This can be delayed to avoid race conditions.
+   *
+   * @param array $cids
+   *   An array of cache IDs to delete.
+   *
+   * @see static::deleteMultiple()
+   */
+  protected abstract function doDeleteMultiple(array $cids);
+
+  /**
+   * Callback to be invoked after a database transaction gets committed.
+   *
+   * Invalidates all delayed cache deletions.
+   *
+   * @param bool $success
+   *   Whether or not the transaction was successful.
+   */
+  public function postRootTransactionCommit($success) {
+    if ($success) {
+      $this->doDeleteMultiple($this->delayedDeletions);
+    }
+    $this->delayedDeletions = [];
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -251,6 +301,11 @@ protected function expandEntry(array $values, $allow_invalid) {
       return FALSE;
     }
 
+    // Ignore items that are scheduled for deletion.
+    if (in_array($values['cid'], $this->delayedDeletions)) {
+      return FALSE;
+    }
+
     $cache = (object) $values;
 
     $cache->tags = explode(' ', $cache->tags);

src/Cache/PhpRedis.php

@@ -102,10 +102,9 @@ public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = []) {
   /**
    * {@inheritdoc}
    */
-  public function deleteMultiple(array $cids) {
+  public function doDeleteMultiple(array $cids) {
     $keys = array_map([$this, 'getKey'], $cids);
     $this->client->del($keys);
   }
 
-
 }

src/Cache/Predis.php

@@ -102,12 +102,11 @@ public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = []) {
   /**
    * {@inheritdoc}
    */
-  public function deleteMultiple(array $cids) {
+  public function doDeleteMultiple(array $cids) {
     if (!empty($cids)) {
       $keys = array_map([$this, 'getKey'], $cids);
       $this->client->del($keys);
     }
   }
 
-
 }

src/Cache/RedisCacheTagsChecksum.php

@@ -3,6 +3,7 @@
 namespace Drupal\redis\Cache;
 
 use Drupal\Core\Cache\CacheTagsChecksumInterface;
+use Drupal\Core\Cache\CacheTagsChecksumTrait;
 use Drupal\Core\Cache\CacheTagsInvalidatorInterface;
 use Drupal\redis\ClientFactory;
 use Drupal\redis\RedisPrefixTrait;
@@ -13,6 +14,7 @@
 class RedisCacheTagsChecksum implements CacheTagsChecksumInterface, CacheTagsInvalidatorInterface {
 
   use RedisPrefixTrait;
+  use CacheTagsChecksumTrait;
 
   /**
    * Contains already loaded cache invalidations from the database.
@@ -51,87 +53,36 @@ public function __construct(ClientFactory $factory) {
   /**
    * {@inheritdoc}
    */
-  public function invalidateTags(array $tags) {
-    $keys_to_increment = [];
-    foreach ($tags as $tag) {
-      // Only invalidate tags once per request unless they are written again.
-      if (isset($this->invalidatedTags[$tag])) {
-        continue;
+  public function doInvalidateTags(array $tags) {
+    $keys = array_map([$this, 'getTagKey'], $tags);
+
+    // We want to differentiate between PhpRedis and Redis clients.
+    if ($this->clientType === 'PhpRedis') {
+      $multi = $this->client->multi();
+      foreach ($keys as $key) {
+        $multi->incr($key);
       }
-      $this->invalidatedTags[$tag] = TRUE;
-      unset($this->tagCache[$tag]);
-      $keys_to_increment[] = $this->getTagKey($tag);
+      $multi->exec();
     }
-    if ($keys_to_increment) {
-
-      // We want to differentiate between PhpRedis and Redis clients.
-      if ($this->clientType === 'PhpRedis') {
-        $multi = $this->client->multi(\Redis::PIPELINE);
-        foreach ($keys_to_increment as $key) {
-          $multi->incr($key);
-        }
-        $multi->exec();
-      }
-      elseif ($this->clientType === 'Predis') {
-
-        $pipe = $this->client->pipeline();
-        foreach ($keys_to_increment as $key) {
-          $pipe->incr($key);
-        }
-        $pipe->execute();
-      }
-    }
-
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getCurrentChecksum(array $tags) {
-    // Remove tags that were already invalidated during this request from the
-    // static caches so that another invalidation can occur later in the same
-    // request. Without that, written cache items would not be invalidated
-    // correctly.
-    foreach ($tags as $tag) {
-      unset($this->invalidatedTags[$tag]);
-    }
-    return $this->calculateChecksum($tags);
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function isValid($checksum, array $tags) {
-    return $checksum == $this->calculateChecksum($tags);
-  }
+    elseif ($this->clientType === 'Predis') {
 
-  /**
-   * {@inheritdoc}
-   */
-  public function calculateChecksum(array $tags) {
-    $checksum = 0;
-
-    $fetch = array_values(array_diff($tags, array_keys($this->tagCache)));
-    if ($fetch) {
-      $keys = array_map([$this, 'getTagKey'], $fetch);
-      foreach ($this->client->mget($keys) as $index => $invalidations) {
-        $this->tagCache[$fetch[$index]] = $invalidations ?: 0;
+      $pipe = $this->client->pipeline();
+      foreach ($keys as $key) {
+        $pipe->incr($key);
       }
+      $pipe->execute();
     }
-
-    foreach ($tags as $tag) {
-      $checksum += $this->tagCache[$tag];
-    }
-
-    return $checksum;
   }
 
   /**
    * {@inheritdoc}
    */
-  public function reset() {
-    $this->tagCache = [];
-    $this->invalidatedTags = [];
+  protected function getTagInvalidationCounts(array $tags) {
+    $keys = array_map([$this, 'getTagKey'], $tags);
+    // The mget command returns the values as an array with numeric keys,
+    // combine it with the tags array to get the expected return value and run
+    // it through intval() to convert to integers and FALSE to 0.
+    return array_map('intval', array_combine($tags, $this->client->mget($keys)));
   }
 
   /**
@@ -147,4 +98,13 @@ protected function getTagKey($tag) {
     return $this->getPrefix() . ':cachetags:' . $tag;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function getDatabaseConnection() {
+    // This is not injected to avoid a dependency on the database in the
+    // critical path. It is only needed during cache tag invalidations.
+    return \Drupal::database();
+  }
+
 }

tests/src/Functional/WebTest.php

@@ -26,6 +26,11 @@ class WebTest extends BrowserTestBase {
    */
   public static $modules = ['redis', 'block'];
 
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'classy';
+
   /**
    * {@inheritdoc}
    */