diff --git a/admin/tool/log/store/legacy/classes/task/cleanup_task.php b/admin/tool/log/store/legacy/classes/task/cleanup_task.php index 474604b4a74..4d0e62aa926 100644 --- a/admin/tool/log/store/legacy/classes/task/cleanup_task.php +++ b/admin/tool/log/store/legacy/classes/task/cleanup_task.php @@ -44,11 +44,26 @@ class cleanup_task extends \core\task\scheduled_task { public function execute() { global $CFG, $DB; - // Delete old logs to save space (this might need a timer to slow it down...). - if (!empty($CFG->loglifetime)) { // Value in days. - $loglifetime = time(0) - ($CFG->loglifetime * 3600 * 24); - $DB->delete_records_select("log", "time < ?", array($loglifetime)); - mtrace(" Deleted old legacy log records"); + if (empty($CFG->loglifetime)) { + return; } + + $loglifetime = time() - ($CFG->loglifetime * 3600 * 24); // Value in days. + $lifetimep = array($loglifetime); + $start = time(); + + while ($min = $DB->get_field_select("log", "MIN(time)", "time < ?", $lifetimep)) { + // Break this down into chunks to avoid transaction for too long and generally thrashing database. + // Experiments suggest deleting one day takes up to a few seconds; probably a reasonable chunk size usually. + // If the cleanup has just been enabled, it might take e.g a month to clean the years of logs. + $params = array(min($min + 3600 * 24, $loglifetime)); + $DB->delete_records_select("log", "time < ?", $params); + if (time() > $start + 300) { + // Do not churn on log deletion for too long each run. + break; + } + } + + mtrace(" Deleted old legacy log records"); } } diff --git a/admin/tool/log/store/legacy/tests/store_test.php b/admin/tool/log/store/legacy/tests/store_test.php index 7fb6353b64d..c77354496fd 100644 --- a/admin/tool/log/store/legacy/tests/store_test.php +++ b/admin/tool/log/store/legacy/tests/store_test.php @@ -273,4 +273,39 @@ class logstore_legacy_store_testcase extends advanced_testcase { $this->assertContains($expectedreport, $reports); } } + + /** + * Test that the legacy log cleanup works correctly. + */ + public function test_cleanup_task() { + global $DB; + + $this->resetAfterTest(); + + // Create some records spread over various days; test multiple iterations in cleanup. + $record = (object) array('time' => time()); + $DB->insert_record('log', $record); + $record->time -= 3600 * 24 * 30; + $DB->insert_record('log', $record); + $record->time -= 3600 * 24 * 30; + $DB->insert_record('log', $record); + $record->time -= 3600 * 24 * 30; + $DB->insert_record('log', $record); + $this->assertEquals(4, $DB->count_records('log')); + + // Remove all logs before "today". + set_config('loglifetime', 1); + + try { + ob_start(); + $clean = new \logstore_legacy\task\cleanup_task(); + $clean->execute(); + ob_end_clean(); + } catch (Exception $e) { + ob_end_clean(); + throw $e; + } + + $this->assertEquals(1, $DB->count_records('log')); + } } diff --git a/admin/tool/log/store/standard/classes/task/cleanup_task.php b/admin/tool/log/store/standard/classes/task/cleanup_task.php index cc27b1fa091..2d6842318cc 100644 --- a/admin/tool/log/store/standard/classes/task/cleanup_task.php +++ b/admin/tool/log/store/standard/classes/task/cleanup_task.php @@ -43,12 +43,29 @@ class cleanup_task extends \core\task\scheduled_task { */ public function execute() { global $DB; + $loglifetime = (int)get_config('logstore_standard', 'loglifetime'); - if ($loglifetime > 0) { - $loglifetime = time() - ($loglifetime * 3600 * 24); // Value in days. - $DB->delete_records_select("logstore_standard_log", "timecreated < ?", array($loglifetime)); - mtrace(" Deleted old log records from standard store."); + if (empty($loglifetime) || $loglifetime < 0) { + return; } + + $loglifetime = time() - ($loglifetime * 3600 * 24); // Value in days. + $lifetimep = array($loglifetime); + $start = time(); + + while ($min = $DB->get_field_select("logstore_standard_log", "MIN(timecreated)", "timecreated < ?", $lifetimep)) { + // Break this down into chunks to avoid transaction for too long and generally thrashing database. + // Experiments suggest deleting one day takes up to a few seconds; probably a reasonable chunk size usually. + // If the cleanup has just been enabled, it might take e.g a month to clean the years of logs. + $params = array(min($min + 3600 * 24, $loglifetime)); + $DB->delete_records_select("logstore_standard_log", "timecreated < ?", $params); + if (time() > $start + 300) { + // Do not churn on log deletion for too long each run. + break; + } + } + + mtrace(" Deleted old log records from standard store."); } } diff --git a/admin/tool/log/store/standard/tests/store_test.php b/admin/tool/log/store/standard/tests/store_test.php index 730714dd774..23315565dbb 100644 --- a/admin/tool/log/store/standard/tests/store_test.php +++ b/admin/tool/log/store/standard/tests/store_test.php @@ -276,4 +276,46 @@ class logstore_standard_store_testcase extends advanced_testcase { get_log_manager(true); } + /** + * Test that the standard log cleanup works correctly. + */ + public function test_cleanup_task() { + global $DB; + + $this->resetAfterTest(); + + // Create some records spread over various days; test multiple iterations in cleanup. + $ctx = context_course::instance(1); + $record = (object) array( + 'edulevel' => 0, + 'contextid' => $ctx->id, + 'contextlevel' => $ctx->contextlevel, + 'contextinstanceid' => $ctx->instanceid, + 'userid' => 1, + 'timecreated' => time(), + ); + $DB->insert_record('logstore_standard_log', $record); + $record->timecreated -= 3600 * 24 * 30; + $DB->insert_record('logstore_standard_log', $record); + $record->timecreated -= 3600 * 24 * 30; + $DB->insert_record('logstore_standard_log', $record); + $record->timecreated -= 3600 * 24 * 30; + $DB->insert_record('logstore_standard_log', $record); + $this->assertEquals(4, $DB->count_records('logstore_standard_log')); + + // Remove all logs before "today". + set_config('loglifetime', 1, 'logstore_standard'); + + try { + ob_start(); + $clean = new \logstore_standard\task\cleanup_task(); + $clean->execute(); + ob_end_clean(); + } catch (Exception $e) { + ob_end_clean(); + throw $e; + } + + $this->assertEquals(1, $DB->count_records('logstore_standard_log')); + } }