MDL-63101 cache: Improve accuracy of cache event invalidation
authorAndrew Nicols <andrew@nicols.co.uk>
Thu, 9 Aug 2018 06:09:00 +0000 (14:09 +0800)
committerAndrew Nicols <andrew@nicols.co.uk>
Mon, 27 Aug 2018 06:43:42 +0000 (14:43 +0800)
cache/classes/helper.php
cache/classes/loaders.php
cache/tests/cache_test.php
cache/tests/fixtures/lib.php
cache/upgrade.txt

index c2aa399..2776339 100644 (file)
@@ -240,6 +240,7 @@ class cache_helper {
         $invalidationeventset = false;
         $factory = cache_factory::instance();
         $inuse = $factory->get_caches_in_use();
+        $purgetoken = null;
         foreach ($instance->get_definitions() as $name => $definitionarr) {
             $definition = cache_definition::load($name, $definitionarr);
             if ($definition->invalidates_on_event($event)) {
@@ -266,8 +267,11 @@ class cache_helper {
                         $data = array();
                     }
                     // Add our keys to them with the current cache timestamp.
+                    if (null === $purgetoken) {
+                        $purgetoken = cache::get_purge_token(true);
+                    }
                     foreach ($keys as $key) {
-                        $data[$key] = cache::now();
+                        $data[$key] = $purgetoken;
                     }
                     // Set that data back to the cache.
                     $cache->set($event, $data);
@@ -315,6 +319,7 @@ class cache_helper {
         $invalidationeventset = false;
         $factory = cache_factory::instance();
         $inuse = $factory->get_caches_in_use();
+        $purgetoken = null;
         foreach ($instance->get_definitions() as $name => $definitionarr) {
             $definition = cache_definition::load($name, $definitionarr);
             if ($definition->invalidates_on_event($event)) {
@@ -338,8 +343,11 @@ class cache_helper {
                     // Get the event invalidation cache.
                     $cache = cache::make('core', 'eventinvalidation');
                     // Create a key to invalidate all.
+                    if (null === $purgetoken) {
+                        $purgetoken = cache::get_purge_token(true);
+                    }
                     $data = array(
-                        'purged' => cache::now()
+                        'purged' => $purgetoken,
                     );
                     // Set that data back to the cache.
                     $cache->set($event, $data);
index 168124d..691d2fb 100644 (file)
@@ -50,6 +50,14 @@ class cache implements cache_loader {
      */
     protected static $now;
 
+    /**
+     * A purge token used to distinguish between multiple cache purges in the same second.
+     * This is in the format <microtime>-<random string>.
+     *
+     * @var string
+     */
+    protected static $purgetoken;
+
     /**
      * The definition used when loading this cache if there was one.
      * @var cache_definition
@@ -286,33 +294,58 @@ class cache implements cache_loader {
             return;
         }
 
+        // Each cache stores the current 'lastinvalidation' value within the cache itself.
         $lastinvalidation = $this->get('lastinvalidation');
         if ($lastinvalidation === false) {
-            // This is a new cache or purged globally, there won't be anything to invalidate.
-            // Set the time of the last invalidation and move on.
-            $this->set('lastinvalidation', self::now());
+            // There is currently no  value for the lastinvalidation token, therefore the token is not set, and there
+            // can be nothing to invalidate.
+            // Set the lastinvalidation value to the current purge token and return early.
+            $this->set('lastinvalidation', self::get_purge_token());
             return;
-        } else if ($lastinvalidation == self::now()) {
-            // We've already invalidated during this request.
+        } else if ($lastinvalidation == self::get_purge_token()) {
+            // The current purge request has already been fully handled by this cache.
             return;
         }
 
-        // Get the event invalidation cache.
+        /*
+         * Now that the whole cache check is complete, we check the meaning of any specific cache invalidation events.
+         * These are stored in the core/eventinvalidation cache as an multi-dimensinoal array in the form:
+         *  [
+         *      eventname => [
+         *          keyname => purgetoken,
+         *      ]
+         *  ]
+         *
+         * The 'keyname' value is used to delete a specific key in the cache.
+         * If the keyname is set to the special value 'purged', then the whole cache is purged instead.
+         *
+         * The 'purgetoken' is the token that this key was last purged.
+         * a) If the purgetoken matches the last invalidation, then the key/cache is not purged.
+         * b) If the purgetoken is newer than the last invalidation, then the key/cache is not purged.
+         * c) If the purge token is older than the last invalidation, or it has a different token component, then the
+         *    cache is purged.
+         *
+         * Option b should not happen under normal operation, but may happen in race condition whereby a long-running
+         * request's cache is cleared in another process during that request, and prior to that long-running request
+         * creating the cache. In such a condition, it would be incorrect to clear that cache.
+         */
         $cache = self::make('core', 'eventinvalidation');
         $events = $cache->get_many($this->definition->get_invalidation_events());
         $todelete = array();
         $purgeall = false;
+
         // Iterate the returned data for the events.
         foreach ($events as $event => $keys) {
             if ($keys === false) {
                 // No data to be invalidated yet.
                 continue;
             }
+
             // Look at each key and check the timestamp.
-            foreach ($keys as $key => $timestamp) {
+            foreach ($keys as $key => $purgetoken) {
                 // If the timestamp of the event is more than or equal to the last invalidation (happened between the last
-                // invalidation and now)then we need to invaliate the key.
-                if ($timestamp >= $lastinvalidation) {
+                // invalidation and now)then we need to invaliate the key.
+                if (self::compare_purge_tokens($purgetoken, $lastinvalidation) > 0) {
                     if ($key === 'purged') {
                         $purgeall = true;
                         break;
@@ -330,7 +363,7 @@ class cache implements cache_loader {
         }
         // Set the time of the last invalidation.
         if ($purgeall || !empty($todelete)) {
-            $this->set('lastinvalidation', self::now());
+            $this->set('lastinvalidation', self::get_purge_token(true));
         }
     }
 
@@ -1186,13 +1219,70 @@ class cache implements cache_loader {
      * This stamp needs to be used for all ttl and time based operations to ensure that we don't end up with
      * timing issues.
      *
-     * @return int
+     * @param   bool    $float Whether to use floating precision accuracy.
+     * @return  int|float
      */
-    public static function now() {
+    public static function now($float = false) {
         if (self::$now === null) {
-            self::$now = time();
+            self::$now = microtime(true);
+        }
+
+        if ($float) {
+            return self::$now;
+        } else {
+            return (int) self::$now;
+        }
+    }
+
+    /**
+     * Get a 'purge' token used for cache invalidation handling.
+     *
+     * Note: This function is intended for use from within the Cache API only and not by plugins, or cache stores.
+     *
+     * @param   bool    $reset  Whether to reset the token and generate a new one.
+     * @return  string
+     */
+    public static function get_purge_token($reset = false) {
+        if (self::$purgetoken === null || $reset) {
+            self::$now = null;
+            self::$purgetoken = self::now(true) . '-' . uniqid('', true);
+        }
+
+        return self::$purgetoken;
+    }
+
+    /**
+     * Compare a pair of purge tokens.
+     *
+     * If the two tokens are identical, then the return value is 0.
+     * If the time component of token A is newer than token B, then a positive value is returned.
+     * If the time component of token B is newer than token A, then a negative value is returned.
+     *
+     * Note: This function is intended for use from within the Cache API only and not by plugins, or cache stores.
+     *
+     * @param   string  $tokena
+     * @param   string  $tokenb
+     * @return  int
+     */
+    public static function compare_purge_tokens($tokena, $tokenb) {
+        if ($tokena === $tokenb) {
+            // There is an exact match.
+            return 0;
+        }
+
+        // The token for when the cache was last invalidated.
+        list($atime) = explode('-', "{$tokena}-", 2);
+
+        // The token for this cache.
+        list($btime) = explode('-', "{$tokenb}-", 2);
+
+        if ($atime >= $btime) {
+            // Token A is newer.
+            return 1;
+        } else {
+            // Token A is older.
+            return -1;
         }
-        return self::$now;
     }
 }
 
index 8d04cd1..6aa268f 100644 (file)
@@ -1003,7 +1003,7 @@ class core_cache_testcase extends advanced_testcase {
         $timefile = $CFG->dataroot."/cache/cachestore_file/default_application/phpunit_eventinvalidationtest/las-cache/lastinvalidation-$hash.cache";
         // Make sure the file is correct.
         $this->assertTrue(file_exists($timefile));
-        $timecont = serialize(cache::now() - 60); // Back 60sec in the past to force it to re-invalidate.
+        $timecont = serialize(cache::now(true) - 60); // Back 60sec in the past to force it to re-invalidate.
         make_writable_directory(dirname($timefile));
         file_put_contents($timefile, $timecont);
         $this->assertTrue(file_exists($timefile));
@@ -1029,6 +1029,7 @@ class core_cache_testcase extends advanced_testcase {
 
         // Test 2: Rebuild and test the invalidation of the event via the invalidation cache.
         cache_factory::reset();
+
         $instance = cache_config_testing::instance();
         $instance->phpunit_add_definition('phpunit/eventinvalidationtest', array(
             'mode' => cache_store::MODE_APPLICATION,
@@ -1040,6 +1041,7 @@ class core_cache_testcase extends advanced_testcase {
                 'crazyevent'
             )
         ));
+
         $cache = cache::make('phpunit', 'eventinvalidationtest');
         $this->assertFalse($cache->get('testkey1'));
 
@@ -1047,23 +1049,30 @@ class core_cache_testcase extends advanced_testcase {
 
         // Make a new cache class.  This should should invalidate testkey2.
         $cache = cache::make('phpunit', 'eventinvalidationtest');
-        // Timestamp should have updated to cache::now().
-        $this->assertEquals(cache::now(), $cache->get('lastinvalidation'));
+
+        // Invalidation token should have been reset.
+        $this->assertEquals(cache::get_purge_token(), $cache->get('lastinvalidation'));
 
         // Set testkey2 data.
         $cache->set('testkey2', 'test data 2');
+
         // Backdate the event invalidation time by 30 seconds.
         $invalidationcache = cache::make('core', 'eventinvalidation');
         $invalidationcache->set('crazyevent', array('testkey2' => cache::now() - 30));
+
         // Lastinvalidation should already be cache::now().
-        $this->assertEquals(cache::now(), $cache->get('lastinvalidation'));
+        $this->assertEquals(cache::get_purge_token(), $cache->get('lastinvalidation'));
+
         // Set it to 15 seconds ago so that we know if it changes.
-        $cache->set('lastinvalidation', cache::now() - 15);
+        $pasttime = cache::now(true) - 15;
+        $cache->set('lastinvalidation', $pasttime);
+
         // Make a new cache class.  This should not invalidate anything.
         cache_factory::instance()->reset_cache_instances();
         $cache = cache::make('phpunit', 'eventinvalidationtest');
+
         // Lastinvalidation shouldn't change since it was already newer than invalidation event.
-        $this->assertEquals(cache::now() - 15, $cache->get('lastinvalidation'));
+        $this->assertEquals($pasttime, $cache->get('lastinvalidation'));
 
         // Now set the event invalidation to newer than the lastinvalidation time.
         $invalidationcache->set('crazyevent', array('testkey2' => cache::now() - 5));
@@ -1071,18 +1080,18 @@ class core_cache_testcase extends advanced_testcase {
         cache_factory::instance()->reset_cache_instances();
         $cache = cache::make('phpunit', 'eventinvalidationtest');
         // Lastinvalidation timestamp should have updated to cache::now().
-        $this->assertEquals(cache::now(), $cache->get('lastinvalidation'));
+        $this->assertEquals(cache::get_purge_token(), $cache->get('lastinvalidation'));
 
         // Now simulate a purge_by_event 5 seconds ago.
         $invalidationcache = cache::make('core', 'eventinvalidation');
-        $invalidationcache->set('crazyevent', array('purged' => cache::now() - 5));
+        $invalidationcache->set('crazyevent', array('purged' => cache::now(true) - 5));
         // Set our lastinvalidation timestamp to 15 seconds ago.
-        $cache->set('lastinvalidation', cache::now() - 15);
+        $cache->set('lastinvalidation', cache::now(true) - 15);
         // Make a new cache class.  This should invalidate the cache.
         cache_factory::instance()->reset_cache_instances();
         $cache = cache::make('phpunit', 'eventinvalidationtest');
         // Lastinvalidation timestamp should have updated to cache::now().
-        $this->assertEquals(cache::now(), $cache->get('lastinvalidation'));
+        $this->assertEquals(cache::get_purge_token(), $cache->get('lastinvalidation'));
 
     }
 
@@ -2272,4 +2281,50 @@ class core_cache_testcase extends advanced_testcase {
         $this->assertArrayNotHasKey($sessionid, $endstats);
         $this->assertArrayNotHasKey($requestid, $endstats);
     }
+
+    /**
+     * Tests session cache event purge and subsequent visit in the same request.
+     *
+     * This test simulates a cache being created, a value being set, then the value being purged.
+     * A subsequent use of the same cache is started in the same request which fills the cache.
+     * A new request is started a short time later.
+     * The cache should be filled.
+     */
+    public function test_session_event_purge_same_second() {
+        $instance = cache_config_testing::instance();
+        $instance->phpunit_add_definition('phpunit/eventpurgetest', array(
+            'mode' => cache_store::MODE_SESSION,
+            'component' => 'phpunit',
+            'area' => 'eventpurgetest',
+            'invalidationevents' => array(
+                'crazyevent',
+            )
+        ));
+
+        // Create the cache, set a value, and immediately purge it by event.
+        $cache = cache::make('phpunit', 'eventpurgetest');
+        $cache->set('testkey1', 'test data 1');
+        $this->assertEquals('test data 1', $cache->get('testkey1'));
+        cache_helper::purge_by_event('crazyevent');
+        $this->assertFalse($cache->get('testkey1'));
+
+        // Set up the cache again in the same request and add a new value back in.
+        $factory = \cache_factory::instance();
+        $factory->reset_cache_instances();
+        $cache = cache::make('phpunit', 'eventpurgetest');
+        $cache->set('testkey1', 'test data 2');
+        $this->assertEquals('test data 2', $cache->get('testkey1'));
+
+        // Trick the cache into thinking that this is a new request.
+        cache_phpunit_cache::simulate_new_request();
+        $factory = \cache_factory::instance();
+        $factory->reset_cache_instances();
+
+        // Set up the cache again.
+        // This is a subsequent request at a new time, so we instead the invalidation time will be checked.
+        // The invalidation time should match the last purged time and the cache will not be re-purged.
+        $cache = cache::make('phpunit', 'eventpurgetest');
+        $this->assertEquals('test data 2', $cache->get('testkey1'));
+    }
+
 }
index 597c00a..6c42c39 100644 (file)
@@ -534,4 +534,21 @@ class cache_phpunit_factory extends cache_factory {
     public static function phpunit_disable() {
         parent::disable();
     }
-}
\ No newline at end of file
+}
+
+/**
+ * Cache PHPUnit specific Cache helper.
+ *
+ * @copyright  2018 Andrew Nicols <andrew@nicols.co.uk>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class cache_phpunit_cache extends cache {
+    /**
+     * Make the changes which simulate a new request within the cache.
+     * This essentially resets currently held static values in the class, and increments the current timestamp.
+     */
+    public static function simulate_new_request() {
+        self::$now += 0.1;
+        self::$purgetoken = null;
+    }
+}
index 16bbbf5..8c3e287 100644 (file)
@@ -1,6 +1,10 @@
 This files describes API changes in /cache/stores/* - cache store plugins.
 Information provided here is intended especially for developers.
 
+=== 3.6 ===
+* The `cache::now()` function now takes an optional boolean parameter to indicate that the cache should return a more
+  accurate time, generated by the PHP `microtime` function.
+
 === 3.3 ===
 * Identifiers and invalidation events have been explictly been marked as incompatible and will
   throw a coding exception. Unexpected results would have occurred if the previous behaviour was attempted.