Merge pull request #88 from lilt/3.x-rework-on-queues-retry

Rework of queue retry logic

CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## 3.4.2 - 2023-02-21
+### Changed
+- Retry logic for queues
+
 ## 3.4.1 - 2023-02-13
 ### Changed 
 - Queues priority decreased for sending jobs to 1024 and receiving to 2048

composer.json

@@ -2,7 +2,7 @@
   "name": "lilt/craft-lilt-plugin",
   "description": "The Lilt plugin makes it easy for you to send content to Lilt for translation right from within Craft CMS.",
   "type": "craft-plugin",
-  "version": "3.4.1",
+  "version": "3.4.2",
   "keywords": [
     "craft",
     "cms",

src/elements/Job.php

@@ -187,7 +187,11 @@ public static function hasStatuses(): bool
     public function afterDelete(): bool
     {
         JobRecord::deleteAll(['id' => $this->id]);
+
         parent::afterDelete();
+
+        Craft::$app->elements->invalidateCachesForElementType(self::class);
+
         return true;
     }
 

src/modules/AbstractRetryJob.php

@@ -0,0 +1,34 @@
+<?php
+
+/**
+ * @link      https://github.com/lilt
+ * @copyright Copyright (c) 2023 Lilt Devs
+ */
+
+declare(strict_types=1);
+
+namespace lilthq\craftliltplugin\modules;
+
+use craft\queue\BaseJob;
+
+abstract class AbstractRetryJob extends BaseJob
+{
+    /**
+     * @var int
+     */
+    public $jobId;
+
+    /**
+     * @var int
+     */
+    public $attempt = 0;
+
+    /**
+     *
+     * Is current job is eligible for retry
+     *
+     * @return bool
+     */
+    abstract public function canRetry(): bool;
+    abstract public function getRetryJob(): BaseJob;
+}

src/modules/FetchJobStatusFromConnector.php

@@ -16,21 +16,15 @@
 use lilthq\craftliltplugin\Craftliltplugin;
 use lilthq\craftliltplugin\elements\Job;
 use lilthq\craftliltplugin\records\JobRecord;
-use yii\queue\RetryableJobInterface;
 
-class FetchJobStatusFromConnector extends BaseJob implements RetryableJobInterface
+class FetchJobStatusFromConnector extends AbstractRetryJob
 {
     public const DELAY_IN_SECONDS = 5 * 60;
     public const PRIORITY = 1024;
     public const TTR = 60 * 30;
 
     private const RETRY_COUNT = 3;
 
-    /**
-     * @var int $jobId
-     */
-    public $jobId;
-
     /**
      * @var int $liltJobId
      */
@@ -195,14 +189,18 @@ private function markAsDone($queue): void
         );
     }
 
-    public function getTtr(): int
+    public function canRetry(): bool
     {
-        return self::TTR;
+        return $this->attempt < self::RETRY_COUNT;
     }
 
-    public function canRetry($attempt, $error): bool
+    public function getRetryJob(): BaseJob
     {
-        return $attempt < self::RETRY_COUNT;
+        return new self([
+            'jobId' => $this->jobId,
+            'liltJobId' => $this->liltJobId,
+            'attempt' => $this->attempt + 1
+        ]);
     }
 
     public static function getDelay(): int

src/modules/FetchTranslationFromConnector.php

@@ -21,9 +21,8 @@
 use lilthq\craftliltplugin\parameters\CraftliltpluginParameters;
 use lilthq\craftliltplugin\records\TranslationRecord;
 use Throwable;
-use yii\queue\RetryableJobInterface;
 
-class FetchTranslationFromConnector extends BaseJob implements RetryableJobInterface
+class FetchTranslationFromConnector extends AbstractRetryJob
 {
     public const DELAY_IN_SECONDS_INSTANT = 10;
     public const DELAY_IN_SECONDS_VERIFIED = 60 * 5;
@@ -32,11 +31,6 @@ class FetchTranslationFromConnector extends BaseJob implements RetryableJobInter
 
     private const RETRY_COUNT = 3;
 
-    /**
-     * @var int $jobId
-     */
-    public $jobId;
-
     /**
      * @var int $liltJobId
      */
@@ -191,14 +185,19 @@ private function markAsDone($queue): void
         );
     }
 
-    public function getTtr(): int
+    public function canRetry(): bool
     {
-        return self::TTR;
+        return $this->attempt < self::RETRY_COUNT;
     }
 
