L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'MDL-75736-m400' of https://github.com/sammarshallou/moodle into MOODLE_400_STABLE

Browse Source
This commit is contained in:
Víctor Déniz
2022-10-14 16:10:43 +01:00
parent 00ad2a14da a7d47d7813
commit dfd9b50595
2 changed files with 50 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/modinfolib.php
+5 -2
View File
@@ -476,13 +476,16 @@ class course_modinfo {
// Retrieve modinfo from cache. If not present or cacherev mismatches, call rebuild and retrieve again.
$coursemodinfo = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev);
if ($coursemodinfo === false || ($course->cacherev != $coursemodinfo->cacherev)) {
// Note the version comparison using the data in the cache should not be necessary, but the
// partial rebuild logic sometimes sets the $coursemodinfo->cacherev to -1 which is an
// indicator that it needs rebuilding.
if ($coursemodinfo === false || ($course->cacherev > $coursemodinfo->cacherev)) {
$lock = self::get_course_cache_lock($course->id);
try {
// Only actually do the build if it's still needed after getting the lock (not if
// somebody else, who might have been holding the lock, built it already).
$coursemodinfo = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev);
if ($coursemodinfo === false || ($course->cacherev != $coursemodinfo->cacherev)) {
if ($coursemodinfo === false || ($course->cacherev > $coursemodinfo->cacherev)) {
$coursemodinfo = self::inner_build_course_cache($course, $lock);
}
} finally {
lib/tests/modinfolib_test.php
+45
View File
@@ -1096,4 +1096,49 @@ class modinfolib_test extends advanced_testcase {
// Make sure that the cacherev will be reset.
$this->assertEquals(-1, $coursemodinfo->cacherev);
}
/**
* Tests that if the modinfo cache returns a newer-than-expected version, Moodle won't rebuild
* it.
*
* This is important to avoid wasted time/effort and poor performance, for example in cases
* where multiple requests are accessing the course.
*
* Certain cases could be particularly bad if this test fails. For example, if using clustered
* databases where there is a 100ms delay between updates to the course table being available
* to all users (but no such delay on the cache infrastructure), then during that 100ms, every
* request that calls get_fast_modinfo and uses the read-only database will rebuild the course
* cache. Since these will then create a still-newer version, future requests for the next
* 100ms will also rebuild it again... etc.
*
* @covers \course_modinfo
*/
public function test_get_modinfo_with_newer_version(): void {
global $DB;
$this->resetAfterTest();
// Get info about a course and build the initial cache, then drop it from memory.
$course = $this->getDataGenerator()->create_course();
get_fast_modinfo($course);
get_fast_modinfo(0, 0, true);
// User A starts a request, which takes some time...
$useracourse = $DB->get_record('course', ['id' => $course->id]);
// User B also starts a request and makes a change to the course.
$userbcourse = $DB->get_record('course', ['id' => $course->id]);
$this->getDataGenerator()->create_module('page', ['course' => $course->id]);
rebuild_course_cache($userbcourse->id, false);
// Finally, user A's request now gets modinfo. It should accept the version from B even
// though the course version (of cache) is newer than the one expected by A.
$before = $DB->perf_get_queries();
$modinfo = get_fast_modinfo($useracourse);
$after = $DB->perf_get_queries();
$this->assertEquals($after, $before, 'Should use cached version, making no DB queries');
// Obviously, modinfo should include the Page now.
$this->assertCount(1, $modinfo->get_instances_of('page'));
}
}