Skip to content

Commit

Permalink
Merge branch 'MDL-68668-extra-redis-debugging' of https://github.com/…
Browse files Browse the repository at this point in the history
  • Loading branch information
junpataleta committed May 23, 2022
2 parents 653011e + e032ac9 commit 69a3c6f
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 12 deletions.
24 changes: 13 additions & 11 deletions config-dist.php
Original file line number Diff line number Diff line change
Expand Up @@ -317,20 +317,22 @@
// Redis session handler (requires redis server and redis extension):
// $CFG->session_handler_class = '\core\session\redis';
// $CFG->session_redis_host = '127.0.0.1';
// $CFG->session_redis_port = 6379; // Optional.
// $CFG->session_redis_database = 0; // Optional, default is db 0.
// $CFG->session_redis_auth = ''; // Optional, default is don't set one.
// $CFG->session_redis_prefix = ''; // Optional, default is don't set one.
// $CFG->session_redis_acquire_lock_timeout = 120;
// $CFG->session_redis_lock_expire = 7200;
// $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_port = 6379; // Optional.
// $CFG->session_redis_database = 0; // Optional, default is db 0.
// $CFG->session_redis_auth = ''; // Optional, default is don't set one.
// $CFG->session_redis_prefix = ''; // Optional, default is don't set one.
// $CFG->session_redis_acquire_lock_timeout = 120; // Default is 2 minutes.
// $CFG->session_redis_acquire_lock_warn = 0; // If set logs early warning if a lock has not been acquried.
// $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.
//
// 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!
// $CFG->session_redis_serializer_use_igbinary = false; // Optional, default is PHP builtin serializer.
// $CFG->session_redis_compressor = 'none'; // Optional, possible values are:
// // 'gzip' - PHP GZip compression
// // 'zstd' - PHP Zstandard compression
// $CFG->session_redis_compressor = 'none'; // Optional, possible values are:
// // 'gzip' - PHP GZip compression
// // 'zstd' - PHP Zstandard compression
//
// Please be aware that when selecting Memcached for sessions that it is advised to use a dedicated
// memcache server. The memcached extension does not provide isolated environments for individual uses.
Expand Down
19 changes: 18 additions & 1 deletion lib/classes/session/redis.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ class redis extends handler {
protected $prefix = '';
/** @var int $acquiretimeout how long to wait for session lock in seconds */
protected $acquiretimeout = 120;
/** @var int $acquirewarn how long before warning when waiting for a lock in seconds */
protected $acquirewarn = null;
/** @var int $lockretry how long to wait between session lock attempts in ms */
protected $lockretry = 100;
/** @var int $serializer The serializer to use */
Expand Down Expand Up @@ -119,6 +121,10 @@ public function __construct() {
$this->acquiretimeout = (int)$CFG->session_redis_acquire_lock_timeout;
}

if (isset($CFG->session_redis_acquire_lock_warn)) {
$this->acquirewarn = (int)$CFG->session_redis_acquire_lock_warn;
}

if (isset($CFG->session_redis_acquire_lock_retry)) {
$this->lockretry = (int)$CFG->session_redis_acquire_lock_retry;
}
Expand Down Expand Up @@ -461,6 +467,8 @@ protected function lock_session($id) {

$whoami = "[pid {$pid}] {$hostname}:$uri";

$haswarned = false; // Have we logged a lock warning?
while (!$haslock) {

$haslock = $this->connection->setnx($lockkey, $whoami);
Expand All @@ -471,13 +479,22 @@ protected function lock_session($id) {
return true;
}

if (!empty($this->acquirewarn) && !$haswarned && $this->time() > $startlocktime + $this->acquirewarn) {
// This is a warning to better inform users.
$whohaslock = $this->connection->get($lockkey);
// phpcs:ignore
error_log("Warning: Cannot obtain session lock for sid: $id within $this->acquirewarn seconds but will keep trying. " .
"It is likely another page ($whohaslock) has a long session lock, or the session lock was never released.");
$haswarned = true;
}

if ($this->time() > $startlocktime + $this->acquiretimeout) {
// This is a fatal error, better inform users.
// It should not happen very often - all pages that need long time to execute
// should close session immediately after access control checks.
$whohaslock = $this->connection->get($lockkey);
// phpcs:ignore
error_log("Cannot obtain session lock for sid: $id within $this->acquiretimeout seconds. " .
error_log("Error: Cannot obtain session lock for sid: $id within $this->acquiretimeout seconds. " .
"It is likely another page ($whohaslock) has a long session lock, or the session lock was never released.");
throw new exception("Unable to obtain session lock");
}
Expand Down
1 change: 1 addition & 0 deletions lib/upgrade.txt
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ defined or can't be applied.
* A new method, force_lock_all_blocks(), has been added to the moodle_page class to allow pages to force the value of
user_can_edit_blocks() to return false where necessary. This makes it possible to remove block editing on a page
from ALL users, including admins, where required on pages with multi region layouts exist, such as "My courses".
* Add an early $CFG->session_redis_acquire_lock_warn option

=== 3.11.4 ===
* A new option dontforcesvgdownload has been added to the $options parameter of the send_file() function.
Expand Down

0 comments on commit 69a3c6f

Please sign in to comment.