MDL-79254 quiz_statistics: Convert recalculate task to ad-hoc

Previously, quiz statistics processing happened on a scheduled task.
This task looked for all quizzes with completed attempts, then
determined if those quizzes had a statistics calculation that's newer
than the most recent attempt, then ran the statistics calculation if
needed. It was hard coded to stop processing after 1 hour.

The queries involved in determining which quizzes needed processing
weren't terribly efficient, and combined with the 1 hour limit this made
the statistics unusable on large sites, where they are the most useful.

This converts the scheduled task to an ad-hoc task, and uses an event
observer for mod_quiz\event\attempt_submitted to queue a task when
it is needed. This removes the need for a query to work out what needs
processing, and allows the task processing to be scaled up as needed.
This commit is contained in:
Mark Johnson 2023-09-14 10:13:03 +01:00
parent ecddfa6ccd
commit e5a7a18ae2
11 changed files with 368 additions and 101 deletions

View File

@ -0,0 +1,47 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
namespace quiz_statistics\event\observer;
use quiz_statistics\task\recalculate;
/**
* Event observer for \mod_quiz\event\attempt_submitted
*
* @package quiz_statistics
* @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net}
* @author Mark Johnson <mark.johnson@catalyst-eu.net>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class attempt_submitted {
/**
* Queue an ad-hoc task to recalculate statistics for the quiz.
*
* This will defer running the task for 1 hour, to give other attempts in progress
* a chance to submit.
*
* @param \mod_quiz\event\attempt_submitted $event
* @return void
*/
public static function process(\mod_quiz\event\attempt_submitted $event): void {
$data = $event->get_data();
$quizid = $data['other']['quizid'];
$task = recalculate::instance($quizid);
$task->set_next_run_time(time() + HOURSECS);
\core\task\manager::queue_adhoc_task($task, true);
}
}

View File

