From 83cf77b622d4739c884181e73592d5c1bd94e2d3 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 9 Aug 2018 14:09:00 +0800 Subject: [PATCH] MDL-63101 cache: Improve accuracy of cache event invalidation --- cache/classes/helper.php | 12 ++- cache/classes/loaders.php | 143 ++++++++++++++++++++++++++++++----- cache/tests/cache_test.php | 74 +++++++++++++++--- cache/tests/fixtures/lib.php | 54 ++++--------- cache/upgrade.txt | 4 + 5 files changed, 218 insertions(+), 69 deletions(-) diff --git a/cache/classes/helper.php b/cache/classes/helper.php index c25369e02da..383be376b16 100644 --- a/cache/classes/helper.php +++ b/cache/classes/helper.php @@ -239,6 +239,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)) { @@ -265,8 +266,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); @@ -310,6 +314,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)) { @@ -333,8 +338,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); diff --git a/cache/classes/loaders.php b/cache/classes/loaders.php index 501b21eaeca..bf47081c60d 100644 --- a/cache/classes/loaders.php +++ b/cache/classes/loaders.php @@ -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 -. + * + * @var string + */ + protected static $purgetoken; + /** * The definition used when loading this cache if there was one. * @var cache_definition @@ -1144,13 +1152,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; } } @@ -1236,18 +1301,40 @@ class cache_application extends cache implements cache_loader_with_locking { if ($lastinvalidation === false) { // This is a new session, there won't be anything to invalidate. Set the time of the last invalidation and // move on. - $this->set('lastinvalidation', cache::now()); + $this->set('lastinvalidation', self::get_purge_token()); return; } else if ($lastinvalidation == cache::now()) { // We've already invalidated during this request. 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 = cache::make('core', 'eventinvalidation'); $events = $cache->get_many($definition->get_invalidation_events()); $todelete = array(); $purgeall = false; + // Iterate the returned data for the events. foreach ($events as $event => $keys) { if ($keys === false) { @@ -1255,10 +1342,10 @@ class cache_application extends cache implements cache_loader_with_locking { 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) { + if (self::compare_purge_tokens($purgetoken, $lastinvalidation) > 0) { if ($key === 'purged') { $purgeall = true; break; @@ -1276,7 +1363,7 @@ class cache_application extends cache implements cache_loader_with_locking { } // Set the time of the last invalidation. if ($purgeall || !empty($todelete)) { - $this->set('lastinvalidation', cache::now()); + $this->set('lastinvalidation', self::get_purge_token(true)); } } } @@ -1637,18 +1724,40 @@ class cache_session extends cache { if ($lastinvalidation === false) { // This is a new session, there won't be anything to invalidate. Set the time of the last invalidation and // move on. - $this->set('lastsessioninvalidation', cache::now()); + $this->set('lastsessioninvalidation', self::get_purge_token()); return; - } else if ($lastinvalidation == cache::now()) { + } else if ($lastinvalidation == self::get_purge_token()) { // We've already invalidated during this request. 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 = cache::make('core', 'eventinvalidation'); $events = $cache->get_many($definition->get_invalidation_events()); $todelete = array(); $purgeall = false; + // Iterate the returned data for the events. foreach ($events as $event => $keys) { if ($keys === false) { @@ -1656,10 +1765,10 @@ class cache_session extends cache { 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; @@ -1677,7 +1786,7 @@ class cache_session extends cache { } // Set the time of the last invalidation. if ($purgeall || !empty($todelete)) { - $this->set('lastsessioninvalidation', cache::now()); + $this->set('lastsessioninvalidation', self::get_purge_token(true)); } } } @@ -2214,4 +2323,4 @@ class cache_session extends cache { */ class cache_request extends cache { // This comment appeases code pre-checker ;) ! -} \ No newline at end of file +} diff --git a/cache/tests/cache_test.php b/cache/tests/cache_test.php index b18cbb1419d..3026debbee1 100644 --- a/cache/tests/cache_test.php +++ b/cache/tests/cache_test.php @@ -1009,7 +1009,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)); @@ -1035,6 +1035,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, @@ -1046,6 +1047,7 @@ class core_cache_testcase extends advanced_testcase { 'crazyevent' ) )); + $cache = cache::make('phpunit', 'eventinvalidationtest'); $this->assertFalse($cache->get('testkey1')); @@ -1053,23 +1055,29 @@ 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); + $cache->set('lastinvalidation', cache::now(true) - 15); + // 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(cache::now(true) - 15, $cache->get('lastinvalidation')); // Now set the event invalidation to newer than the lastinvalidation time. $invalidationcache->set('crazyevent', array('testkey2' => cache::now() - 5)); @@ -1077,18 +1085,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')); } @@ -2223,4 +2231,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')); + } + } diff --git a/cache/tests/fixtures/lib.php b/cache/tests/fixtures/lib.php index 95c2fe1fa3d..0a825edc19c 100644 --- a/cache/tests/fixtures/lib.php +++ b/cache/tests/fixtures/lib.php @@ -534,47 +534,21 @@ class cache_phpunit_factory extends cache_factory { public static function phpunit_disable() { parent::disable(); } +} +/** + * Cache PHPUnit specific Cache helper. + * + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class cache_phpunit_cache extends cache { /** - * @var bool Whether the warning notice about alternative cache store used has been displayed. + * 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. */ - protected $altcachestorenotice = false; - - /** - * Creates a store instance given its name and configuration. - * - * If the store has already been instantiated then the original object will be returned. (reused) - * - * @param string $name The name of the store (must be unique remember) - * @param array $details - * @param cache_definition $definition The definition to instantiate it for. - * @return boolean|cache_store - */ - public function create_store_from_config($name, array $details, cache_definition $definition) { - - if (isset($details['use_test_store'])) { - // name, plugin, alt - $class = 'cachestore_'.$details['plugin']; - $method = 'initialise_unit_test_instance'; - if (class_exists($class) && method_exists($class, $method)) { - $instance = $class::$method($definition); - - if ($instance) { - return $instance; - } - } - - // Notify user that alternative store is being used, so action can be taken. - if (!$this->altcachestorenotice) { - echo PHP_EOL . "++ WARNING: " . 'Failed to use "' . $details['plugin'] . '" cache store, alt "' . - $details['alt']['plugin'] . '" cache store is used.' . PHP_EOL . PHP_EOL; - $this->altcachestorenotice = true; - } - $details = $details['alt']; - $details['class'] = 'cachestore_'.$details['plugin']; - $name = $details['name']; - } - - return parent::create_store_from_config($name, $details, $definition); + public static function simulate_new_request() { + self::$now += 0.1; + self::$purgetoken = null; } -} \ No newline at end of file +} diff --git a/cache/upgrade.txt b/cache/upgrade.txt index 0b4cbb941fc..f6e06d5a26d 100644 --- a/cache/upgrade.txt +++ b/cache/upgrade.txt @@ -1,6 +1,10 @@ This files describes API changes in /cache/stores/* - cache store plugins. Information provided here is intended especially for developers. +=== 3.1.14 === +* 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.1 === * Cache stores has a new feature DEREFERENCES_OBJECTS. This allows the cache loader to decide if it needs to handle dereferencing or whether the data