mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-01-19 06:18:28 +01:00
Code Issues Projects Releases Wiki Activity

Merge branch 'MDL-75736-master' of https://github.com/sammarshallou/moodle

Browse Source
This commit is contained in:
Víctor Déniz 2022-10-14 16:09:01 +01:00
commit ab764c0062
2 changed files with 50 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
lib/modinfolib.php
View File

@ -471,13 +471,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 {

45
lib/tests/modinfolib_test.php
View File

@ -1126,4 +1126,49 @@ class modinfolib_test extends advanced_testcase {
$this->expectExceptionMessage('Invalid course module id: ' . $forum1->cmid);
delete_course($course, false);
}
/**
* 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'));
}
}