@ -18,7 +18,6 @@ namespace quiz_statistics\task;
use core\dml\sql_join;
use mod_quiz\quiz_attempt;
use mod_quiz\quiz_settings;
use quiz_statistics_report;
defined('MOODLE_INTERNAL') || die();
@ -36,9 +35,25 @@ require_once($CFG->dirroot . '/mod/quiz/report/statistics/report.php');
* @author Nathan Nguyen <nathannguyen@catalyst-au.net>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class recalculate extends \core\task\scheduled_task {
/** @var int the maximum length of time one instance of this task will run. */
const TIME_LIMIT = 3600;
class recalculate extends \core\task\adhoc_task {
/**
* Create a new instance of the task.
*
* This sets the properties so that only one task will be queued at a time for a given quiz.
*
* @param int $quizid
* @return recalculate
*/
public static function instance(int $quizid): recalculate {
$task = new self();
$task->set_component('quiz_statistics');
$task->set_userid(get_admin()->id);
$task->set_custom_data((object)[
'quizid' => $quizid,
]);
return $task;
}
public function get_name(): string {
return get_string('recalculatetask', 'quiz_statistics');
@ -46,78 +61,37 @@ class recalculate extends \core\task\scheduled_task {
public function execute(): void {
global $DB;
$stoptime = time() + self::TIME_LIMIT;
$dateformat = get_string('strftimedatetimeshortaccurate', 'core_langconfig');
// TODO: MDL-75197, add quizid in quiz_statistics so that it is simpler to find quizzes for stats calculation.
// Only calculate stats for quizzes which have recently finished attempt.
$latestattempts = $DB->get_records_sql("
SELECT q.id AS quizid,
q.name AS quizname,
q.grademethod AS quizgrademethod,
c.id AS courseid,
c.shortname AS courseshortname,
MAX(quiza.timefinish) AS mostrecentattempttime,
COUNT(1) AS numberofattempts
FROM {quiz_attempts} quiza
JOIN {quiz} q ON q.id = quiza.quiz
JOIN {course} c ON c.id = q.course
WHERE quiza.preview = 0
AND quiza.state = :quizstatefinished
GROUP BY q.id, q.name, q.grademethod, c.id, c.shortname
ORDER BY MAX(quiza.timefinish) DESC
", ["quizstatefinished" => quiz_attempt::FINISHED]);
$anyexception = null;
foreach ($latestattempts as $latestattempt) {
if (time() >= $stoptime) {
mtrace("This task has been running for more than " .
format_time(self::TIME_LIMIT) . " so stopping this execution.");
break;
}
// Check if there is any existing question stats, and it has been calculated after latest quiz attempt.
$qubaids = quiz_statistics_qubaids_condition($latestattempt->quizid,
new sql_join(), $latestattempt->quizgrademethod);
$lateststatstime = $DB->get_field('quiz_statistics', 'COALESCE(MAX(timemodified), 0)',
['hashcode' => $qubaids->get_hash_code()]);
$quizinfo = "'$latestattempt->quizname' ($latestattempt->quizid) in course " .
"$latestattempt->courseshortname ($latestattempt->courseid) has most recent attempt finished at " .
userdate($latestattempt->mostrecentattempttime, $dateformat);
if ($lateststatstime) {
$quizinfo .= " and statistics from " . userdate($lateststatstime, $dateformat);
}
if ($lateststatstime >= $latestattempt->mostrecentattempttime) {
mtrace(" " . $quizinfo . " so nothing to do.");
continue;
}
// OK, so we need to calculate for this quiz.
mtrace(" " . $quizinfo . " so re-calculating statistics for $latestattempt->numberofattempts attempts, start time " .
userdate(time(), $dateformat) . " ...");
try {
$quizobj = quiz_settings::create($latestattempt->quizid);
$report = new quiz_statistics_report();
$report->clear_cached_data($qubaids);
$report->calculate_questions_stats_for_question_bank($quizobj->get_quizid());
mtrace(" Calculations completed at " . userdate(time(), $dateformat) . ".");
} catch (\Throwable $e) {
// We don't want an exception from one quiz to stop processing of other quizzes.
mtrace_exception($e);
$anyexception = $e;
}
$data = $this->get_custom_data();
$quiz = $DB->get_record('quiz', ['id' => $data->quizid]);
if (!$quiz) {
mtrace('Could not find quiz with ID ' . $data->quizid . '.');
return;
}
$course = $DB->get_record('course', ['id' => $quiz->course]);
if (!$course) {
mtrace('Could not find course with ID ' . $quiz->course . '.');
return;
}
$attemptcount = $DB->count_records('quiz_attempts', ['quiz' => $data->quizid, 'state' => quiz_attempt::FINISHED]);
if ($attemptcount === 0) {
mtrace('Could not find any finished attempts for course with ID ' . $data->quizid . '.');
return;
}
if ($anyexception) {
// If there was any error, ensure the task fails.
throw $anyexception;
}
mtrace("Re-calculating statistics for quiz {$quiz->name} ({$quiz->id}) " .
"from course {$course->shortname} ({$course->id}) with {$attemptcount} attempts, start time " .
userdate(time(), $dateformat) . " ...");
$qubaids = quiz_statistics_qubaids_condition(
$quiz->id,
new sql_join(),
$quiz->grademethod
);
$report = new quiz_statistics_report();
$report->clear_cached_data($qubaids);
$report->calculate_questions_stats_for_question_bank($quiz->id);
mtrace(' Calculations completed at ' . userdate(time(), $dateformat) . '.');
}
}

View File

@ -0,0 +1,46 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
namespace quiz_statistics\tests;
/**
* Test helper functions for statistics
*
* @package quiz_statistics
* @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net}
* @author Mark Johnson <mark.johnson@catalyst-eu.net>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class statistics_helper {
/**
* Run any ad-hoc recalculation tasks that have been scheduled.
*
* We need a special function to do this as the tasks are deferred by one hour,
* so we need to pass a custom $timestart argument.
*
* @return void
*/
public static function run_pending_recalculation_tasks(): void {
while ($task = \core\task\manager::get_next_adhoc_task(
time() + HOURSECS + 1,
false,
'\quiz_statistics\task\recalculate'
)) {
$task->execute();
\core\task\manager::adhoc_task_complete($task);
}
}
}

View File

@ -15,25 +15,19 @@
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
/**
* Legacy Cron Quiz Reports Task
*
* @package quiz_statistics
* @copyright 2017 Michael Hughes, University of Strathclyde
* @author Michael Hughes <michaelhughes@strath.ac.uk>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* Add event observers for quiz_statistics
*
* @package quiz_statistics
* @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net}
* @author Mark Johnson <mark.johnson@catalyst-eu.net>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
defined('MOODLE_INTERNAL') || die();
$tasks = [
$observers = [
[
'classname' => 'quiz_statistics\task\recalculate',
'blocking' => 0,
'minute' => 'R',
'hour' => '*/4',
'day' => '*',
'dayofweek' => '*',
'month' => '*'
]
'eventname' => '\mod_quiz\event\attempt_submitted',
'callback' => '\quiz_statistics\event\observer\attempt_submitted::process',
],
];

View File

@ -0,0 +1,165 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
namespace quiz_statistics\event\observer;
defined('MOODLE_INTERNAL') || die();
global $CFG;
require_once($CFG->dirroot . '/mod/quiz/tests/quiz_question_helper_test_trait.php');
use core\task\manager;
use quiz_statistics\task\recalculate;
use quiz_statistics\tests\statistics_helper;
/**
* Unit tests for attempt_submitted observer
*
* @package quiz_statistics
* @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net}
* @author Mark Johnson <mark.johnson@catalyst-eu.net>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @covers \quiz_statistics\event\observer\attempt_submitted
*/
class attempt_submitted_test extends \advanced_testcase {
use \quiz_question_helper_test_trait;
/**
* Return a user, and a quiz with 2 questions.
*
* @return array [$user, $quiz, $course]
*/
protected function create_test_data(): array {
$this->resetAfterTest(true);
$generator = $this->getDataGenerator();
$user = $generator->create_user();
$course = $generator->create_course();
$quiz = $this->create_test_quiz($course);
$this->add_two_regular_questions($generator->get_plugin_generator('core_question'), $quiz);
return [$user, $quiz, $course];
}
/**
* Assert that a task is queued for a quiz.
*
* Check that the quizid stored in the task's custom data matches the provided quiz,
* and that the run time is in one hour from when the test is being run (within a small margin of error).
*
* @param recalculate $task
* @param \stdClass $quiz
* @return void
*/
protected function assert_task_is_queued_for_quiz(recalculate $task, \stdClass $quiz): void {
$data = $task->get_custom_data();
$this->assertEquals($quiz->id, $data->quizid);
$this->assertEqualsWithDelta(time() + HOURSECS, $task->get_next_run_time(), 1);
}
/**
* Attempting a quiz should queue the recalculation task for that quiz in 1 hour's time.
*
* @return void
*/
public function test_queue_task_on_submission(): void {
[$user, $quiz] = $this->create_test_data();
$tasks = manager::get_adhoc_tasks(recalculate::class);
$this->assertEmpty($tasks);
$this->attempt_quiz($quiz, $user);
$tasks = manager::get_adhoc_tasks(recalculate::class);
$this->assertCount(1, $tasks);
$task = reset($tasks);
$this->assert_task_is_queued_for_quiz($task, $quiz);
}
/**
* Attempting a quiz multiple times should only queue one instance of the task.
*
* @return void
*/
public function test_queue_single_task_for_multiple_submissions(): void {
[$user1, $quiz] = $this->create_test_data();
$user2 = $this->getDataGenerator()->create_user();
$tasks = manager::get_adhoc_tasks(recalculate::class);
$this->assertEmpty($tasks);
$this->attempt_quiz($quiz, $user1);
$this->attempt_quiz($quiz, $user2);
$tasks = manager::get_adhoc_tasks(recalculate::class);
$this->assertCount(1, $tasks);
$task = reset($tasks);
$this->assert_task_is_queued_for_quiz($task, $quiz);
}
/**
* Attempting the quiz again after processing the task should queue a new task.
*
* @return void
*/
public function test_queue_new_task_after_processing(): void {
[$user1, $quiz, $course] = $this->create_test_data();
$user2 = $this->getDataGenerator()->create_user();
$tasks = manager::get_adhoc_tasks(recalculate::class);
$this->assertEmpty($tasks);
$this->attempt_quiz($quiz, $user1);
$tasks = manager::get_adhoc_tasks(recalculate::class);
$this->assertCount(1, $tasks);
$this->expectOutputRegex("~Re-calculating statistics for quiz {$quiz->name} \({$quiz->id}\) " .
"from course {$course->shortname} \({$course->id}\) with 1 attempts~");
statistics_helper::run_pending_recalculation_tasks();
$tasks = manager::get_adhoc_tasks(recalculate::class);
$this->assertEmpty($tasks);
$this->attempt_quiz($quiz, $user2);
$tasks = manager::get_adhoc_tasks(recalculate::class);
$this->assertCount(1, $tasks);
$task = reset($tasks);
$this->assert_task_is_queued_for_quiz($task, $quiz);
}
/**
* Attempting different quizzes will queue a task for each.
*
* @return void
*/
public function test_queue_separate_tasks_for_multiple_quizzes(): void {
[$user1, $quiz1] = $this->create_test_data();
[$user2, $quiz2] = $this->create_test_data();
$tasks = manager::get_adhoc_tasks(recalculate::class);
$this->assertEmpty($tasks);
$this->attempt_quiz($quiz1, $user1);
$this->attempt_quiz($quiz2, $user2);
$tasks = manager::get_adhoc_tasks(recalculate::class);
$this->assertCount(2, $tasks);
$task1 = array_shift($tasks);
$this->assert_task_is_queued_for_quiz($task1, $quiz1);
$task2 = array_shift($tasks);
$this->assert_task_is_queued_for_quiz($task2, $quiz2);
}
}

View File

@ -24,6 +24,6 @@
defined('MOODLE_INTERNAL') || die();
$plugin->version = 2023042400;
$plugin->version = 2023042403;
$plugin->requires = 2023041800;
$plugin->component = 'quiz_statistics';

View File

@ -0,0 +1,41 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
require_once(__DIR__ . '/../../../../../lib/behat/behat_base.php');
/**
* Behat steps for qbank_statistics
*
* @package qbank_statistics
* @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net}
* @author Mark Johnson <mark.johnson@catalyst-eu.net>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class behat_qbank_statistics extends behat_base {
/**
* Run pending recalculation tasks.
*
* This runs the recalcuation ad-hoc tasks. We need a special step for this
* as the run time for these tasks is set to an hour in the future, so
* "I run all ad-hoc tasks" will not trigger them.
*
* @Given /^I run pending statistics recalculation tasks$/
*/
function i_run_pending_statistics_recalculation_tasks() {
\quiz_statistics\tests\statistics_helper::run_pending_recalculation_tasks();
}
}

