From: Alexander Ebert Date: Mon, 30 Jul 2018 13:34:36 +0000 (+0200) Subject: Reset previously failed cronjobs before executing them (#2726) X-Git-Tag: 3.1.5~22 X-Git-Url: https://git.stricted.de/?a=commitdiff_plain;h=e620364aeae356abf0d762f8d4d0be97e56f3294;p=GitHub%2FWoltLab%2FWCF.git Reset previously failed cronjobs before executing them (#2726) * Reset previously failed cronjobs before executing them * Missing method call to reset the failed cronjobs --- diff --git a/wcfsetup/install/files/lib/system/cronjob/CronjobScheduler.class.php b/wcfsetup/install/files/lib/system/cronjob/CronjobScheduler.class.php index a29eaffb28..f6c3ff0db6 100644 --- a/wcfsetup/install/files/lib/system/cronjob/CronjobScheduler.class.php +++ b/wcfsetup/install/files/lib/system/cronjob/CronjobScheduler.class.php @@ -46,6 +46,8 @@ class CronjobScheduler extends SingletonFactory { return; } + $this->resetFailedCronjobs(); + // get outstanding cronjobs $this->loadCronjobs(); @@ -105,6 +107,56 @@ class CronjobScheduler extends SingletonFactory { return $this->cache['nextExec']; } + /** + * Resets any cronjobs that have previously failed to execute. Cronjobs that have failed too often will + * be disabled automatically. + */ + protected function resetFailedCronjobs() { + WCF::getDB()->beginTransaction(); + /** @noinspection PhpUnusedLocalVariableInspection */ + $committed = false; + try { + $sql = "SELECT * + FROM wcf" . WCF_N . "_cronjob + WHERE isDisabled = ? + AND failCount < ? + AND afterNextExec <= ? + FOR UPDATE"; + $statement = WCF::getDB()->prepareStatement($sql); + $statement->execute([ + 0, + Cronjob::MAX_FAIL_COUNT, + TIME_NOW + ]); + /** @var Cronjob $cronjob */ + while ($cronjob = $statement->fetchObject(Cronjob::class)) { + $failCount = $cronjob->failCount + 1; + $data['failCount'] = $failCount; + + if ($failCount == Cronjob::MAX_FAIL_COUNT) { + $data['isDisabled'] = 1; + $data['state'] = Cronjob::READY; + } + + // Schedule the cronjob for execution at the next regular execution date. The previous + // implementation was executing the cronjob immediately, which may be undesirable if + // the cronjob is expected to be executed in a specific time window only. + $data['nextExec'] = $cronjob->getNextExec(TIME_NOW); + $data['afterNextExec'] = $cronjob->getNextExec($data['nextExec']); + + (new CronjobEditor($cronjob))->update($data); + } + + WCF::getDB()->commitTransaction(); + $committed = true; + } + finally { + if (!$committed) { + WCF::getDB()->rollBackTransaction(); + } + } + } + /** * Loads outstanding cronjobs. */ @@ -114,46 +166,24 @@ class CronjobScheduler extends SingletonFactory { $committed = false; try { $sql = "SELECT * - FROM wcf" . WCF_N . "_cronjob cronjob - WHERE (cronjob.nextExec <= ? OR cronjob.afterNextExec <= ?) - AND cronjob.isDisabled = ? - AND cronjob.failCount < ? + FROM wcf" . WCF_N . "_cronjob + WHERE isDisabled = ? + AND state = ? + AND nextExec <= ? FOR UPDATE"; $statement = WCF::getDB()->prepareStatement($sql); - $statement->execute([TIME_NOW, TIME_NOW, 0, Cronjob::MAX_FAIL_COUNT]); - while ($row = $statement->fetchArray()) { - $cronjob = new Cronjob(null, $row); + $statement->execute([ + 0, + Cronjob::READY, + TIME_NOW + ]); + while ($cronjob = $statement->fetchObject(Cronjob::class)) { $cronjobEditor = new CronjobEditor($cronjob); - $executeCronjob = true; - $data = ['state' => Cronjob::PENDING]; + // Mark the cronjob as pending to prevent concurrent requests from executing it. + $cronjobEditor->update(['state' => Cronjob::PENDING]); - // reset cronjob if it got stuck before and afterNextExec is in the past - if ($cronjobEditor->afterNextExec <= TIME_NOW) { - if ($cronjobEditor->state == Cronjob::EXECUTING) { - $failCount = $cronjobEditor->failCount + 1; - $data['failCount'] = $failCount; - - // disable cronjob - if ($failCount == Cronjob::MAX_FAIL_COUNT) { - $data['isDisabled'] = 1; - $data['state'] = 0; - $executeCronjob = false; - } - } - } // ignore cronjobs which seem to be running - else { - if ($cronjobEditor->nextExec <= TIME_NOW && $cronjobEditor->state != Cronjob::READY) { - $executeCronjob = false; - } - } - - // mark cronjob as pending, preventing parallel execution - $cronjobEditor->update($data); - - if ($executeCronjob) { - $this->cronjobEditors[] = $cronjobEditor; - } + $this->cronjobEditors[] = $cronjobEditor; } WCF::getDB()->commitTransaction(); $committed = true;