Issue #3397: always delete config_log records in cleanup task

[Guillermo Gomez Arias (guillogo)]

Fixes #3397

Problem

The adhoc task \local_o365\task\deleteinvalidconfiglog does not delete the config_log records it is supposed to clean up, but it still queues a new copy of itself after every run, so the number of adhoc tasks keeps growing.

The deletion of the config_log records is nested inside the if (!empty($logidstodelete)) block, so those records are removed only when a matching record still exists in logstore_standard_log.

That match is not guaranteed. Old log records are removed every night by \logstore_standard\task\cleanup_task, according to the loglifetime setting, while config_log is never purged by Moodle. So the old config_log records stay while their log entries are already gone.

When there is no match, nothing is deleted. The task then counts the records again, sees they are still there, sets $hasmore = true and queues another task, which reads the same records (ORDER BY id ASC) and does the same thing.

On our production site this created around 6400 adhoc tasks, each running heavy COUNT queries on mdl_config_log. See #3397 for the full report and the cron output.

Change

Close the if block right after the logstore deletion, so the config_log records are always deleted for every chunk. The execution time check and the pause between chunks move out of the block as well, so they also apply to every chunk.

This is the smallest possible change: no new variables, no new conditions, and the logstore deletion logic is untouched.

Introduced in

Commit 807457d (PR #3067). Before that commit the config_log deletion was in its own loop and was unconditional, so the task always made progress.

Testing

1. Make sure there are config_log records for local_o365 whose matching entries in logstore_standard_log no longer exist (for example after the log cleanup task has run).
2. Run \local_o365\task\checkinvalidconfiglog, then run the queued adhoc task.
3. Before the fix, the output shows ... N records remaining with the same number as ... Found N config_log records, and a new adhoc task is queued on every run.
4. After the fix, the output shows ... Deleted 500 config_log records for chunk and the remaining count goes down on each run, until All invalid config log records have been deleted. is reported and no new task is queued.

Note

The issue also mentions two other points that are not addressed here, to keep this PR minimal: manager::queue_adhoc_task() is called without the $checkforexisting parameter, and checkinvalidconfiglog.php only recognises existing tasks with faildelay == 0. Happy to send a follow-up PR for those if you want.

[Guillermo Gomez Arias (guillogo)]

@microsoft-github-policy-service agree company="Catalyst IT Canada"