Skip to content

Commit

Permalink
MDL-83753 redis: Allow for configurable max retries setting
Browse files Browse the repository at this point in the history
  • Loading branch information
djarran committed Dec 19, 2024
1 parent 90fcb57 commit 2a28dbd
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 6 deletions.
1 change: 1 addition & 0 deletions config-dist.php
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@
// $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_maxretries = 3; // Optional, default is 3.
//
// 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!
Expand Down
12 changes: 8 additions & 4 deletions lib/classes/session/redis.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class redis extends handler implements SessionHandlerInterface {
protected bool $clustermode = false;

/** @var int Maximum number of retries for cache store operations. */
const MAX_RETRIES = 5;
protected int $maxretries = 3;

/** @var int $firstaccesstimeout The initial timeout (seconds) for the first browser access without login. */
protected int $firstaccesstimeout = 180;
Expand Down Expand Up @@ -208,6 +208,10 @@ public function __construct() {
$this->connectiontimeout = (int)$CFG->session_redis_connection_timeout;
}

if (isset($CFG->session_redis_max_retries)) {
$this->maxretries = (int)$CFG->session_redis_max_retries;
}

$this->clock = di::get(clock::class);
}

Expand Down Expand Up @@ -282,10 +286,10 @@ public function init(): bool {
}
}

// MDL-59866: Add retries for connections (up to 5 times) to make sure it goes through.
// Add retries for connections to make sure it goes through.
$counter = 1;
$exceptionclass = $this->clustermode ? 'RedisClusterException' : 'RedisException';
while ($counter <= self::MAX_RETRIES) {
while ($counter <= $this->maxretries) {
$this->connection = null;
// Make a connection to Redis server(s).
try {
Expand Down Expand Up @@ -387,7 +391,7 @@ public function init(): bool {
return true;
} catch (RedisException | RedisClusterException $e) {
$redishost = $this->clustermode ? implode(',', $this->host) : $server . ':' . $port;
$logstring = "Failed to connect (try {$counter} out of " . self::MAX_RETRIES . ") to Redis ";
$logstring = "Failed to connect (try {$counter} out of " . $this->maxretries . ") to Redis ";
$logstring .= "at ". $redishost .", the error returned was: {$e->getMessage()}";
debugging($logstring);
}
Expand Down
4 changes: 2 additions & 2 deletions lib/tests/session/redis_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -360,8 +360,8 @@ public function test_exception_when_connection_attempts_exceeded(): void {
// Therefore, to get the host, we need to explode it.
list($host, ) = explode(':', TEST_SESSION_REDIS_HOST);

$expected = "Failed to connect (try 5 out of 5) to Redis at $host:111111";
$this->assertDebuggingCalledCount(5);
$expected = "Failed to connect (try 3 out of 3) to Redis at $host:111111";
$this->assertDebuggingCalledCount(3);
$this->assertStringContainsString($expected, $actual);
}

Expand Down

0 comments on commit 2a28dbd

Please sign in to comment.