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.
This commit removes code that only was being executed by php < 73
and it's 100% safe to do so because Moodle 3.11 and up require
php 73, hence it was not executed ever.
Removed code includes:
- ldap_control_paged_result and ldap_control_paged_result_response
(that were deprecated in php 73 and have been removed in php 80).
- conditional code in the session manager, where some hacks were
needed for php < 73. Note that this removes the private function
append_samesite_cookie_attribute() completely because it was
doinf nothing (first line was returning for php < 73).
- Also removed the old session.hash_function ini setting because
it was removed in php 71.
Kept code includes:
- The environmental check_igbinary322_version test has not been
removed because it doesn't hurt (always returns "ok" for php 73
sites) and doing it would involve to backport the environment.xml
file to 39 and 310. Instead, a note has been added to MDL-71747
in order to get rid of that check for 4.1 and up.
When session lock debugging and read only sessions deubgging are both
enabled, session lock debugging becomes moot. This patch causes
the session lock deubgging code to exit early if read only sessions
debugging is enabled.
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>
Prior to this patch the debugging mode (when enabled) would trigger
on everywhere, regardless of whether or not READ_ONLY_SESSION is defined.
This patch modifies that behaviour so that the debugging only kicks in
if READ_ONLY_SESSION is defined and set to true.
If a developer has debugging on then they will recieve all debugging
messages for locking whether wanted or not. We already have a setting
to display these messages.
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 ''.
CSRF protection for the login form. The authenticate_user_login function was
extended to validate the token (in \core\session\manager) but by default it
does not perform the extra validation. Existing uses of this function from
auth plugins and features like "change password" will continue to work without
changes. New config value $CFG->disablelogintoken can bypass this check.
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.