View File

@ -73,7 +73,7 @@ Feature: Show statistics in question bank
| slot | response |
| 1 | True |
| 2 | True |
And I run the scheduled task "\quiz_statistics\task\recalculate"
And I run pending statistics recalculation tasks
When I am on the "Course 1" "core_question > course question bank" page logged in as "admin"
Then I should see "50.00%" in the "TF1" "table_row"
And I should see "75.00%" in the "TF2" "table_row"
@ -89,7 +89,7 @@ Feature: Show statistics in question bank
| slot | response |
| 1 | True |
| 2 | True |
And I run the scheduled task "\quiz_statistics\task\recalculate"
And I run pending statistics recalculation tasks
When I am on the "Course 1" "core_question > course question bank" page logged in as "admin"
Then I should see "50.00%" in the "TF1" "table_row"
And I should see "75.00%" in the "TF2" "table_row"
@ -105,7 +105,7 @@ Feature: Show statistics in question bank
| slot | response |
| 1 | True |
| 2 | True |
And I run the scheduled task "\quiz_statistics\task\recalculate"
And I run pending statistics recalculation tasks
When I am on the "Course 1" "core_question > course question bank" page logged in as "admin"
Then I should see "Likely" in the "TF1" "table_row"
And I should see "Unlikely" in the "TF2" "table_row"
@ -127,7 +127,7 @@ Feature: Show statistics in question bank
| slot | response |
| 1 | True |
| 2 | False |
And I run the scheduled task "\quiz_statistics\task\recalculate"
And I run pending statistics recalculation tasks
When I am on the "Course 1" "core_question > course question bank" page logged in as "admin"
Then I should see "Likely" in the "TF1" "table_row"
And I should see "Very likely" in the "TF2" "table_row"
@ -207,7 +207,7 @@ Feature: Show statistics in question bank
| 3 | Two |
| 4 | Two |
| 5 | One |
And I run the scheduled task "\quiz_statistics\task\recalculate"
And I run pending statistics recalculation tasks
# Confirm the "Needs checking?" column matches the expected values based on students' answers
When I am on the "Quiz 3" "mod_quiz > question bank" page logged in as "admin"
Then the following should exist in the "categoryquestions" table:

