MDL-75576 quiz/question statistics: don't expire by time

Previously, a set of calculated quiz statistics would only 'last' for
15 minutes. Then they would be considered invalid and not used.

Now, computed statistics are kept indefinitely. Instead, when a new
batch of values are computed for a particular set of settings, older numbers
for the same settings are deleted first. Therefore,
question_stats_cleanup_task is no more.
This commit is contained in:
Tim Hunt 2023-03-14 14:33:24 +00:00
parent a532f407bb
commit efe895f1bb
12 changed files with 105 additions and 115 deletions

View File

@ -1,65 +0,0 @@
<?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/>.
/**
* Task to cleanup old question statistics cache.
*
* @package core
* @copyright 2019 Simey Lameze <simey@moodle.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
namespace core\task;
defined('MOODLE_INTERNAL') || die();
/**
* A task to cleanup old question statistics cache.
*
* @copyright 2019 Simey Lameze <simey@moodle.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class question_stats_cleanup_task extends scheduled_task {
/**
* Get a descriptive name for this task (shown to admins).
*
* @return string
*/
public function get_name() {
return get_string('taskquestionstatscleanupcron', 'admin');
}
/**
* Perform the cleanup task.
*/
public function execute() {
global $DB;
mtrace("\n Cleaning up old question statistics cache records...", '');
$expiretime = time() - 5 * HOURSECS;
$DB->delete_records_select('question_statistics', 'timemodified < ?', [$expiretime]);
$responseanlysisids = $DB->get_records_select_menu('question_response_analysis',
'timemodified < ?',
[$expiretime],
'id',
'id, id AS id2');
$DB->delete_records_list('question_response_analysis', 'id', $responseanlysisids);
$DB->delete_records_list('question_response_count', 'analysisid', $responseanlysisids);
mtrace('done.');
}
}

View File

