Merge pull request #80 from tarosky/issue/79

Handle orphaned cron jobs

Close #79.

Author: harai

build/build-image

@@ -4,4 +4,4 @@ set -eu
 
 # shellcheck disable=SC1091
 source build/envs.sh
-exec docker-compose -f build/docker-compose.yml build
+exec docker compose -f build/docker-compose.yml build

build/docker-compose.yml

@@ -1,4 +1,3 @@
-version: '3.9'
 services:
   wordpress:
     build:

build/run-package

@@ -4,4 +4,4 @@ set -eu
 
 # shellcheck disable=SC1091
 source build/envs.sh
-exec docker-compose -f build/docker-compose.yml run --rm cavalcade-runner package
+exec docker compose -f build/docker-compose.yml run --rm cavalcade-runner package

build/run-test

@@ -2,8 +2,8 @@
 
 set -eu
 
-trap 'docker-compose -f build/docker-compose.yml down' EXIT
+trap 'docker compose -f build/docker-compose.yml down' EXIT
 
 # shellcheck disable=SC1091
 source build/envs.sh
-docker-compose -f build/docker-compose.yml run --rm wordpress
+docker compose -f build/docker-compose.yml run --rm wordpress

build/run-validate

@@ -4,4 +4,4 @@ set -eu
 
 # shellcheck disable=SC1091
 source build/envs.sh
-exec docker-compose -f build/docker-compose.yml run --rm cavalcade-runner validate
+exec docker compose -f build/docker-compose.yml run --rm cavalcade-runner validate

build/wordpress/entrypoint

@@ -48,3 +48,4 @@ cat "$ctrl_done_fifo" > /dev/null
 mv work/log work/test-logs/entrypoint
 
 vendor/bin/phpunit --log-junit work/test-result.xml
+# vendor/bin/phpunit --log-junit work/test-result.xml --filter test_ms_single_event_without_siteid

inc/class-job.php

@@ -93,11 +93,19 @@ function ($stmt) {
                 $stmt->execute();
 
                 $data = $stmt->fetch(PDO::FETCH_OBJ);
+                if ($data === false) {
+                    return false;
+                }
+
                 return $data->domain . $data->path;
             },
             true,
         );
 
+        if ($res === false) {
+            throw new SiteNotFoundException('site id not found');
+        }
+
         $this->log->debug('site url', ['siteurl' => $res] + $this->log_values());
 
         return $res;
@@ -120,11 +128,14 @@ public function acquire_lock()
         $nextrun = DateTime::createFromFormat('Y-m-d H:i:s', $this->nextrun, new DateTimeZone('UTC'));
         $this->execution_delay = $started_at->getTimestamp() - $nextrun->getTimestamp();
 
+        $this->status = 'running';
+
         $res = $this->db->prepare_query(
             "UPDATE `$this->table`
-             SET `status` = 'running', `started_at` = :started_at
+             SET `status` = :status, `started_at` = :started_at
              WHERE `status` = 'waiting' AND id = :id",
             function ($stmt) {
+                $stmt->bindValue(':status', $this->status);
                 $stmt->bindValue(':id', $this->id, PDO::PARAM_INT);
                 $stmt->bindValue(':started_at', $this->started_at);
                 $stmt->execute();
@@ -147,12 +158,17 @@ public function cancel_lock()
     {
         $this->log->debug('canceling lock', $this->log_values());
 
+        $this->status = 'waiting';
+        $this->started_at = null;
+
         $this->db->prepare_query(
             "UPDATE `$this->table`
-             SET `status` = 'waiting', `started_at` = NULL
+             SET `status` = :status, `started_at` = :started_at
              WHERE id = :id",
             function ($stmt) {
+                $stmt->bindValue(':status', $this->status);
                 $stmt->bindValue(':id', $this->id, PDO::PARAM_INT);
+                $stmt->bindValue(':started_at', $this->started_at);
                 $stmt->execute();
             },
             true,
@@ -172,11 +188,14 @@ public function mark_done()
             $this->reschedule();
             $this->log->debug('rescheduled', $this->log_values());
         } else {
+            $this->status = 'done';
+
             $this->db->prepare_query(
                 "UPDATE `$this->table`
-                 SET `status` = 'done', `finished_at` = :finished_at
+                 SET `status` = :status, `finished_at` = :finished_at
                  WHERE `id` = :id",
                 function ($stmt) {
+                    $stmt->bindValue(':status', $this->status);
                     $stmt->bindValue(':id', $this->id, PDO::PARAM_INT);
                     $stmt->bindValue(':finished_at', $this->finished_at);
                     $stmt->execute();

inc/class-runner.php

@@ -604,6 +604,10 @@ protected function run_job($job)
                 @fclose($pipes[2]);
                 throw new Exception('failed to set stdout to non-blocking');
             }
+        } catch (SiteNotFoundException $e) {
+            $job->mark_done();
+            $this->log->error_app('job failed; failed to get site id for job', $job->log_values_full());
+            return;
         } catch (Exception $e) {
             $this->log->error('exception during starting job', [
                 'ex_message' => $e->getMessage(),

inc/class-sitenotfoundexception.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace HM\Cavalcade\Runner;
+
+use RuntimeException;
+
+class SiteNotFoundException extends RuntimeException
+{
+    public function __construct($message)
+    {
+        parent::__construct($message);
+    }
+}

test/test-job.php

@@ -74,6 +74,58 @@ public function test_single_event()
         $this->assertNull($this->get_job(JOB));
     }
 
+    public function test_ms_single_event_without_siteid()
+    {
+        global $wpdb;
+
+        if (getenv('WP_MULTISITE') !== '1') {
+            $this->markTestSkipped('Not multisite');
+            return;
+        }
+
+        $pre_time = time();
+        wp_schedule_single_event($pre_time, JOB, [__FUNCTION__]);
+        $job = $this->get_job(JOB);
+        $wpdb->query("UPDATE `$this->table` SET `site` = 9999 WHERE `id` = $job->id");
+        $this->assertEquals(STATUS_WAITING, $job->status);
+        $this->assertNull($job->started_at);
+        $this->assertNull($job->finished_at);
+
+        sleep(3);
+
+        $post_time = time();
+        $job = $this->get_job(JOB);
+        $this->assertEquals(STATUS_DONE, $job->status);
+        $this->assertBetweenWeak($pre_time, $post_time, self::as_epoch($job->started_at));
+        $this->assertBetweenWeak($pre_time, $post_time, self::as_epoch($job->finished_at));
+        $this->assertEquals(EMPTY_DELETED_AT, $job->deleted_at);
+
+        $this->assertBetweenWeak(
+            strtotime('-1 minutes'),
+            time(),
+            self::as_epoch($job->nextrun),
+        );
+
+        sleep(3);
+
+        $job = $this->get_job(JOB);
+        $this->assertEquals(STATUS_DONE, $job->status);
+
+        sleep(6);
+
+        $this->assertNull($this->get_job(JOB));
+
+        $log_lines = explode("\n", file_get_contents(RUNNER_LOG));
+        foreach ($log_lines as $line) {
+            if (strstr($line, '"ERROR"') !== false) {
+                $this->assertStringContainsString('"job failed; failed to get site id for job"', $line);
+                $this->assertStringContainsString('"app"', $line);
+                return;
+            }
+        }
+        $this->fail();
+    }
+
     public function test_deleted_event()
     {
         global $wpdb;