-    public function canRetry($attempt, $error): bool
+    public function getRetryJob(): BaseJob
     {
-        return $attempt < self::RETRY_COUNT;
+        return new self([
+            'jobId' => $this->jobId,
+            'liltJobId' => $this->liltJobId,
+            'translationId' => $this->translationId,
+            'attempt' => $this->attempt + 1
+        ]);
     }
 
     /**
@@ -233,7 +232,7 @@ private function isTranslationFinished($job, TranslationResponse $translationFro
             ) === TranslationResponse::STATUS_EXPORT_COMPLETE);
     }
 
-    public static function getDelay(string $flow): int
+    public static function getDelay(string $flow = CraftliltpluginParameters::TRANSLATION_WORKFLOW_INSTANT): int
     {
         $envDelay = getenv('CRAFT_LILT_PLUGIN_QUEUE_DELAY_IN_SECONDS');
         if (!empty($envDelay) || $envDelay === '0') {

src/modules/SendJobToConnector.php

@@ -17,21 +17,15 @@
 use lilthq\craftliltplugin\elements\Job;
 use lilthq\craftliltplugin\records\JobRecord;
 use Throwable;
-use yii\queue\RetryableJobInterface;
 
-class SendJobToConnector extends BaseJob implements RetryableJobInterface
+class SendJobToConnector extends AbstractRetryJob
 {
     public const DELAY_IN_SECONDS = 60;
     public const PRIORITY = 1024;
     public const TTR = 60 * 30;
 
     private const RETRY_COUNT = 3;
 
-    /**
-     * @var int $jobId
-     */
-    public $jobId;
-
     /**
      * @inheritdoc
      *
@@ -106,23 +100,26 @@ private function markAsDone($queue): void
         );
     }
 
-    public function getTtr(): int
-    {
-        return self::TTR;
-    }
-
-    public function canRetry($attempt, $error): bool
-    {
-        return $attempt < self::RETRY_COUNT;
-    }
-
     public static function getDelay(): int
     {
         $envDelay = getenv('CRAFT_LILT_PLUGIN_QUEUE_DELAY_IN_SECONDS');
         if (!empty($envDelay) || $envDelay === '0') {
-            return (int) $envDelay;
+            return (int)$envDelay;
         }
 
         return self::DELAY_IN_SECONDS;
     }
+
+    public function canRetry(): bool
+    {
+        return $this->attempt < self::RETRY_COUNT;
+    }
+
+    public function getRetryJob(): BaseJob
+    {
+        return new self([
+            'jobId' => $this->jobId,
+            'attempt' => $this->attempt + 1
+        ]);
+    }
 }

src/services/listeners/AfterErrorListener.php

@@ -53,27 +53,6 @@ private function isEventEligible(Event $event): bool
             return false;
         }
 
-        if ($event->retry) {
-            // we only wait for job which will be not retried anymore
-
-            Craftliltplugin::getInstance()->jobLogsRepository->create(
-                $event->job->jobId,
-                Craft::$app->getUser()->getId(),
-                substr(
-                    sprintf(
-                        'Job %s failed on %d attempt. Error message: %s',
-                        get_class($event->job),
-                        $event->attempt,
-                        $event->error->getMessage()
-                    ),
-                    0,
-                    255
-                )
-            );
-
-            return false;
-        }
-
         $jobClass = get_class($event->job);
 
         return in_array($jobClass, self::SUPPORTED_JOBS);
@@ -85,25 +64,30 @@ public function __invoke(Event $event): Event
             return $event;
         }
 
-        $jobRecord = JobRecord::findOne(['id' => $event->job->jobId]);
+        /**
+         * @var FetchTranslationFromConnector|FetchJobStatusFromConnector|SendJobToConnector $queueJob
+         */
+        $queueJob = $event->job;
+
+        $jobRecord = JobRecord::findOne(['id' => $queueJob->jobId]);
+
+        Craftliltplugin::getInstance()->jobLogsRepository->create(
+            $jobRecord->id,
+            Craft::$app->getUser()->getId(),
+            substr(
+                sprintf(
+                    'Job failed after %d attempt(s). Error message: %s',
+                    $queueJob->attempt,
+                    $event->error->getMessage()
+                ),
+                0,
+                255
+            )
+        );
 
-        if ($jobRecord !== null) {
+        if (!$queueJob->canRetry()) {
             $jobRecord->status = Job::STATUS_FAILED;
 
-            Craftliltplugin::getInstance()->jobLogsRepository->create(
-                $jobRecord->id,
-                Craft::$app->getUser()->getId(),
-                substr(
-                    sprintf(
-                        'Job failed after %d attempt(s). Error message: %s',
-                        $event->attempt,
-                        $event->error->getMessage()
-                    ),
-                    0,
-                    255
-                )
-            );
-
             $jobRecord->save();
 
             TranslationRecord::updateAll(
@@ -113,12 +97,26 @@ public function __invoke(Event $event): Event
 
             Craft::$app->elements->invalidateCachesForElementType(Translation::class);
             Craft::$app->elements->invalidateCachesForElementType(Job::class);
+
+            Craft::$app->queue->release(
+                (string) $event->id
+            );
+
+            return $event;
         }
 
         Craft::$app->queue->release(
             (string) $event->id
         );
 
+        ++$queueJob->attempt;
+
+        \craft\helpers\Queue::push(
+            $queueJob,
+            $queueJob::PRIORITY,
+            $queueJob::getDelay()
+        );
+
         return $event;
     }
 }