@ -221,15 +221,6 @@ $tasks = array(
'dayofweek' => '*',
'month' => '*'
),
array(
'classname' => 'core\task\question_stats_cleanup_task',
'blocking' => 0,
'minute' => '*',
'hour' => '*',
'day' => '*',
'dayofweek' => '*',
'month' => '*'
),
array(
'classname' => 'core\task\registration_cron_task',
'blocking' => 0,

View File

@ -225,9 +225,13 @@ class calculated {
$toinsert->standarderror = null;
}
// Delete older statistics before we save the new ones.
$transaction = $DB->start_delegated_transaction();
$DB->delete_records('quiz_statistics', ['hashcode' => $qubaids->get_hash_code()]);
// Store the data.
$DB->insert_record('quiz_statistics', $toinsert);
$transaction->allow_commit();
}
/**

View File

@ -128,12 +128,17 @@ class calculator {
* Load cached statistics from the database.
*
* @param \qubaid_condition $qubaids
* @return calculated The statistics for overall attempt scores or false if not cached.
* @return calculated|false The statistics for overall attempt scores or false if not cached.
*/
public function get_cached($qubaids) {
global $DB;
$fromdb = $DB->get_record('quiz_statistics', ['hashcode' => $qubaids->get_hash_code()]);
$lastcalculatedtime = $this->get_last_calculated_time($qubaids);
if (!$lastcalculatedtime) {
return false;
}
$fromdb = $DB->get_record('quiz_statistics', ['hashcode' => $qubaids->get_hash_code(),
'timemodified' => $lastcalculatedtime]);
$stats = new calculated();
$stats->populate_from_record($fromdb);
return $stats;
@ -147,7 +152,13 @@ class calculator {
*/
public function get_last_calculated_time($qubaids) {
global $DB;
return $DB->get_field('quiz_statistics', 'timemodified', ['hashcode' => $qubaids->get_hash_code()]);
$lastcalculatedtime = $DB->get_field('quiz_statistics', 'COALESCE(MAX(timemodified), 0)',
['hashcode' => $qubaids->get_hash_code()]);
if ($lastcalculatedtime) {
return $lastcalculatedtime;
} else {
return false;
}
}
/**

View File

@ -25,6 +25,7 @@
defined('MOODLE_INTERNAL') || die();
use core_question\statistics\responses\analyser;
use core_question\statistics\questions\all_calculated_for_qubaid_condition;
require_once($CFG->dirroot . '/mod/quiz/report/default.php');
@ -420,7 +421,7 @@ class quiz_statistics_report extends quiz_default_report {
}
}
$responesanalyser = new \core_question\statistics\responses\analyser($question, $whichtries);
$responesanalyser = new analyser($question, $whichtries);
$responseanalysis = $responesanalyser->load_cached($qubaids, $whichtries);
$qtable->question_setup($reporturl, $question, $s, $responseanalysis);
@ -748,10 +749,8 @@ class quiz_statistics_report extends quiz_default_report {
foreach ($questions as $question) {
$progress->increment_progress();
if (question_bank::get_qtype($question->qtype, false)->can_analyse_responses() && !isset($done[$question->id])) {
$responesstats = new \core_question\statistics\responses\analyser($question, $whichtries);
if ($responesstats->get_last_analysed_time($qubaids, $whichtries) === false) {
$responesstats->calculate($qubaids, $whichtries);
}
$responesstats = new analyser($question, $whichtries);
$responesstats->calculate($qubaids, $whichtries);
}
$done[$question->id] = 1;
}

View File

@ -38,7 +38,7 @@ use question_bank;
*/
class all_calculated_for_qubaid_condition {
/** @var int Time after which statistics are automatically recomputed. */
/** @var int No longer used. Previously, the time after which statistics are automatically recomputed. */
const TIME_TO_CACHE = 900; // 15 minutes.
/**
@ -197,9 +197,9 @@ class all_calculated_for_qubaid_condition {
public function get_cached($qubaids) {
global $DB;
$timemodified = time() - self::TIME_TO_CACHE;
$questionstatrecs = $DB->get_records_select('question_statistics', 'hashcode = ? AND timemodified > ?',
array($qubaids->get_hash_code(), $timemodified));
$timemodified = self::get_last_calculated_time($qubaids);
$questionstatrecs = $DB->get_records('question_statistics',
['hashcode' => $qubaids->get_hash_code(), 'timemodified' => $timemodified]);
$questionids = array();
foreach ($questionstatrecs as $fromdb) {
@ -251,18 +251,26 @@ class all_calculated_for_qubaid_condition {
*/
public function get_last_calculated_time($qubaids) {
global $DB;
$timemodified = time() - self::TIME_TO_CACHE;
return $DB->get_field_select('question_statistics', 'timemodified', 'hashcode = ? AND timemodified > ?',
array($qubaids->get_hash_code(), $timemodified), IGNORE_MULTIPLE);
$lastcalculatedtime = $DB->get_field('question_statistics', 'COALESCE(MAX(timemodified), 0)',
['hashcode' => $qubaids->get_hash_code()]);
if ($lastcalculatedtime) {
return $lastcalculatedtime;
} else {
return false;
}
}
/**
* Save stats to db.
* Save stats to db, first cleaning up any old ones.
*
* @param \qubaid_condition $qubaids Which question usages are we caching the stats of?
*/
public function cache($qubaids) {
global $DB;
$transaction = $DB->start_delegated_transaction();
$timemodified = time();
foreach ($this->get_all_slots() as $slot) {
$this->for_slot($slot)->cache($qubaids);
}
@ -270,6 +278,8 @@ class all_calculated_for_qubaid_condition {
foreach ($this->get_all_subq_ids() as $subqid) {
$this->for_subq($subqid)->cache($qubaids);
}
$transaction->allow_commit();
}
/**

View File

@ -41,7 +41,7 @@ class analyser {
*/
const MAX_TRY_COUNTED = 5;
/** @var int Time after which responses are automatically reanalysed. */
/** @var int No longer used. Previously the time after which statistics are automatically recomputed. */
const TIME_TO_CACHE = 900; // 15 minutes.
/** @var object full question data from db. */
@ -52,6 +52,11 @@ class analyser {
*/
public $analysis;
/**
* @var int used during calculations, so all results are stored with the same timestamp.
*/
protected $calculationtime;
/**
* @var array Two index array first index is unique string for each sub question part, the second string index is the 'class'
* that sub-question part can be classified into.
@ -109,7 +114,7 @@ class analyser {
}
/**
* Analyse all the response data for for all the specified attempts at this question.
* Analyse all the response data for all the specified attempts at this question.
*
* @param \qubaid_condition $qubaids which attempts to consider.
* @param string $whichtries which tries to analyse. Will be one of
@ -117,6 +122,7 @@ class analyser {
* @return analysis_for_question
*/
public function calculate($qubaids, $whichtries = \question_attempt::LAST_TRY) {
$this->calculationtime = time();
// Load data.
$dm = new \question_engine_data_mapper();
$questionattempts = $dm->load_attempts_at_question($this->questiondata->id, $qubaids);
@ -131,7 +137,7 @@ class analyser {
}
}
$this->analysis->cache($qubaids, $whichtries, $this->questiondata->id);
$this->analysis->cache($qubaids, $whichtries, $this->questiondata->id, $this->calculationtime);
return $this->analysis;
}
@ -145,23 +151,23 @@ class analyser {
public function load_cached($qubaids, $whichtries) {
global $DB;
$timemodified = time() - self::TIME_TO_CACHE;
$timemodified = self::get_last_analysed_time($qubaids, $whichtries);
// Variable name 'analyses' is the plural of 'analysis'.
$responseanalyses = $DB->get_records_select('question_response_analysis',
'hashcode = ? AND whichtries = ? AND questionid = ? AND timemodified > ?',
array($qubaids->get_hash_code(), $whichtries, $this->questiondata->id, $timemodified));
$responseanalyses = $DB->get_records('question_response_analysis',
['hashcode' => $qubaids->get_hash_code(), 'whichtries' => $whichtries,
'questionid' => $this->questiondata->id, 'timemodified' => $timemodified]);
if (!$responseanalyses) {
return false;
}
$analysisids = array();
$analysisids = [];
foreach ($responseanalyses as $responseanalysis) {
$analysisforsubpart = $this->analysis->get_analysis_for_subpart($responseanalysis->variant, $responseanalysis->subqid);
$class = $analysisforsubpart->get_response_class($responseanalysis->aid);
$class->add_response($responseanalysis->response, $responseanalysis->credit);
$analysisids[] = $responseanalysis->id;
}
list($sql, $params) = $DB->get_in_or_equal($analysisids);
[$sql, $params] = $DB->get_in_or_equal($analysisids);
$counts = $DB->get_records_select('question_response_count', "analysisid {$sql}", $params);
foreach ($counts as $count) {
$responseanalysis = $responseanalyses[$count->analysisid];
@ -183,11 +189,8 @@ class analyser {
*/
public function get_last_analysed_time($qubaids, $whichtries) {
global $DB;
$timemodified = time() - self::TIME_TO_CACHE;
return $DB->get_field_select('question_response_analysis', 'timemodified',
'hashcode = ? AND whichtries = ? AND questionid = ? AND timemodified > ?',
array($qubaids->get_hash_code(), $whichtries, $this->questiondata->id, $timemodified),
IGNORE_MULTIPLE);
return $DB->get_field('question_response_analysis', 'MAX(timemodified)',
['hashcode' => $qubaids->get_hash_code(), 'whichtries' => $whichtries,
'questionid' => $this->questiondata->id]);
}
}

View File

@ -111,8 +111,9 @@ class analysis_for_actual_response {
* @param int $variantno which variant.
* @param string $subpartid which sub part is this actual response in?
* @param string $responseclassid which response class is this actual response in?
* @param int|null $calculationtime time when the analysis was done. (Defaults to time()).
*/
public function cache($qubaids, $whichtries, $questionid, $variantno, $subpartid, $responseclassid) {
public function cache($qubaids, $whichtries, $questionid, $variantno, $subpartid, $responseclassid, $calculationtime = null) {
global $DB;
$row = new \stdClass();
$row->hashcode = $qubaids->get_hash_code();
@ -127,7 +128,7 @@ class analysis_for_actual_response {
}
$row->response = $this->response;
$row->credit = $this->fraction;
$row->timemodified = time();
$row->timemodified = $calculationtime ? $calculationtime : time();
$analysisid = $DB->insert_record('question_response_analysis', $row);
if ($whichtries === \question_attempt::ALL_TRIES) {
foreach ($this->trycount as $try => $count) {

View File

@ -103,11 +103,13 @@ class analysis_for_class {
* @param int $questionid which question.
* @param int $variantno which variant.
* @param string $subpartid which sub part.
* @param int|null $calculationtime time when the analysis was done. (Defaults to time()).
*/
public function cache($qubaids, $whichtries, $questionid, $variantno, $subpartid) {
public function cache($qubaids, $whichtries, $questionid, $variantno, $subpartid, $calculationtime = null) {
foreach ($this->get_responses() as $response) {
$analysisforactualresponse = $this->get_response($response);
$analysisforactualresponse->cache($qubaids, $whichtries, $questionid, $variantno, $subpartid, $this->responseclassid);
$analysisforactualresponse->cache($qubaids, $whichtries, $questionid, $variantno, $subpartid,
$this->responseclassid, $calculationtime);
}
}

View File

@ -196,17 +196,36 @@ class analysis_for_question {
}
/**
* Save the analysis to the DB, first cleaning up any old ones.
*
* @param \qubaid_condition $qubaids which question usages have been analysed.
* @param string $whichtries which tries have been analysed?
* @param int $questionid which question.
* @param int|null $calculationtime time when the analysis was done. (Defaults to time()).
*/
public function cache($qubaids, $whichtries, $questionid) {
public function cache($qubaids, $whichtries, $questionid, $calculationtime = null) {
global $DB;
$transaction = $DB->start_delegated_transaction();
$DB->delete_records_select('question_response_count',
'analysisid IN (
SELECT id
FROM {question_response_analysis}
WHERE hashcode= ? AND whichtries = ? AND questionid = ?
)', [$qubaids->get_hash_code(), $whichtries, $questionid]);
$DB->delete_records('question_response_analysis',
['hashcode' => $qubaids->get_hash_code(), 'whichtries' => $whichtries, 'questionid' => $questionid]);
foreach ($this->get_variant_nos() as $variantno) {
foreach ($this->get_subpart_ids($variantno) as $subpartid) {
$analysisforsubpart = $this->get_analysis_for_subpart($variantno, $subpartid);
$analysisforsubpart->cache($qubaids, $whichtries, $questionid, $variantno, $subpartid);
$analysisforsubpart->cache($qubaids, $whichtries, $questionid, $variantno, $subpartid, $calculationtime);
}
}
$transaction->allow_commit();
}
/**

View File

@ -119,11 +119,12 @@ class analysis_for_subpart {
* @param int $questionid which question.
* @param int $variantno which variant.
* @param string $subpartid which sub part.
* @param int|null $calculationtime time when the analysis was done. (Defaults to time()).
*/
public function cache($qubaids, $whichtries, $questionid, $variantno, $subpartid) {
public function cache($qubaids, $whichtries, $questionid, $variantno, $subpartid, $calculationtime = null) {
foreach ($this->get_response_class_ids() as $responseclassid) {
$analysisforclass = $this->get_response_class($responseclassid);
$analysisforclass->cache($qubaids, $whichtries, $questionid, $variantno, $subpartid, $responseclassid);
$analysisforclass->cache($qubaids, $whichtries, $questionid, $variantno, $subpartid, $calculationtime);
}
}

View File

@ -10,11 +10,25 @@ This files describes API changes for code that uses the question API.
$question = $questiongenerator->update_question($question, ...);
Also, the $question object returned now has fields questionbankentryid, versionid, version and status.
2) question_stats_cleanup_task has been removed. It is no longer required. Instead,
older statistics are deleted whenever a new set are calculated for a particular quiz.
3) In the past, the methods get_last_calculated_time() and get_cached() of \core_question\statistics\responses\analyser
and \core_question\statistics\questions\all_calculated_for_qubaid_condition
only returned the pre-computed statistics if they were computed less than 15 minutes ago. Now, they will
always return any computed statistics that exist. The constants TIME_TO_CACHE in those classes
will be deprecated in Moodle 4.3.
4) The cache() methods of classes analysis_for_question, analysis_for_subpart, analysis_for_class
and analysis_for_actual_response now take an optional $calculationtime, which is used the time
stored in the database. If not given, time() is used.
=== 4.1 ===
1) get_bulk_action_key() in core_question\local\bank\bulk_action_base class is deprecated and renamed to get_key().
=== 4.0.5 ===
1) Question bank plugins can now define more than one bulk action. Therefore, plugin_features_base::get_bulk_actions has been