From fcb3f1de83c13fa931f3b9b22d109fe6653966de Mon Sep 17 00:00:00 2001 From: Benjamin Walker Date: Mon, 2 Jun 2025 11:59:03 +1000 Subject: [PATCH 1/4] MDL-85336 cachestore_redis: Support floats in connection timeout --- public/cache/stores/redis/addinstanceform.php | 2 +- public/cache/stores/redis/lib.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/public/cache/stores/redis/addinstanceform.php b/public/cache/stores/redis/addinstanceform.php index e775bbcd0db..9cd816415fa 100644 --- a/public/cache/stores/redis/addinstanceform.php +++ b/public/cache/stores/redis/addinstanceform.php @@ -69,6 +69,6 @@ class cachestore_redis_addinstance_form extends cachestore_addinstance_form { $form->addElement('text', 'connectiontimeout', get_string('connectiontimeout', 'cachestore_redis')); $form->addHelpButton('connectiontimeout', 'connectiontimeout', 'cachestore_redis'); $form->setDefault('connectiontimeout', cachestore_redis::CONNECTION_TIMEOUT); - $form->setType('connectiontimeout', PARAM_INT); + $form->setType('connectiontimeout', PARAM_FLOAT); } } diff --git a/public/cache/stores/redis/lib.php b/public/cache/stores/redis/lib.php index 023ac6fae1e..178a35ec008 100644 --- a/public/cache/stores/redis/lib.php +++ b/public/cache/stores/redis/lib.php @@ -68,8 +68,8 @@ class cachestore_redis extends store implements */ const TTL_EXPIRE_BATCH = 10000; - /** @var int The number of seconds to wait for a connection or response from the Redis server. */ - const CONNECTION_TIMEOUT = 3; + /** @var float The number of seconds to wait for a connection or response from the Redis server. */ + const CONNECTION_TIMEOUT = 3.0; /** * Name of this store. @@ -124,7 +124,7 @@ class cachestore_redis extends store implements /** * The number of seconds to wait for a connection or response from the Redis server. * - * @var int + * @var float */ protected $connectiontimeout = self::CONNECTION_TIMEOUT; @@ -212,7 +212,7 @@ class cachestore_redis extends store implements $this->compressor = (int)$configuration['compressor']; } if (array_key_exists('connectiontimeout', $configuration)) { - $this->connectiontimeout = (int)$configuration['connectiontimeout']; + $this->connectiontimeout = (float)$configuration['connectiontimeout']; } if (array_key_exists('lockwait', $configuration)) { $this->lockwait = (int)$configuration['lockwait']; From d018536312d7b0da589b69ed992eda067541d643 Mon Sep 17 00:00:00 2001 From: Benjamin Walker Date: Mon, 2 Jun 2025 12:00:51 +1000 Subject: [PATCH 2/4] MDL-85336 redis: Support floats in connection timeout --- config-dist.php | 2 +- public/lib/classes/session/redis.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/config-dist.php b/config-dist.php index 4bf70da9b32..4acd504a306 100644 --- a/config-dist.php +++ b/config-dist.php @@ -380,7 +380,7 @@ $CFG->admin = 'admin'; // $CFG->session_redis_lock_expire = 7200; // Optional, defaults to session timeout. // $CFG->session_redis_lock_retry = 100; // Optional wait between lock attempts in ms, default is 100. // // After 5 seconds it will throttle down to once per second. -// $CFG->session_redis_connection_timeout = 3; // Optional, default is 3. +// $CFG->session_redis_connection_timeout = 3.0; // Optional, default is 3.0. // $CFG->session_redis_maxretries = 3; // Optional, default is 3. // // Use the igbinary serializer instead of the php default one. Note that phpredis must be compiled with diff --git a/public/lib/classes/session/redis.php b/public/lib/classes/session/redis.php index 81ab0e15b08..0713c352b28 100644 --- a/public/lib/classes/session/redis.php +++ b/public/lib/classes/session/redis.php @@ -114,8 +114,8 @@ class redis extends handler implements SessionHandlerInterface { /** @var clock A clock instance */ protected clock $clock; - /** @var int $connectiontimeout The number of seconds to wait for a connection or response from the Redis server. */ - protected int $connectiontimeout = 3; + /** @var float $connectiontimeout The number of seconds to wait for a connection or response from the Redis server. */ + protected float $connectiontimeout = 3.0; /** * Create new instance of handler. @@ -205,7 +205,7 @@ class redis extends handler implements SessionHandlerInterface { } if (isset($CFG->session_redis_connection_timeout)) { - $this->connectiontimeout = (int)$CFG->session_redis_connection_timeout; + $this->connectiontimeout = (float)$CFG->session_redis_connection_timeout; } if (isset($CFG->session_redis_max_retries)) { From 989b236bc8970c56b73b03219333b52636efe75e Mon Sep 17 00:00:00 2001 From: Benjamin Walker Date: Mon, 2 Jun 2025 12:02:55 +1000 Subject: [PATCH 3/4] MDL-85336 cachestore_redis: Split connection and read timeouts --- public/cache/stores/redis/addinstanceform.php | 5 ++++ .../stores/redis/lang/en/cachestore_redis.php | 2 ++ public/cache/stores/redis/lib.php | 24 +++++++++++++++---- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/public/cache/stores/redis/addinstanceform.php b/public/cache/stores/redis/addinstanceform.php index 9cd816415fa..1a86319bb8e 100644 --- a/public/cache/stores/redis/addinstanceform.php +++ b/public/cache/stores/redis/addinstanceform.php @@ -70,5 +70,10 @@ class cachestore_redis_addinstance_form extends cachestore_addinstance_form { $form->addHelpButton('connectiontimeout', 'connectiontimeout', 'cachestore_redis'); $form->setDefault('connectiontimeout', cachestore_redis::CONNECTION_TIMEOUT); $form->setType('connectiontimeout', PARAM_FLOAT); + + $form->addElement('text', 'readtimeout', get_string('readtimeout', 'cachestore_redis')); + $form->addHelpButton('readtimeout', 'readtimeout', 'cachestore_redis'); + $form->setDefault('readtimeout', cachestore_redis::CONNECTION_TIMEOUT); + $form->setType('readtimeout', PARAM_FLOAT); } } diff --git a/public/cache/stores/redis/lang/en/cachestore_redis.php b/public/cache/stores/redis/lang/en/cachestore_redis.php index c3332562c8f..fdc4471623b 100644 --- a/public/cache/stores/redis/lang/en/cachestore_redis.php +++ b/public/cache/stores/redis/lang/en/cachestore_redis.php @@ -46,6 +46,8 @@ $string['prefix_help'] = 'This prefix is used for all key names on the Redis ser $string['prefixinvalid'] = 'Invalid prefix. You can only use a-z A-Z 0-9-_.'; $string['privacy:metadata:redis'] = 'The Redis cachestore plugin stores data briefly as part of its caching functionality. This data is stored on an Redis server where data is regularly removed.'; $string['privacy:metadata:redis:data'] = 'The various data stored in the cache'; +$string['readtimeout'] = 'Read timeout'; +$string['readtimeout_help'] = 'This sets the number of seconds to wait for a read from the Redis server.'; $string['serializer_igbinary'] = 'Igbinary serializer'; $string['serializer_php'] = 'Default PHP serializer'; $string['server'] = 'Server(s)'; diff --git a/public/cache/stores/redis/lib.php b/public/cache/stores/redis/lib.php index 178a35ec008..4fc318b256f 100644 --- a/public/cache/stores/redis/lib.php +++ b/public/cache/stores/redis/lib.php @@ -122,12 +122,19 @@ class cachestore_redis extends store implements /** - * The number of seconds to wait for a connection or response from the Redis server. + * The number of seconds to wait for a connection response from the Redis server. * * @var float */ protected $connectiontimeout = self::CONNECTION_TIMEOUT; + /** + * The number of seconds to wait for a read from the Redis server. + * + * @var float + */ + protected $readtimeout = self::CONNECTION_TIMEOUT; + /** * Bytes read or written by last call to set()/get() or set_many()/get_many(). * @@ -214,6 +221,9 @@ class cachestore_redis extends store implements if (array_key_exists('connectiontimeout', $configuration)) { $this->connectiontimeout = (float)$configuration['connectiontimeout']; } + if (array_key_exists('readtimeout', $configuration)) { + $this->readtimeout = (float)$configuration['readtimeout']; + } if (array_key_exists('lockwait', $configuration)) { $this->lockwait = (int)$configuration['lockwait']; } @@ -296,7 +306,7 @@ class cachestore_redis extends store implements name: null, seeds: $trimmedservers, timeout: $this->connectiontimeout, // Timeout. - read_timeout: $this->connectiontimeout, // Read timeout. + read_timeout: $this->readtimeout, // Read timeout. persistent: true, auth: $password, context: !empty($opts) ? $opts : null, @@ -306,7 +316,7 @@ class cachestore_redis extends store implements null, $trimmedservers, $this->connectiontimeout, - $this->connectiontimeout, + $this->readtimeout, true, $password, !empty($opts) ? $opts : null, ); @@ -320,7 +330,7 @@ class cachestore_redis extends store implements port: $port, timeout: $this->connectiontimeout, // Timeout. retry_interval: 100, // Retry interval. - read_timeout: $this->connectiontimeout, // Read timeout. + read_timeout: $this->readtimeout, // Read timeout. context: $opts, ); } else { @@ -329,7 +339,7 @@ class cachestore_redis extends store implements $this->connectiontimeout, null, 100, - $this->connectiontimeout, + $this->readtimeout, $opts, ); } @@ -912,6 +922,7 @@ class cachestore_redis extends store implements 'serializer' => $data->serializer, 'compressor' => $data->compressor, 'connectiontimeout' => $data->connectiontimeout, + 'readtimeout' => $data->readtimeout, 'encryption' => $data->encryption, 'cafile' => $data->cafile, 'clustermode' => $data->clustermode, @@ -939,6 +950,9 @@ class cachestore_redis extends store implements if (!empty($config['connectiontimeout'])) { $data['connectiontimeout'] = $config['connectiontimeout']; } + if (!empty($config['readtimeout'])) { + $data['readtimeout'] = $config['readtimeout']; + } if (!empty($config['encryption'])) { $data['encryption'] = $config['encryption']; } From 838ec4527c4a1e39aea733b2aa664a77f752e16c Mon Sep 17 00:00:00 2001 From: Benjamin Walker Date: Mon, 2 Jun 2025 12:03:17 +1000 Subject: [PATCH 4/4] MDL-85336 redis: Split connection and read timeouts --- .upgradenotes/MDL-85336-2025112802512668.yml | 8 ++++++++ config-dist.php | 1 + public/lib/classes/session/redis.php | 17 ++++++++++++----- 3 files changed, 21 insertions(+), 5 deletions(-) create mode 100644 .upgradenotes/MDL-85336-2025112802512668.yml diff --git a/.upgradenotes/MDL-85336-2025112802512668.yml b/.upgradenotes/MDL-85336-2025112802512668.yml new file mode 100644 index 00000000000..20584faecbb --- /dev/null +++ b/.upgradenotes/MDL-85336-2025112802512668.yml @@ -0,0 +1,8 @@ +issueNumber: MDL-85336 +notes: + core: + - message: >- + Redis connection timeout settings for cachestores and sessions have been + split into connection timeout and read timeout to allow for finer + control. These settings now also accept floats. + type: improved diff --git a/config-dist.php b/config-dist.php index 4acd504a306..eddef3902d5 100644 --- a/config-dist.php +++ b/config-dist.php @@ -382,6 +382,7 @@ $CFG->admin = 'admin'; // // After 5 seconds it will throttle down to once per second. // $CFG->session_redis_connection_timeout = 3.0; // Optional, default is 3.0. // $CFG->session_redis_maxretries = 3; // Optional, default is 3. +// $CFG->session_redis_read_timeout = 3.0; // Optional, default is 3.0. // // Use the igbinary serializer instead of the php default one. Note that phpredis must be compiled with // igbinary support to make the setting to work. Also, if you change the serializer you have to flush the database! diff --git a/public/lib/classes/session/redis.php b/public/lib/classes/session/redis.php index 0713c352b28..ec9ef56f654 100644 --- a/public/lib/classes/session/redis.php +++ b/public/lib/classes/session/redis.php @@ -114,9 +114,12 @@ class redis extends handler implements SessionHandlerInterface { /** @var clock A clock instance */ protected clock $clock; - /** @var float $connectiontimeout The number of seconds to wait for a connection or response from the Redis server. */ + /** @var float $connectiontimeout The number of seconds to wait for a connection response from the Redis server. */ protected float $connectiontimeout = 3.0; + /** @var float $readtimeout The number of seconds to wait for a read from the Redis server. */ + protected float $readtimeout = 3.0; + /** * Create new instance of handler. */ @@ -212,6 +215,10 @@ class redis extends handler implements SessionHandlerInterface { $this->maxretries = (int)$CFG->session_redis_max_retries; } + if (isset($CFG->session_redis_read_timeout)) { + $this->readtimeout = (float)$CFG->session_redis_read_timeout; + } + $this->clock = di::get(clock::class); } @@ -316,7 +323,7 @@ class redis extends handler implements SessionHandlerInterface { name: null, seeds: $trimmedservers, timeout: $this->connectiontimeout, // Timeout. - read_timeout: $this->connectiontimeout, // Read timeout. + read_timeout: $this->readtimeout, // Read timeout. persistent: true, auth: $this->auth, context: !empty($opts) ? $opts : null, @@ -326,7 +333,7 @@ class redis extends handler implements SessionHandlerInterface { null, $trimmedservers, $this->connectiontimeout, - $this->connectiontimeout, + $this->readtimeout, true, $this->auth, !empty($opts) ? $opts : null @@ -342,7 +349,7 @@ class redis extends handler implements SessionHandlerInterface { port: $port, timeout: $this->connectiontimeout, // Timeout. retry_interval: $delay, - read_timeout: $this->connectiontimeout, // Read timeout. + read_timeout: $this->readtimeout, // Read timeout. context: $opts, ); } else { @@ -352,7 +359,7 @@ class redis extends handler implements SessionHandlerInterface { $this->connectiontimeout, null, $delay, - $this->connectiontimeout, + $this->readtimeout, $opts ); }