View File

@ -19,6 +19,7 @@ namespace qbank_statistics;
defined('MOODLE_INTERNAL') || die();
use core_question\statistics\questions\all_calculated_for_qubaid_condition;
use quiz_statistics\tests\statistics_helper;
use mod_quiz\quiz_attempt;
use mod_quiz\quiz_settings;
use question_engine;
@ -235,8 +236,7 @@ class helper_test extends \advanced_testcase {
// Calculate the statistics.
$this->expectOutputRegex('~.*Calculations completed.*~');
$statisticstask = new \quiz_statistics\task\recalculate();
$statisticstask->execute();
statistics_helper::run_pending_recalculation_tasks();
return [$quiz1, $quiz2, $questions];
}

View File

@ -16,6 +16,7 @@
namespace core_question;
use quiz_statistics\tests\statistics_helper;
use mod_quiz\quiz_attempt;
use mod_quiz\quiz_settings;
use qubaid_list;
@ -376,7 +377,6 @@ class datalib_reporting_queries_test extends \qbehaviour_walkthrough_test_base {
// Calculate the statistics.
$this->expectOutputRegex('~.*Calculations completed.*~');
$statisticstask = new \quiz_statistics\task\recalculate();
$statisticstask->execute();
statistics_helper::run_pending_recalculation_tasks();
}
}

View File

@ -22,6 +22,7 @@ use advanced_testcase;
use context;
use context_module;
use core_question\statistics\questions\all_calculated_for_qubaid_condition;
use quiz_statistics\tests\statistics_helper;
use core_question_generator;
use Generator;
use mod_quiz\quiz_attempt;
@ -256,8 +257,7 @@ class statistics_bulk_loader_test extends advanced_testcase {
// Calculate the statistics.
$this->expectOutputRegex('~.*Calculations completed.*~');
$statisticstask = new \quiz_statistics\task\recalculate();
$statisticstask->execute();
statistics_helper::run_pending_recalculation_tasks();
return [$quiz1, $quiz2, $questions];
}