PHP allows the specification of invalid values for `max_execution_time`,
including negative values. These should be normalised the same way that
an empty (unlimited) value is.
Storage of session metadata has moved into the session handler class.
This allows for other classes to fully control session handling and
removes the dependancy on the core sessions database table.
Previously, the standard method of interaction with the
session metadata was direct DB calls; this may break other plugins as there
are now proper APIs available through the session manager.
Co-authored-by: Darren Cocco <moodle@darren.cocco.id.au>
Co-authored-by: Trisha Milan <trishamilan@catalyst-au.net>
Co-authored-by: Andrew Nicols <andrew@nicols.co.uk>
The fix covers two places:
- The Redis session.
- The Redis cache store.
The PHP Redis has the option to establish the connection using TLS/SSL instead of using the "tls://" prefix.
This is done to ensure consistency with the Redis cluster connection,
as the code for both single and cluster connections is located in one place.
Also added @runInSeparateProcess on each session Redis cluster test to let the test run
in a separate process to avoid the error "the headers are not already sent" by PHPUnit.
Unfortunately, the read_timeout named parameter was not recognized on some machines.
To avoid such errors, this patch removed the named parameter on a single Redis connect() and the Redis cluster.
If for any reason a Redis session lock is not being released, all subsequent
requests will wait to acquire the lock, forcing them to time out eventually.
This will happen till the original lock finally expires after the session timeout.
This sets the Redis session lock expiry time to whatever is lower,
either the PHP execution time `max_execution_time`, if the value was
defined in the `php.ini` or the globally configured `sessiontimeout`.
Setting it to the lower of the two will not make things worse it if the
execution timeout is longer than the session timeout.
For the PHP execution time, once the PHP execution time is over, we can
be sure that the lock is no longer actively held so that the lock can
expire safely. Although at `lib/classes/php_time_limit.php::raise(int)`,
Moodle can progressively increase the maximum PHP execution time, this
is limited to the `max_execution_time` value defined in the `php.ini`.
For the session timeout, we assume it is safe to consider the lock to
expire once the session itself expires.
If we unnecessarily hold the lock any longer, it blocks other session
requests.
Co-authored-by: Daniel Ziegenberg <daniel@ziegenberg.at>
Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
PHP before version 8.1 automatically converted to int if the function
parameter (or array key) is expected to be int. PHP 8.1 shows notice in
this case
The Redis cache store and session handler both do a 'ping()' after
connecting to Redis. This is unnecessary because the connect() call
has just checked the network connection and it's hardly likely that
the server has gone down since then.
According to my profiling, both connect() and ping() take
measurable time when talking to a separate server, i.e. a few
milliseconds. So it's not the case that connect() doesn't really
talk to the server, as I initially wondered.
If using Redis on a separate (non-localhost) server for both session
and cache store, removing these ping calls can save a millisecond
or two per request.
The 'unable to obtain session lock'-exception raised by the Redis
session handler is hardcoded in English and not all that useful
to the end user.
This change adds the error message to the lang/error.php and gives
the user further hints why the error might have occured and how it
could be fixed.
Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
The PHP_CodeSniffer @codingStandardsIgnore annotations are deprecated
and, since version 3.x, the new // phpcs:ignore comments should be used
instead.
This commits just reviews all the uses in core, replacing them for
the better new candidate, or removing when no longer needed.
The random retry delay for redis session cache was calculated as
rand(100000, 500000) giving an effective retry delay of 100 seconds
to 500 seconds. That's off by a factor of a thousand! Using Redis as a
session cache and when the connection hangs, you can get random
"cannot obtain session lock" errors because it's waiting up to
500 seconds (or about 8.33 minutes) for a Redis connection.
This sets the delay to the originally intended 100ms to 500ms.
(see MDL-59866).
Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
In 39770792ca read-only sessions were allowed.
In the redis case, as called from the mobile application,
this can lead to returning 'false' for session rather than ''.
There is an edge case whereby redis will fail
to accept connections on the first try but
retrying the connection seems to make it work
Included in this commit:
* Retry functionality in the session init
The redis session handler doesn't use the sessiontimeout config setting
to determine session lifetime.
It has a lock expiry, which is set to 7200 (or a config setting) that is
used to determine how long a lock is held onto, but that should be
distinct from the session timeout.
It is now possible to set up Redis as a session handler for Moodle.
Ensure that the phpredis extension is enabled and working on your sever.
Please place the following lines in config.php
$CFG->session_handler_class = '\core\session\redis';
$CFG->session_redis_save_path = 'tcp://127.0.0.1';
To determine if it has been set correctly, navigate to
$CFG->wwwroot/admin/phpinfo.php and find following the strings in the
session block,
session.save_handler = redis
session.save_path = tcp://127.0.0.1
Jun Pataleta