LazyCron writes to modules table on every page view even when no jobs are due (and blanks lastResult)

[adrianbj]

Short description

LazyCron::execute() calls saveResult() unconditionally at the end of every run, even when no cron jobs were due. That does an UPDATE modules SET data=… on every normal page view, and it also blanks lastResult, so the module's "Last results" config panel almost never shows anything.

Details

LazyCron::init() hooks ProcessPageView::finished, so execute() runs after every normal-response page view. At the end of execute() (wire/modules/System/LazyCron/LazyCron.module:288-294):

		if($writeFile) {
			$files->filePutContents($filename, implode("\n", $times), LOCK_EX);
		}
		
		$this->removeLockfile();
		
		$this->saveResult(implode("\n", $results), false);

The cache-file write is correctly guarded by $writeFile, which is only set when a job actually ran. saveResult() has no equivalent guard.

saveResult() (:444) then does:

		$data = $modules->getConfig($this);
		$data['lastResult'] = $result;
		$data['lastTime'] = time();
		$modules->saveConfig($this, $data);

And Modules::saveConfig() (wire/core/Modules/ModulesConfigs.php:512) has no change detection — it always issues the UPDATE:

		$query = $database->prepare("UPDATE modules SET data=:data WHERE id=:id", "modules.saveConfig($moduleName)");

Since lastTime is time(), the data always differs anyway, so change detection wouldn't help here.

Two consequences

1. A write on every page view. Every uncached front-end page view issues one UPDATE modules SET data=… that serves no purpose. On a production database with log_bin=1 and sync_binlog=1 (the RDS default), each is a durable commit — measured at ~7ms on the site where I found this. On a steady-state front-end page view it was 1 of only 2 writes in the entire request.

2. lastResult is wiped by no-op runs. When nothing is due, $results is empty, so lastResult is overwritten with ''. The config screen (:474-489) builds its "Last results" panel from lastTime and lastResult:

		foreach(explode("\n", (string) $this->lastResult) as $line) {
			if(!empty($line)) $a[] = $line;
		}

Because the most recent write is almost always a no-op page view, the panel effectively always shows just "Last run at: a few seconds ago" with no results. You can't see what LazyCron actually did.

Steps to reproduce

On any site with LazyCron installed, count saveConfig(LazyCron) queries per request. Add to site/config.php:

$config->dbQueryLogMax = 100000;

and to site/ready.php:

$this->wire()->database->queryLog(1);
register_shutdown_function(function() {
	$log = \ProcessWire\wire()->database->queryLog();
	if(!is_array($log)) return;
	$lc = 0;
	foreach($log as $k => $sql) {
		if($k !== 'error' && stripos($sql, 'saveConfig(LazyCron)') !== false) $lc++;
	}
	@file_put_contents('/tmp/lc.log', sprintf("%-14s lazycron_saveConfig=%d\n", $_SERVER['REQUEST_URI'] ?? 'cli', $lc), FILE_APPEND);
});

Then load the site's front page a few times. Observed:

/              lazycron_saveConfig=1
/              lazycron_saveConfig=1
/              lazycron_saveConfig=1

Expected: 0 on views where no cron interval was due.

Suggested fix

Guard the call the same way the cache-file write is guarded:

- 		$this->saveResult(implode("\n", $results), false);
+ 		if(count($results)) $this->saveResult(implode("\n", $results), false);

With that applied, the same test gives:

/              lazycron_saveConfig=1     <- cold (LazyCron.cache deleted, all jobs run)
/              lazycron_saveConfig=0
/              lazycron_saveConfig=0
/              lazycron_saveConfig=0

So the write still happens whenever a job runs, and not otherwise. This also fixes the lastResult blanking, since it can then only be set by a run that produced results.

Note this changes the meaning of lastTime from "when LazyCron last checked" to "when a job last ran". Given the config screen labels it "Last run at:" and displays it alongside lastResult, the latter looks like the intended meaning — but it is a behaviour change, so worth a deliberate decision.

The CLI saveResult() calls at :214 and :229 are already if($this->cli) guarded and are unaffected.

Environment

• ProcessWire 3.0.270, LazyCron version 104
• Reproduced on a local MariaDB 12.3 install; originally found on a production site using MySQL 8 on AWS RDS, where the durable-commit cost made it visible.