L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-67020 Cache: Make locking work for local file caches

Browse Source 
* Add new requirelockingbeforewrite flag for cache definitions
* Implement native locking in cachestore_file (leveraging
file_lock_factory).
This commit is contained in:
Mark Johnson
2022-10-20 11:31:33 +01:00
parent dcc7d7b8df
commit 045ee091da
12 changed files with 328 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cache/classes/definition.php
Vendored
+24 -1
View File
@@ -208,6 +208,12 @@ class cache_definition {
*/
protected $requirelockingwrite = false;
/**
* Gets set to true if this definition requires a lock to be acquired before a write is attempted.
* @var bool
*/
protected $requirelockingbeforewrite = false;
/**
* Gets set to true if this definition requires searchable stores.
* @since Moodle 2.4.4
@@ -357,6 +363,7 @@ class cache_definition {
$requiremultipleidentifiers = false;
$requirelockingread = false;
$requirelockingwrite = false;
$requirelockingbeforewrite = false;
$requiresearchable = ($mode === cache_store::MODE_SESSION) ? true : false;
$maxsize = null;
$overrideclass = null;
@@ -395,7 +402,14 @@ class cache_definition {
if (array_key_exists('requirelockingwrite', $definition)) {
$requirelockingwrite = (bool)$definition['requirelockingwrite'];
}
$requirelocking = $requirelockingwrite || $requirelockingread;
if (array_key_exists('requirelockingbeforewrite', $definition)) {
$requirelockingbeforewrite = (bool)$definition['requirelockingbeforewrite'];
}
if ($requirelockingbeforewrite && ($requirelockingwrite || $requirelockingread)) {
throw new coding_exception('requirelockingbeforewrite cannot be set with requirelockingread or requirelockingwrite
in a cache definition, as this will result in conflicting locks.');
}
$requirelocking = $requirelockingwrite || $requirelockingbeforewrite || $requirelockingread;
if (array_key_exists('requiresearchable', $definition)) {
$requiresearchable = (bool)$definition['requiresearchable'];
@@ -523,6 +537,7 @@ class cache_definition {
$cachedefinition->requirelocking = $requirelocking;
$cachedefinition->requirelockingread = $requirelockingread;
$cachedefinition->requirelockingwrite = $requirelockingwrite;
$cachedefinition->requirelockingbeforewrite = $requirelockingbeforewrite;
$cachedefinition->requiresearchable = $requiresearchable;
$cachedefinition->maxsize = $maxsize;
$cachedefinition->overrideclass = $overrideclass;
@@ -741,6 +756,14 @@ class cache_definition {
return $this->requirelockingwrite;
}
/**
* Returns true if this definition requires a lock to be aquired before a write is attempted.
* @return bool
*/
public function require_locking_before_write() {
return $this->requirelockingbeforewrite;
}
/**
* Returns true if this definition allows local storage to be used for caching.
* @since Moodle 3.1.0
cache/classes/helper.php
Vendored
+2
View File
@@ -382,6 +382,7 @@ class cache_helper {
'misses' => 0,
'sets' => 0,
'iobytes' => cache_store::IO_BYTES_NOT_SUPPORTED,
'locks' => 0,
)
)
);
@@ -392,6 +393,7 @@ class cache_helper {
'misses' => 0,
'sets' => 0,
'iobytes' => cache_store::IO_BYTES_NOT_SUPPORTED,
'locks' => 0,
);
}
}
cache/classes/loaders.php
Vendored
+51 -4
View File
@@ -1581,12 +1581,24 @@ class cache_application extends cache implements cache_loader_with_locking {
*/
protected $requirelockingwrite = false;
/**
* Gets set to true if the cache writes (set|delete) must have a manual lock created first
* @var bool
*/
protected $requirelockingbeforewrite = false;
/**
* Gets set to a cache_store to use for locking if the caches primary store doesn't support locking natively.
* @var cache_lock_interface
*/
protected $cachelockinstance;
/**
* Store a list of locks acquired by this process.
* @var array
*/
protected $locks;
/**
* Overrides the cache construct method.
*
@@ -1603,6 +1615,7 @@ class cache_application extends cache implements cache_loader_with_locking {
$this->requirelocking = true;
$this->requirelockingread = $definition->require_locking_read();
$this->requirelockingwrite = $definition->require_locking_write();
$this->requirelockingbeforewrite = $definition->require_locking_before_write();
}
$this->handle_invalidation_events();
@@ -1648,13 +1661,24 @@ class cache_application extends cache implements cache_loader_with_locking {
* @return bool Returns true if the lock could be acquired, false otherwise.
*/
public function acquire_lock($key) {
global $CFG;
$key = $this->parse_key($key);
$before = microtime(true);
if ($this->nativelocking) {
return $this->get_store()->acquire_lock($key, $this->get_identifier());
$lock = $this->get_store()->acquire_lock($key, $this->get_identifier());
} else {
$this->ensure_cachelock_available();
return $this->cachelockinstance->lock($key, $this->get_identifier());
$lock = $this->cachelockinstance->lock($key, $this->get_identifier());
}
$after = microtime(true);
if ($lock) {
$this->locks[$key] = $lock;
if (defined('MDL_PERF') || !empty($CFG->perfdebug)) {
\core\lock\timing_wrapper_lock_factory::record_lock_data($after, $before,
$this->get_definition()->get_id(), $key, $lock, $this->get_identifier() . $key);
}
}
return $lock;
}
/**
@@ -1666,6 +1690,9 @@ class cache_application extends cache implements cache_loader_with_locking {
*/
public function check_lock_state($key) {
$key = $this->parse_key($key);
if (!empty($this->locks[$key])) {
return true; // Shortcut to save having to make a call to the cache store if the lock is held by this process.
}
if ($this->nativelocking) {
return $this->get_store()->check_lock_state($key, $this->get_identifier());
} else {
@@ -1683,11 +1710,18 @@ class cache_application extends cache implements cache_loader_with_locking {
public function release_lock($key) {
$key = $this->parse_key($key);
if ($this->nativelocking) {
return $this->get_store()->release_lock($key, $this->get_identifier());
$released = $this->get_store()->release_lock($key, $this->get_identifier());
} else {
$this->ensure_cachelock_available();
return $this->cachelockinstance->unlock($key, $this->get_identifier());
$released = $this->cachelockinstance->unlock($key, $this->get_identifier());
}
if ($released && array_key_exists($key, $this->locks)) {
unset($this->locks[$key]);
if (defined('MDL_PERF') || !empty($CFG->perfdebug)) {
\core\lock\timing_wrapper_lock_factory::record_lock_released_data($this->get_identifier() . $key);
}
}
return $released;
}
/**
@@ -1719,6 +1753,10 @@ class cache_application extends cache implements cache_loader_with_locking {
* @return bool True on success, false otherwise.
*/
protected function set_implementation($key, int $version, $data, bool $setparents = true): bool {
if ($this->requirelockingbeforewrite && !$this->check_lock_state($key)) {
throw new coding_exception('Attempted to set cache key "' . $key . '" without a lock. '
. 'Locking before writes is required for ' . $this->get_definition()->get_id());
}
if ($this->requirelockingwrite && !$this->acquire_lock($key)) {
return false;
}
@@ -1753,6 +1791,15 @@ class cache_application extends cache implements cache_loader_with_locking {
* ... if they care that is.
*/
public function set_many(array $keyvaluearray) {
if ($this->requirelockingbeforewrite) {
foreach ($keyvaluearray as $id => $pair) {
if (!$this->check_lock_state($pair['key'])) {
debugging('Attempted to set cache key "' . $pair['key'] . '" without a lock. '
. 'Locking before writes is required for ' . $this->get_definition()->get_name(), DEBUG_DEVELOPER);
unset($keyvaluearray[$id]);
}
}
}
if ($this->requirelockingwrite) {
$locks = array();
foreach ($keyvaluearray as $id => $pair) {
cache/disabledlib.php
Vendored
+20
View File
@@ -200,6 +200,26 @@ class cache_disabled extends cache {
public function purge() {
return true;
}
/**
* Pretend that we got a lock to avoid errors.
*
* @param string $key
* @return bool
*/
public function acquire_lock(string $key) : bool {
return true;
}
/**
* Pretend that we released a lock to avoid errors.
*
* @param string $key
* @return void
*/
public function release_lock(string $key) : bool {
return true;
}
}
/**
cache/stores/file/addinstanceform.php
Vendored
+5
View File
@@ -62,5 +62,10 @@ class cachestore_file_addinstance_form extends cachestore_addinstance_form {
$form->addElement('checkbox', 'asyncpurge', get_string('asyncpurge', 'cachestore_file'));
$form->setType('asyncpurge', PARAM_BOOL);
$form->addHelpButton('asyncpurge', 'asyncpurge', 'cachestore_file');
$form->addElement('text', 'lockwait', get_string('lockwait', 'cachestore_file'));
$form->setDefault('lockwait', 60);
$form->setType('lockwait', PARAM_INT);
$form->addHelpButton('lockwait', 'lockwait', 'cachestore_file');
}
}
cache/stores/file/lang/en/cachestore_file.php
Vendored
+2
View File
@@ -32,6 +32,8 @@ $string['asyncpurge'] = 'Asynchronously purge directory';
$string['asyncpurge_help'] = 'If enabled, the new directory is created with cache revision and the old directory will be deleted asynchronously via a scheduled task.';
$string['autocreate'] = 'Auto create directory';
$string['autocreate_help'] = 'If enabled the directory specified in path will be automatically created if it does not already exist.';
$string['lockwait'] = 'Maximum lock wait time';
$string['lockwait_help'] = 'The maximum amount of time in seconds to wait for an exclusive lock before reading or writing a cache key. This is only used for cache definitions that have read or write locking required.';
$string['path'] = 'Cache path';
$string['path_help'] = 'The directory that should be used to store files for this cache store. If left blank (default) a directory will be automatically created in the moodledata directory. This can be used to point a file store towards a directory on a better performing drive (such as one in memory).';
$string['pluginname'] = 'File cache';
cache/stores/file/lib.php
Vendored
+106 -1
View File
@@ -37,7 +37,8 @@
* @copyright 2012 Sam Hemelryk
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class cachestore_file extends cache_store implements cache_is_key_aware, cache_is_configurable, cache_is_searchable {
class cachestore_file extends cache_store implements cache_is_key_aware, cache_is_configurable, cache_is_searchable,
cache_is_lockable {
/**
* The name of the store.
@@ -128,6 +129,23 @@ class cachestore_file extends cache_store implements cache_is_key_aware, cache_i
*/
private $cfg = null;
/** @var int Maximum number of seconds to wait for a lock before giving up. */
protected $lockwait = 60;
/**
* Instance of file_lock_factory configured to create locks in the cache directory.
*
* @var \core\lock\file_lock_factory $lockfactory
*/
protected $lockfactory = null;
/**
* List of current locks.
*
* @var array $locks
*/
protected $locks = [];
/**
* Constructs the store instance.
*
@@ -193,6 +211,21 @@ class cachestore_file extends cache_store implements cache_is_key_aware, cache_i
} else {
$this->asyncpurge = false;
}
// Leverage cachelock_file to provide native locking, to avoid duplicating logic.
// This will store locks alongside the cache, so local cache uses local locks.
$lockdir = $path . '/filelocks';
if (!file_exists($lockdir)) {
make_writable_directory($lockdir);
}
if (array_key_exists('lockwait', $configuration)) {
$this->lockwait = (int)$configuration['lockwait'];
}
$this->lockfactory = new \core\lock\file_lock_factory('cachestore_file', $lockdir);
if (!$this->lockfactory->is_available()) {
// File locking is disabled in config, fall back to default lock factory.
$this->lockfactory = \core\lock\lock_config::get_lock_factory('cachestore_file');
}
}
/**
@@ -675,6 +708,9 @@ class cachestore_file extends cache_store implements cache_is_key_aware, cache_i
if (isset($data->asyncpurge)) {
$config['asyncpurge'] = $data->asyncpurge;
}
if (isset($data->lockwait)) {
$config['lockwait'] = $data->lockwait;
}
return $config;
}
@@ -702,6 +738,9 @@ class cachestore_file extends cache_store implements cache_is_key_aware, cache_i
if (isset($config['asyncpurge'])) {
$data['asyncpurge'] = (bool)$config['asyncpurge'];
}
if (isset($config['lockwait'])) {
$data['lockwait'] = (int)$config['lockwait'];
}
$editform->set_data($data);
}
@@ -924,4 +963,70 @@ class cachestore_file extends cache_store implements cache_is_key_aware, cache_i
$result->sd = sqrt($squarediff);
return $result;
}
/**
* Use lock factory to determine the lock state.
*
* @param string $key Lock identifier
* @param string $ownerid Cache identifier
* @return bool|null
*/
public function check_lock_state($key, $ownerid) : ?bool {
if (!array_key_exists($key, $this->locks)) {
return null; // Lock does not exist.
}
if (!array_key_exists($ownerid, $this->locks[$key])) {
return false; // Lock exists, but belongs to someone else.
}
if ($this->locks[$key][$ownerid] instanceof \core\lock\lock) {
return true; // Lock exists, and we own it.
}
// Try to get the lock with an immediate timeout. If this succeeds, the lock does not currently exist.
$lock = $this->lockfactory->get_lock($key, 0);
if ($lock) {
// Lock was not already held.
$lock->release();
return null;
} else {
// Lock is held by someone else.
return false;
}
}
/**
* Use lock factory to acquire a lock.
*
* @param string $key Lock identifier
* @param string $ownerid Cache identifier
* @return bool
* @throws cache_exception
*/
public function acquire_lock($key, $ownerid) : bool {
$lock = $this->lockfactory->get_lock($key, $this->lockwait);
if ($lock) {
$this->locks[$key][$ownerid] = $lock;
}
return (bool)$lock;
}
/**
* Use lock factory to release a lock.
*
* @param string $key Lock identifier
* @param string $ownerid Cache identifier
* @return bool
*/
public function release_lock($key, $ownerid) : bool {
if (!array_key_exists($key, $this->locks)) {
return false; // No lock to release.
}
if (!array_key_exists($ownerid, $this->locks[$key])) {
return false; // Tried to release someone else's lock.
}
$unlocked = $this->locks[$key][$ownerid]->release();
if ($unlocked) {
unset($this->locks[$key]);
}
return $unlocked;
}
}
cache/stores/file/tests/store_test.php
Vendored
+12
View File
@@ -33,6 +33,7 @@ require_once($CFG->dirroot.'/cache/stores/file/lib.php');
* @package cachestore_file
* @copyright 2013 Sam Hemelryk
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @covers \cachestore_file
*/
class store_test extends \cachestore_tests {
/**
@@ -99,4 +100,15 @@ class store_test extends \cachestore_tests {
]);
$this->assertEquals(21, $store->get_last_io_bytes());
}
public function test_lock() {
$store = new \cachestore_file('Test');
$this->assertTrue($store->acquire_lock('lock', '123'));
$this->assertTrue($store->check_lock_state('lock', '123'));
$this->assertFalse($store->check_lock_state('lock', '321'));
$this->assertNull($store->check_lock_state('notalock', '123'));
$this->assertFalse($store->release_lock('lock', '321'));
$this->assertTrue($store->release_lock('lock', '123'));
}
}
cache/tests/cache_test.php
Vendored
+49
View File
@@ -2111,6 +2111,55 @@ class cache_test extends \advanced_testcase {
$this->assertFalse($cache->has('a'));
}
/**
* Test requiring a lock before attempting to set a key.
*
* @covers ::set_implementation
*/
public function test_application_locking_before_write() {
$instance = cache_config_testing::instance(true);
$instance->phpunit_add_definition('phpunit/test_application_locking', array(
'mode' => cache_store::MODE_APPLICATION,
'component' => 'phpunit',
'area' => 'test_application_locking',
'staticacceleration' => true,
'staticaccelerationsize' => 1,
'requirelockingbeforewrite' => true
));
$cache = cache::make('phpunit', 'test_application_locking');
$this->assertInstanceOf(cache_application::class, $cache);
$cache->acquire_lock('a');
$this->assertTrue($cache->set('a', 'A'));
$cache->release_lock('a');
$this->expectExceptionMessage('Attempted to set cache key "b" without a lock. '
. 'Locking before writes is required for phpunit/test_application_locking');
$this->assertFalse($cache->set('b', 'B'));
}
/**
* Test that invalid lock setting combinations are caught.
*
* @covers ::make
*/
public function test_application_conflicting_locks() {
$instance = cache_config_testing::instance(true);
$instance->phpunit_add_definition('phpunit/test_application_locking', array(
'mode' => cache_store::MODE_APPLICATION,
'component' => 'phpunit',
'area' => 'test_application_locking',
'staticacceleration' => true,
'staticaccelerationsize' => 1,
'requirelockingwrite' => true,
'requirelockingbeforewrite' => true,
));
$this->expectException('coding_exception');
cache::make('phpunit', 'test_application_locking');
}
/**
* Test the static cache_helper method purge_stores_used_by_definition.
*/
cache/upgrade.txt
Vendored
+8
View File
@@ -1,6 +1,14 @@
This files describes API changes in /cache/stores/* - cache store plugins.
Information provided here is intended especially for developers.
=== 4.1 ===
* Added new `requirelockingbeforewrite` option for cache definitions. This will check that a lock for a given cache key already
exists before it will perform a `set()` on that key. A `coding_exception` is thrown if the lock has not been acquired.
* Added native locking to cachestore_file. This will use an instance of file_lock_factory pointing at a subdirectory in the same
location as the cache instance, meaning a local file cache will have its locks stored locally. If file locks are disabled
globally, it will fall back to use the default lock factory, which may not be in the same location as the cache. cachestore_file
includes an additional setting to control how long it will wait for a lock before giving up, default is 60 seconds.
=== 4.0 ===
* Cache stores may implement new optional function cache_store::get_last_io_bytes() to provide
information about the size of data transferred (shown in footer if performance info enabled).
lib/classes/lock/file_lock_factory.php
+6 -3
View File
@@ -58,12 +58,15 @@ class file_lock_factory implements lock_factory {
* Create this lock factory.
*
* @param string $type - The type, e.g. cron, cache, session
* @param string|null $lockdirectory - Optional path to the lock directory, to override defaults.
*/
public function __construct($type) {
public function __construct($type, ?string $lockdirectory = null) {
global $CFG;
$this->type = $type;
if (!isset($CFG->file_lock_root)) {
if (!is_null($lockdirectory)) {
$this->lockdirectory = $lockdirectory;
} else if (!isset($CFG->file_lock_root)) {
$this->lockdirectory = $CFG->dataroot . '/lock';
} else {
$this->lockdirectory = $CFG->file_lock_root;
@@ -100,7 +103,7 @@ class file_lock_factory implements lock_factory {
global $CFG;
$preventfilelocking = !empty($CFG->preventfilelocking);
$lockdirisdataroot = true;
if (!empty($CFG->file_lock_root) && strpos($CFG->file_lock_root, $CFG->dataroot) !== 0) {
if (strpos($this->lockdirectory, $CFG->dataroot) !== 0) {
$lockdirisdataroot = false;
}
return !$preventfilelocking || !$lockdirisdataroot;
lib/classes/lock/timing_wrapper_lock_factory.php
+43 -18
View File
@@ -70,40 +70,67 @@ class timing_wrapper_lock_factory implements lock_factory {
* @return \core\lock\lock|boolean - An instance of \core\lock\lock if the lock was obtained, or false.
*/
public function get_lock($resource, $timeout, $maxlifetime = 86400) {
global $PERF;
$before = microtime(true);
$result = $this->factory->get_lock($resource, $timeout, $maxlifetime);
$after = microtime(true);
$duration = $after - $before;
if (empty($PERF->locks)) {
$PERF->locks = [];
}
$lockdata = (object) [
'type' => $this->type,
'resource' => $resource,
'wait' => $duration,
'success' => (bool)$result
];
self::record_lock_data($after, $before, $this->type, $resource, (bool)$result, $result);
if ($result) {
$lockdata->lock = $result;
$lockdata->timestart = $after;
$result->init_factory($this);
}
$PERF->locks[] = $lockdata;
return $result;
}
/**
* Release a lock that was previously obtained with @lock.
* Records statistics about a lock to the performance data.
*
* @param float $after The time after the lock was achieved.
* @param float $before The time before the lock was requested.
* @param string $type The type of lock.
* @param string $resource The resource being locked.
* @param bool $result Whether the lock was successful.
* @param lock|string $lock A value uniquely identifying the lock.
* @return void
*/
public static function record_lock_data(float $after, float $before, string $type, string $resource, bool $result, $lock) {
global $PERF;
$duration = $after - $before;
if (empty($PERF->locks)) {
$PERF->locks = [];
}
$lockdata = (object) [
'type' => $type,
'resource' => $resource,
'wait' => $duration,
'success' => $result
];
if ($result) {
$lockdata->lock = $lock;
$lockdata->timestart = $after;
}
$PERF->locks[] = $lockdata;
}
/**
* Release a lock that was previously obtained with {@see get_lock}.
*
* @param lock $lock - The lock to release.
* @return boolean - True if the lock is no longer held (including if it was never held).
*/
public function release_lock(lock $lock) {
self::record_lock_released_data($lock);
return $this->factory->release_lock($lock);
}
/**
* Find the lock in the performance info and update it with the time held.
*
* @param lock|string $lock A value uniquely identifying the lock.
* @return void
*/
public static function record_lock_released_data($lock) {
global $PERF;
// Find this lock in the list of locks we got, looking backwards since it is probably
@@ -117,8 +144,6 @@ class timing_wrapper_lock_factory implements lock_factory {
break;
}
}
return $this->factory->release_lock($lock);
}
/**