MDL-41106 cache: several fixes for the session cache.

This issue makes several fixes for the session loader and the session store.
* maxsize argument now works for session caches.
* fixed performance hole when interation occurs frequently.
* fixed cache purge bug occuring when multiple caches are defined before being used.
* improved lastaccess handling.
This commit is contained in:
Sam Hemelryk
2013-08-16 11:27:55 +12:00
parent 838d78a9ff
commit 2e9d5a534e
3 changed files with 217 additions and 173 deletions
+124 -130
View File
@@ -858,7 +858,7 @@ class cache implements cache_loader {
* Returns the loader associated with this instance.
*
* @since 2.4.4
* @return cache_loader|false
* @return cache|false
*/
protected function get_loader() {
return $this->loader;
@@ -1458,6 +1458,7 @@ class cache_application extends cache implements cache_loader_with_locking {
* @todo we should support locking in the session as well. Should be pretty simple to set up.
*
* @internal don't use me directly.
* @method cache_store|cache_is_searchable get_store() Returns the cache store which must implement both cache_is_searchable.
*
* @package core
* @category cache
@@ -1494,6 +1495,11 @@ class cache_session extends cache {
*/
const KEY_PREFIX = 'sess_';
/**
* This is the key used to track last access.
*/
const LASTACCESS = 'lastaccess';
/**
* Override the cache::construct method.
*
@@ -1507,12 +1513,15 @@ class cache_session extends cache {
* @param cache_definition $definition
* @param cache_store $store
* @param cache_loader|cache_data_source $loader
* @return void
*/
public function __construct(cache_definition $definition, cache_store $store, $loader = null) {
// First up copy the loadeduserid to the current user id.
$this->currentuserid = self::$loadeduserid;
parent::__construct($definition, $store, $loader);
// This will trigger check tracked user. If this gets removed a call to that will need to be added here in its place.
$this->set(self::LASTACCESS, cache::now());
if ($definition->has_invalidation_events()) {
$lastinvalidation = $this->get('lastsessioninvalidation');
if ($lastinvalidation === false) {
@@ -1561,6 +1570,21 @@ class cache_session extends cache {
}
}
/**
* Sets the session id for the loader.
*/
protected function set_session_id() {
$this->sessionid = preg_replace('#[^a-zA-Z0-9_]#', '_', session_id());
}
/**
* Returns the prefix used for all keys.
* @return string
*/
protected function get_key_prefix() {
return 'u'.$this->currentuserid.'_'.$this->sessionid;
}
/**
* Parses the key turning it into a string (or array is required) suitable to be passed to the cache store.
*
@@ -1573,17 +1597,18 @@ class cache_session extends cache {
* @return string|array String unless the store supports multi-identifiers in which case an array if returned.
*/
protected function parse_key($key) {
if ($key === 'lastaccess') {
$key = '__lastaccess__';
$prefix = $this->get_key_prefix();
if ($key === self::LASTACCESS) {
return '__lastaccess__'.$prefix;
}
return 'sess_'.parent::parse_key($key);
return $prefix.'_'.parent::parse_key($key);
}
/**
* Check that this cache instance is tracking the current user.
*/
protected function check_tracked_user() {
if (isset($_SESSION['USER']->id)) {
if (isset($_SESSION['USER']->id) && $_SESSION['USER']->id !== null) {
// Get the id of the current user.
$new = $_SESSION['USER']->id;
} else {
@@ -1597,55 +1622,25 @@ class cache_session extends cache {
// This way we don't bloat the session.
$this->purge();
// Update the session id just in case!
$this->sessionid = session_id();
$this->set_session_id();
}
self::$loadeduserid = $new;
$this->currentuserid = $new;
} else if ($new !== $this->currentuserid) {
// The current user matches the loaded user but not the user last used by this cache.
$this->purge();
$this->purge_current_user();
$this->currentuserid = $new;
// Update the session id just in case!
$this->sessionid = session_id();
$this->set_session_id();
}
}
/**
* Gets the session data.
*
* @param bool $force If true the session data will be loaded from the store again.
* @return array An array of session data.
* Purges the session cache of all data belonging to the current user.
*/
protected function get_session_data($force = false) {
if ($this->sessionid === null) {
$this->sessionid = session_id();
}
if (is_array($this->session) && !$force) {
return $this->session;
}
$session = parent::get($this->sessionid);
if ($session === false) {
$session = array();
}
// We have to write here to ensure that the lastaccess time is recorded.
// And also in order to ensure the session entry exists as when we save it on __destruct
// $CFG is likely to have already been destroyed.
$this->save_session($session);
return $this->session;
}
/**
* Saves the session data.
*
* This function also updates the last access time.
*
* @param array $session
* @return bool
*/
protected function save_session(array $session) {
$session['lastaccess'] = time();
$this->session = $session;
return parent::set($this->sessionid, $this->session);
public function purge_current_user() {
$keys = $this->get_store()->find_all($this->get_key_prefix());
$this->get_store()->delete_many($keys);
}
/**
@@ -1664,10 +1659,8 @@ class cache_session extends cache {
// 2. Parse the key.
$parsedkey = $this->parse_key($key);
// 3. Get it from the store.
$result = false;
$session = $this->get_session_data();
if (array_key_exists($parsedkey, $session)) {
$result = $session[$parsedkey];
$result = $this->get_store()->get($parsedkey);
if ($result !== false) {
if ($result instanceof cache_ttl_wrapper) {
if ($result->has_expired()) {
$this->get_store()->delete($parsedkey);
@@ -1681,10 +1674,9 @@ class cache_session extends cache {
}
}
// 4. Load if from the loader/datasource if we don't already have it.
$setaftervalidation = false;
if ($result === false) {
if ($this->perfdebug) {
cache_helper::record_cache_miss('**static session**', $this->get_definition()->get_id());
cache_helper::record_cache_miss($this->storetype, $this->get_definition()->get_id());
}
if ($this->get_loader() !== false) {
// We must pass the original (unparsed) key to the next loader in the chain.
@@ -1694,19 +1686,18 @@ class cache_session extends cache {
} else if ($this->get_datasource() !== false) {
$result = $this->get_datasource()->load_for_cache($key);
}
$setaftervalidation = ($result !== false);
// 5. Set it to the store if we got it from the loader/datasource.
if ($result !== false) {
$this->set($key, $result);
}
} else if ($this->perfdebug) {
cache_helper::record_cache_hit('**static session**', $this->get_definition()->get_id());
cache_helper::record_cache_hit($this->storetype, $this->get_definition()->get_id());
}
// 5. Validate strictness.
if ($strictness === MUST_EXIST && $result === false) {
throw new moodle_exception('Requested key did not exist in any cache stores and could not be loaded.');
}
// 6. Set it to the store if we got it from the loader/datasource.
if ($setaftervalidation) {
$this->set($key, $result);
}
// 7. Make sure we don't pass back anything that could be a reference.
// 6. Make sure we don't pass back anything that could be a reference.
// We don't want people modifying the data in the cache.
if (!is_scalar($result)) {
// If data is an object it will be a reference.
@@ -1738,7 +1729,7 @@ class cache_session extends cache {
public function set($key, $data) {
$this->check_tracked_user();
if ($this->perfdebug) {
cache_helper::record_cache_set('**static session**', $this->get_definition()->get_id());
cache_helper::record_cache_set($this->storetype, $this->get_definition()->get_id());
}
if (is_object($data) && $data instanceof cacheable_object) {
$data = new cache_cached_object($data);
@@ -1750,12 +1741,10 @@ class cache_session extends cache {
$data = $this->unref($data);
}
// We dont' support native TTL here as we consolidate data for sessions.
if ($this->has_a_ttl()) {
if ($this->has_a_ttl() && !$this->store_supports_native_ttl()) {
$data = new cache_ttl_wrapper($data, $this->get_definition()->get_ttl());
}
$session = $this->get_session_data();
$session[$this->parse_key($key)] = $data;
return $this->save_session($session);
return $this->get_store()->set($this->parse_key($key), $data);
}
/**
@@ -1767,15 +1756,12 @@ class cache_session extends cache {
* @return bool True of success, false otherwise.
*/
public function delete($key, $recurse = true) {
$this->check_tracked_user();
$parsedkey = $this->parse_key($key);
if ($recurse && $this->get_loader() !== false) {
// Delete from the bottom of the stack first.
$this->get_loader()->delete($key, $recurse);
}
$session = $this->get_session_data();
unset($session[$parsedkey]);
return $this->save_session($session);
return $this->get_store()->delete($parsedkey);
}
/**
@@ -1798,11 +1784,37 @@ class cache_session extends cache {
*/
public function get_many(array $keys, $strictness = IGNORE_MISSING) {
$this->check_tracked_user();
$return = array();
$parsedkeys = array();
$keymap = array();
foreach ($keys as $key) {
$return[$key] = $this->get($key, $strictness);
$parsedkey = $this->parse_key($key);
$parsedkeys[] = $parsedkey;
$keymap[$parsedkey] = $key;
}
$result = $this->get_store()->get_many($parsedkeys);
$mustexist = $strictness === MUST_EXIST;
$return = array();
foreach ($result as $parsedkey => $value) {
if ($value instanceof cache_ttl_wrapper) {
/* @var cache_ttl_wrapper $value */
if ($value->has_expired()) {
$this->get_store()->delete($parsedkey);
$value = false;
} else {
$value = $value->data;
}
}
if ($value instanceof cache_cached_object) {
/* @var cache_cached_object $value */
$value = $value->restore_object();
}
if ($value === false && $mustexist) {
throw new moodle_exception('Not all the requested keys existed within the cache stores.');
}
$return[$keymap[$parsedkey]] = $value;
}
return $return;
}
/**
@@ -1814,18 +1826,12 @@ class cache_session extends cache {
* @return int The number of items successfully deleted.
*/
public function delete_many(array $keys, $recurse = true) {
$this->check_tracked_user();
$parsedkeys = array_map(array($this, 'parse_key'), $keys);
if ($recurse && $this->get_loader() !== false) {
// Delete from the bottom of the stack first.
$this->get_loader()->delete_many($keys, $recurse);
}
$session = $this->get_session_data();
foreach ($parsedkeys as $parsedkey) {
unset($session[$parsedkey]);
}
$this->save_session($session);
return count($keys);
return $this->get_store()->delete_many($parsedkeys);
}
/**
@@ -1853,8 +1859,9 @@ class cache_session extends cache {
*/
public function set_many(array $keyvaluearray) {
$this->check_tracked_user();
$session = $this->get_session_data();
$simulatettl = $this->has_a_ttl();
$data = array();
$definitionid = $this->get_definition()->get_ttl();
$simulatettl = $this->has_a_ttl() && !$this->store_supports_native_ttl();
foreach ($keyvaluearray as $key => $value) {
if (is_object($value) && $value instanceof cacheable_object) {
$value = new cache_cached_object($value);
@@ -1866,16 +1873,17 @@ class cache_session extends cache {
$value = $this->unref($value);
}
if ($simulatettl) {
$value = new cache_ttl_wrapper($value, $this->get_definition()->get_ttl());
$value = new cache_ttl_wrapper($value, $definitionid);
}
$parsedkey = $this->parse_key($key);
$session[$parsedkey] = $value;
$data[$key] = array(
'key' => $this->parse_key($key),
'value' => $value
);
}
if ($this->perfdebug) {
cache_helper::record_cache_set($this->storetype, $this->get_definition()->get_id());
cache_helper::record_cache_set($this->storetype, $definitionid);
}
$this->save_session($session);
return count($keyvaluearray);
return $this->get_store()->set_many($data);
}
/**
@@ -1884,13 +1892,9 @@ class cache_session extends cache {
* @return bool True on success, false otherwise
*/
public function purge() {
// 1. Purge the session object.
$this->session = array();
// 2. Delete the record for this users session from the store.
$this->get_store()->delete($this->sessionid);
// 3. Optionally purge any stacked loaders in the same way.
$this->get_store()->purge();
if ($this->get_loader()) {
$this->get_loader()->delete($this->sessionid);
$this->get_loader()->purge();
}
return true;
}
@@ -1919,21 +1923,27 @@ class cache_session extends cache {
public function has($key, $tryloadifpossible = false) {
$this->check_tracked_user();
$parsedkey = $this->parse_key($key);
$session = $this->get_session_data();
$has = false;
if ($this->has_a_ttl()) {
$store = $this->get_store();
if ($this->has_a_ttl() && !$this->store_supports_native_ttl()) {
// The data has a TTL and the store doesn't support it natively.
// We must fetch the data and expect a ttl wrapper.
if (array_key_exists($parsedkey, $session)) {
$data = $session[$parsedkey];
$has = ($data instanceof cache_ttl_wrapper && !$data->has_expired());
}
$data = $store->get($parsedkey);
$has = ($data instanceof cache_ttl_wrapper && !$data->has_expired());
} else if (!$this->store_supports_key_awareness()) {
// The store doesn't support key awareness, get the data and check it manually... puke.
// Either no TTL is set of the store supports its handling natively.
$data = $store->get($parsedkey);
$has = ($data !== false);
} else {
$has = array_key_exists($parsedkey, $session);
// The store supports key awareness, this is easy!
// Either no TTL is set of the store supports its handling natively.
/* @var cache_store|cache_is_key_aware $store */
$has = $store->has($parsedkey);
}
if (!$has && $tryloadifpossible) {
$result = null;
if ($this->get_loader() !== false) {
$result = $this->get_loader()->get($key);
$result = $this->get_loader()->get($parsedkey);
} else if ($this->get_datasource() !== null) {
$result = $this->get_datasource()->load_for_cache($key);
}
@@ -1960,25 +1970,18 @@ class cache_session extends cache {
*/
public function has_all(array $keys) {
$this->check_tracked_user();
$session = $this->get_session_data();
foreach ($keys as $key) {
$has = false;
$parsedkey = $this->parse_key($key);
if ($this->has_a_ttl()) {
// The data has a TTL and the store doesn't support it natively.
// We must fetch the data and expect a ttl wrapper.
if (array_key_exists($parsedkey, $session)) {
$data = $session[$parsedkey];
$has = ($data instanceof cache_ttl_wrapper && !$data->has_expired());
if (($this->has_a_ttl() && !$this->store_supports_native_ttl()) || !$this->store_supports_key_awareness()) {
foreach ($keys as $key) {
if (!$this->has($key)) {
return false;
}
} else {
$has = array_key_exists($parsedkey, $session);
}
if (!$has) {
return false;
}
return true;
}
return true;
// The cache must be key aware and if support native ttl if it a ttl is set.
/* @var cache_store|cache_is_key_aware $store */
$store = $this->get_store();
return $store->has_all(array_map(array($this, 'parse_key'), $keys));
}
/**
@@ -1995,26 +1998,17 @@ class cache_session extends cache {
* @return bool True if the cache has at least one of the given keys
*/
public function has_any(array $keys) {
$this->check_tracked_user();
$session = $this->get_session_data();
foreach ($keys as $key) {
$has = false;
$parsedkey = $this->parse_key($key);
if ($this->has_a_ttl()) {
// The data has a TTL and the store doesn't support it natively.
// We must fetch the data and expect a ttl wrapper.
if (array_key_exists($parsedkey, $session)) {
$data = $session[$parsedkey];
$has = ($data instanceof cache_ttl_wrapper && !$data->has_expired());
if (($this->has_a_ttl() && !$this->store_supports_native_ttl()) || !$this->store_supports_key_awareness()) {
foreach ($keys as $key) {
if ($this->has($key)) {
return true;
}
} else {
$has = array_key_exists($parsedkey, $session);
}
if ($has) {
return true;
}
return false;
}
return false;
/* @var cache_store|cache_is_key_aware $store */
$store = $this->get_store();
return $store->has_any(array_map(array($this, 'parse_key'), $keys));
}
/**
+20 -8
View File
@@ -237,7 +237,7 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
*/
public function get($key) {
if (isset($this->store[$key])) {
if ($this->ttl == 0) {
if ($this->ttl === 0) {
return $this->store[$key][0];
} else if ($this->store[$key][1] >= (cache::now() - $this->ttl)) {
return $this->store[$key][0];
@@ -257,6 +257,7 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
*/
public function get_many($keys) {
$return = array();
$maxtime = 0;
if ($this->ttl != 0) {
$maxtime = cache::now() - $this->ttl;
}
@@ -284,11 +285,9 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
*/
public function set($key, $data, $testmaxsize = true) {
$testmaxsize = ($testmaxsize && $this->maxsize !== false);
if ($testmaxsize) {
$increment = (!isset($this->store[$key]));
}
if ($this->ttl == 0) {
$this->store[$key][0] = $data;
$increment = ($testmaxsize && !isset($this->store[$key]));
if ($this->ttl === 0) {
$this->store[$key] = array($data, 0);
} else {
$this->store[$key] = array($data, cache::now());
}
@@ -311,12 +310,22 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
*/
public function set_many(array $keyvaluearray) {
$count = 0;
$increment = 0;
foreach ($keyvaluearray as $pair) {
$this->set($pair['key'], $pair['value'], false);
$key = $pair['key'];
$data = $pair['value'];
$count++;
if (!isset($this->store[$key])) {
$increment++;
}
if ($this->ttl === 0) {
$this->store[$key] = array($data, 0);
} else {
$this->store[$key] = array($data, cache::now());
}
}
if ($this->maxsize !== false) {
$this->storecount += $count;
$this->storecount += $increment;
if ($this->storecount > $this->maxsize) {
$this->reduce_for_maxsize();
}
@@ -348,6 +357,7 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
* @return bool
*/
public function has_all(array $keys) {
$maxtime = 0;
if ($this->ttl != 0) {
$maxtime = cache::now() - $this->ttl;
}
@@ -370,6 +380,7 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
* @return bool
*/
public function has_any(array $keys) {
$maxtime = 0;
if ($this->ttl != 0) {
$maxtime = cache::now() - $this->ttl;
}
@@ -502,6 +513,7 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
* Finds all of the keys whose keys start with the given prefix.
*
* @param string $prefix
* @return array An array of keys.
*/
public function find_by_prefix($prefix) {
$return = array();
+73 -35
View File
@@ -49,60 +49,66 @@ class cachestore_session_test extends cachestore_tests {
* Test the maxsize option.
*/
public function test_maxsize() {
$defid = 'phpunit/testmaxsize';
$config = cache_config_phpunittest::instance();
$config->phpunit_add_definition($defid, array(
$config->phpunit_add_definition('phpunit/one', array(
'mode' => cache_store::MODE_SESSION,
'component' => 'phpunit',
'area' => 'testmaxsize',
'area' => 'one',
'maxsize' => 3
));
$definition = cache_definition::load($defid, $config->get_definition_by_id($defid));
$instance = cachestore_session::initialise_test_instance($definition);
$this->assertTrue($instance->set('key1', 'value1'));
$this->assertTrue($instance->set('key2', 'value2'));
$this->assertTrue($instance->set('key3', 'value3'));
$config->phpunit_add_definition('phpunit/two', array(
'mode' => cache_store::MODE_SESSION,
'component' => 'phpunit',
'area' => 'two',
'maxsize' => 3
));
$this->assertTrue($instance->has('key1'));
$this->assertTrue($instance->has('key2'));
$this->assertTrue($instance->has('key3'));
$cacheone = cache::make('phpunit', 'one');
$this->assertTrue($instance->set('key4', 'value4'));
$this->assertTrue($instance->set('key5', 'value5'));
$this->assertTrue($cacheone->set('key1', 'value1'));
$this->assertTrue($cacheone->set('key2', 'value2'));
$this->assertTrue($cacheone->set('key3', 'value3'));
$this->assertFalse($instance->has('key1'));
$this->assertFalse($instance->has('key2'));
$this->assertTrue($instance->has('key3'));
$this->assertTrue($instance->has('key4'));
$this->assertTrue($instance->has('key5'));
$this->assertTrue($cacheone->has('key1'));
$this->assertTrue($cacheone->has('key2'));
$this->assertTrue($cacheone->has('key3'));
$this->assertFalse($instance->get('key1'));
$this->assertFalse($instance->get('key2'));
$this->assertEquals('value3', $instance->get('key3'));
$this->assertEquals('value4', $instance->get('key4'));
$this->assertEquals('value5', $instance->get('key5'));
$this->assertTrue($cacheone->set('key4', 'value4'));
$this->assertTrue($cacheone->set('key5', 'value5'));
$this->assertFalse($cacheone->has('key1'));
$this->assertFalse($cacheone->has('key2'));
$this->assertTrue($cacheone->has('key3'));
$this->assertTrue($cacheone->has('key4'));
$this->assertTrue($cacheone->has('key5'));
$this->assertFalse($cacheone->get('key1'));
$this->assertFalse($cacheone->get('key2'));
$this->assertEquals('value3', $cacheone->get('key3'));
$this->assertEquals('value4', $cacheone->get('key4'));
$this->assertEquals('value5', $cacheone->get('key5'));
// Test adding one more.
$this->assertTrue($instance->set('key6', 'value6'));
$this->assertFalse($instance->get('key3'));
$this->assertTrue($cacheone->set('key6', 'value6'));
$this->assertFalse($cacheone->get('key3'));
// Test reducing and then adding to make sure we don't lost one.
$this->assertTrue($instance->delete('key6'));
$this->assertTrue($instance->set('key7', 'value7'));
$this->assertEquals('value4', $instance->get('key4'));
$this->assertTrue($cacheone->delete('key6'));
$this->assertTrue($cacheone->set('key7', 'value7'));
$this->assertEquals('value4', $cacheone->get('key4'));
// Set the same key three times to make sure it doesn't count overrides.
for ($i = 0; $i < 3; $i++) {
$this->assertTrue($instance->set('key8', 'value8'));
$this->assertTrue($cacheone->set('key8', 'value8'));
}
$this->assertEquals('value7', $instance->get('key7'), 'Overrides are incorrectly incrementing size');
$this->assertEquals('value7', $cacheone->get('key7'), 'Overrides are incorrectly incrementing size');
// Test adding many.
$this->assertEquals(3, $instance->set_many(array(
array('key' => 'keyA', 'value' => 'valueA'),
array('key' => 'keyB', 'value' => 'valueB'),
array('key' => 'keyC', 'value' => 'valueC')
$this->assertEquals(3, $cacheone->set_many(array(
'keyA' => 'valueA',
'keyB' => 'valueB',
'keyC' => 'valueC'
)));
$this->assertEquals(array(
'key4' => false,
@@ -112,8 +118,40 @@ class cachestore_session_test extends cachestore_tests {
'keyA' => 'valueA',
'keyB' => 'valueB',
'keyC' => 'valueC'
), $instance->get_many(array(
), $cacheone->get_many(array(
'key4', 'key5', 'key6', 'key7', 'keyA', 'keyB', 'keyC'
)));
$cachetwo = cache::make('phpunit', 'two');
// Test adding many.
$this->assertEquals(3, $cacheone->set_many(array(
'keyA' => 'valueA',
'keyB' => 'valueB',
'keyC' => 'valueC'
)));
$this->assertEquals(3, $cachetwo->set_many(array(
'key1' => 'value1',
'key2' => 'value2',
'key3' => 'value3'
)));
$this->assertEquals(array(
'keyA' => 'valueA',
'keyB' => 'valueB',
'keyC' => 'valueC'
), $cacheone->get_many(array(
'keyA', 'keyB', 'keyC'
)));
$this->assertEquals(array(
'key1' => 'value1',
'key2' => 'value2',
'key3' => 'value3'
), $cachetwo->get_many(array(
'key1', 'key2', 'key3'
)));
}
}