diff --git a/docs/redis-queue-worker.md b/docs/redis-queue-worker.md index 08757c9c..ce794574 100644 --- a/docs/redis-queue-worker.md +++ b/docs/redis-queue-worker.md @@ -234,3 +234,27 @@ The OpenAI API key may not be configured in WordPress: - Check `Settings > CDCF` in wp-admin for the API key - Check the WordPress debug log for OpenAI API errors + +### The dashboard lists old jobs as "queued" + +The Redis Queue dashboard lists the job records in MySQL; the worker +processes only what is in Redis. Check Redis first: + +```bash +redis-cli -n 0 ZCARD redis_queue:queue:default +redis-cli -n 0 ZCARD redis_queue:delayed +``` + +If both are `0`, the "queued" records will never run, and **Purge → Older +Than N** removes them (it deletes records only, never Redis entries). +Records can be left behind in two ways: + +- **Retries.** redis-queue retries a failed job as a new job and leaves the + old record set to `queued`. The plugin deletes that old record on + `redis_queue_job_retried` (`includes/retry-cleanup.php`) once the new job + is enqueued, so this only affects records from before that change and + retries whose enqueue failed. +- **"Reset stuck jobs".** A job leaves Redis as soon as it is picked up, so a + `processing` record whose request died stays out of Redis. Resetting it + to `queued` does not put it back. Re-run the translation from the post's + AI Translation meta box instead. diff --git a/wordpress/plugins/cdcf-redis-translations/cdcf-redis-translations.php b/wordpress/plugins/cdcf-redis-translations/cdcf-redis-translations.php index 16d2baa2..e4bb0c1e 100644 --- a/wordpress/plugins/cdcf-redis-translations/cdcf-redis-translations.php +++ b/wordpress/plugins/cdcf-redis-translations/cdcf-redis-translations.php @@ -18,6 +18,10 @@ require_once __DIR__ . '/includes/class-translation-job.php'; require_once __DIR__ . '/includes/functions.php'; + require_once __DIR__ . '/includes/retry-cleanup.php'; + + add_action('redis_queue_job_enqueued', 'cdcf_remember_enqueued_job', 10, 2); + add_action('redis_queue_job_retried', 'cdcf_delete_superseded_job_record', 10, 2); }, 20); // REST endpoint to trigger queue processing (for cron via curl). diff --git a/wordpress/plugins/cdcf-redis-translations/includes/class-translation-job.php b/wordpress/plugins/cdcf-redis-translations/includes/class-translation-job.php index c7ba3068..6e95a1d3 100644 --- a/wordpress/plugins/cdcf-redis-translations/includes/class-translation-job.php +++ b/wordpress/plugins/cdcf-redis-translations/includes/class-translation-job.php @@ -12,6 +12,22 @@ public function get_job_type() { return 'cdcf_translation'; } + /** + * redis-queue retries a job as a new one whose own attempt count + * starts again at zero, so that count never reaches the limit and a + * job that keeps failing retries forever. Count the attempts in the + * payload instead, which the retried job carries over. + */ + public function should_retry($exception, $attempt) { + $attempt = max((int) $attempt, (int) ($this->payload['_cdcf_attempt'] ?? 0) + 1); + $this->payload['_cdcf_attempt'] = $attempt; + return parent::should_retry($exception, $attempt); + } + + public function get_retry_delay($attempt) { + return parent::get_retry_delay(max((int) $attempt, (int) ($this->payload['_cdcf_attempt'] ?? 0))); + } + public function execute() { $post_id = $this->get_payload_value('post_id'); $source_id = $this->get_payload_value('source_id'); diff --git a/wordpress/plugins/cdcf-redis-translations/includes/functions.php b/wordpress/plugins/cdcf-redis-translations/includes/functions.php index b4fa1261..a28b9d63 100644 --- a/wordpress/plugins/cdcf-redis-translations/includes/functions.php +++ b/wordpress/plugins/cdcf-redis-translations/includes/functions.php @@ -16,8 +16,12 @@ function cdcf_enqueue_translation(int $post_id, int $source_id, string $target_l 'source_id' => $source_id, 'target_lang' => $target_lang, ]); - redis_queue()->queue_manager->enqueue($job); - return 'redis'; + // enqueue() returns false, without throwing, when Redis is + // unreachable or the job record cannot be stored. + if (redis_queue()->queue_manager->enqueue($job) !== false) { + return 'redis'; + } + error_log('cdcf_enqueue_translation: Redis enqueue returned false, falling back to WP Cron'); } catch (\Throwable $e) { error_log('cdcf_enqueue_translation: Redis enqueue failed, falling back to WP Cron – ' . $e->getMessage()); } diff --git a/wordpress/plugins/cdcf-redis-translations/includes/retry-cleanup.php b/wordpress/plugins/cdcf-redis-translations/includes/retry-cleanup.php new file mode 100644 index 00000000..e7da4d07 --- /dev/null +++ b/wordpress/plugins/cdcf-redis-translations/includes/retry-cleanup.php @@ -0,0 +1,68 @@ +delete( + $wpdb->prefix . 'redis_queue_jobs', + ['job_id' => (string) $job_id, 'status' => 'queued'], + ['%s', '%s'] + ); +} diff --git a/wordpress/plugins/cdcf-redis-translations/tests/EnqueueTranslationFallbackTest.php b/wordpress/plugins/cdcf-redis-translations/tests/EnqueueTranslationFallbackTest.php index d0f5a466..3aab0131 100644 --- a/wordpress/plugins/cdcf-redis-translations/tests/EnqueueTranslationFallbackTest.php +++ b/wordpress/plugins/cdcf-redis-translations/tests/EnqueueTranslationFallbackTest.php @@ -118,6 +118,34 @@ public function test_enqueue_throw_falls_back_to_wp_schedule_single_event(): voi $this->assertSame('wp-cron', $result); } + /** + * redis-queue's enqueue() returns false, without throwing, when it is + * not connected to Redis or cannot store the job record. + */ + public function test_enqueue_returning_false_falls_back_to_wp_schedule_single_event(): void + { + $queueManager = Mockery::mock(); + $queueManager->shouldReceive('enqueue')->once()->andReturn(false); + + $queue = Mockery::mock(); + $queue->queue_manager = $queueManager; + + Functions\when('redis_queue')->justReturn($queue); + Functions\when('error_log')->justReturn(true); + + Functions\expect('wp_schedule_single_event') + ->once() + ->with( + Mockery::type('integer'), + 'cdcf_async_translate', + [42, 7, 'it'] + ); + + $result = cdcf_enqueue_translation(42, 7, 'it'); + + $this->assertSame('wp-cron', $result); + } + public function test_redis_up_passes_documented_payload_shape(): void { $capturedPayload = null; diff --git a/wordpress/plugins/cdcf-redis-translations/tests/RetryCleanupTest.php b/wordpress/plugins/cdcf-redis-translations/tests/RetryCleanupTest.php new file mode 100644 index 00000000..1f30a442 --- /dev/null +++ b/wordpress/plugins/cdcf-redis-translations/tests/RetryCleanupTest.php @@ -0,0 +1,88 @@ +prefix = 'wp_'; + $wpdb->shouldReceive('delete') + ->once() + ->with('wp_redis_queue_jobs', ['job_id' => 'job_abc', 'status' => 'queued'], ['%s', '%s']) + ->andReturn(1); + $GLOBALS['wpdb'] = $wpdb; + + $job = new stdClass(); + cdcf_remember_enqueued_job('job_new', $job); + cdcf_delete_superseded_job_record('job_abc', $job); + + $this->addToAssertionCount(1); + } + + public function test_keeps_the_record_when_the_replacement_was_not_enqueued(): void + { + $wpdb = Mockery::mock(); + $wpdb->prefix = 'wp_'; + $wpdb->shouldNotReceive('delete'); + $GLOBALS['wpdb'] = $wpdb; + + // An earlier, unrelated job was enqueued; the retry's enqueue failed. + cdcf_remember_enqueued_job('job_other', new stdClass()); + cdcf_delete_superseded_job_record('job_abc', new stdClass()); + + $this->addToAssertionCount(1); + } + + public function test_a_successful_enqueue_is_used_once(): void + { + $wpdb = Mockery::mock(); + $wpdb->prefix = 'wp_'; + $wpdb->shouldReceive('delete')->once()->andReturn(1); + $GLOBALS['wpdb'] = $wpdb; + + $job = new stdClass(); + cdcf_remember_enqueued_job('job_new', $job); + cdcf_delete_superseded_job_record('job_abc', $job); + // A second retry of the same job whose enqueue failed. + cdcf_delete_superseded_job_record('job_new', $job); + + $this->addToAssertionCount(1); + } + + public function test_keeps_the_record_without_the_job(): void + { + $wpdb = Mockery::mock(); + $wpdb->prefix = 'wp_'; + $wpdb->shouldNotReceive('delete'); + $GLOBALS['wpdb'] = $wpdb; + + cdcf_delete_superseded_job_record('job_abc'); + + $this->addToAssertionCount(1); + } +} diff --git a/wordpress/plugins/cdcf-redis-translations/tests/TranslationJobTest.php b/wordpress/plugins/cdcf-redis-translations/tests/TranslationJobTest.php index d568906a..815b713d 100644 --- a/wordpress/plugins/cdcf-redis-translations/tests/TranslationJobTest.php +++ b/wordpress/plugins/cdcf-redis-translations/tests/TranslationJobTest.php @@ -170,4 +170,37 @@ function (int $post_id, int $source_id, string $target_lang) use (&$calls): bool $this->assertSame([200, 100, 'it'], $calls); } + + /** + * redis-queue re-enqueues a retry as a new job whose own attempt + * count starts again at zero; the payload has to carry the count. + */ + public function test_the_attempt_count_carries_over_into_the_retried_job(): void + { + $payload = ['post_id' => 200, 'source_id' => 100, 'target_lang' => 'it']; + $error = new \RuntimeException('OpenAI timed out'); + + $first = new CDCF_Translation_Job($payload); + $this->assertTrue($first->should_retry($error, 1)); + + // The queue stores get_payload() for the retry and rebuilds the job from it. + $retry = new CDCF_Translation_Job($first->get_payload()); + $this->assertFalse($retry->should_retry($error, 1)); + } + + public function test_the_retry_delay_follows_the_carried_attempt_count(): void + { + $job = new CDCF_Translation_Job(['post_id' => 200, '_cdcf_attempt' => 1]); + $job->should_retry(new \RuntimeException('again'), 1); + + $this->assertSame(120, $job->get_retry_delay(1)); + } + + public function test_a_first_failure_retries_after_the_first_backoff(): void + { + $job = new CDCF_Translation_Job(['post_id' => 200]); + $job->should_retry(new \RuntimeException('once'), 1); + + $this->assertSame(30, $job->get_retry_delay(1)); + } } diff --git a/wordpress/plugins/cdcf-redis-translations/tests/stubs/AbstractBaseJobStub.php b/wordpress/plugins/cdcf-redis-translations/tests/stubs/AbstractBaseJobStub.php index 11e9c8c3..d5bee246 100644 --- a/wordpress/plugins/cdcf-redis-translations/tests/stubs/AbstractBaseJobStub.php +++ b/wordpress/plugins/cdcf-redis-translations/tests/stubs/AbstractBaseJobStub.php @@ -13,6 +13,10 @@ abstract class Abstract_Base_Job { protected array $payload; + protected int $retry_attempts = 3; + + protected array $retry_backoff = [60, 300, 900]; + public function __construct(array $payload = []) { $this->payload = $payload; @@ -33,6 +37,18 @@ protected function success(array $data = []): array return ['success' => true, 'data' => $data]; } + // Mirrors upstream's limit check and backoff lookup; the filters + // and non-retryable exception list are left out. + public function should_retry($exception, $attempt) + { + return $attempt < $this->retry_attempts; + } + + public function get_retry_delay($attempt) + { + return $this->retry_backoff[$attempt - 1] ?? min(2 ** $attempt * 60, 3600); + } + abstract public function get_job_type(); abstract public function execute(); diff --git a/wordpress/themes/cdcf-headless/includes/review/email-job.php b/wordpress/themes/cdcf-headless/includes/review/email-job.php index 9e006d56..3aa435dd 100644 --- a/wordpress/themes/cdcf-headless/includes/review/email-job.php +++ b/wordpress/themes/cdcf-headless/includes/review/email-job.php @@ -16,6 +16,24 @@ public function get_job_type() return CDCF_REVIEW_EMAIL_JOB; } + /** + * redis-queue retries a job as a new one whose own attempt count + * starts again at zero, so that count never reaches the limit and a + * job that keeps failing retries forever. Count the attempts in the + * payload instead, which the retried job carries over. + */ + public function should_retry($exception, $attempt) + { + $attempt = max((int) $attempt, (int) ($this->payload['_cdcf_attempt'] ?? 0) + 1); + $this->payload['_cdcf_attempt'] = $attempt; + return parent::should_retry($exception, $attempt); + } + + public function get_retry_delay($attempt) + { + return parent::get_retry_delay(max((int) $attempt, (int) ($this->payload['_cdcf_attempt'] ?? 0))); + } + public function execute() { $member = $this->get_payload_value('member'); diff --git a/wordpress/themes/cdcf-headless/tests/ReviewMailerTest.php b/wordpress/themes/cdcf-headless/tests/ReviewMailerTest.php index 19baa839..697b49b4 100644 --- a/wordpress/themes/cdcf-headless/tests/ReviewMailerTest.php +++ b/wordpress/themes/cdcf-headless/tests/ReviewMailerTest.php @@ -279,4 +279,20 @@ public function test_member_job_delivers_by_id_and_retries_only_on_failure(): vo $this->expectException(RuntimeException::class); $job->execute(); } + + public function test_a_job_that_keeps_failing_stops_retrying_after_three_attempts(): void + { + $error = new \RuntimeException('wp_mail failed'); + $job = new CDCF_Review_Email_Job(['member' => ['user_id' => 7, 'event_id' => 3]]); + + // Each retry is a new job built from the stored payload, whose own + // attempt count is 1 again. + $this->assertTrue($job->should_retry($error, 1)); + $this->assertSame(60, $job->get_retry_delay(1)); + $job = new CDCF_Review_Email_Job($job->get_payload()); + $this->assertTrue($job->should_retry($error, 1)); + $this->assertSame(300, $job->get_retry_delay(1)); + $job = new CDCF_Review_Email_Job($job->get_payload()); + $this->assertFalse($job->should_retry($error, 1)); + } } diff --git a/wordpress/themes/cdcf-headless/tests/stubs/AbstractBaseJobStub.php b/wordpress/themes/cdcf-headless/tests/stubs/AbstractBaseJobStub.php index 98b6643e..48508e79 100644 --- a/wordpress/themes/cdcf-headless/tests/stubs/AbstractBaseJobStub.php +++ b/wordpress/themes/cdcf-headless/tests/stubs/AbstractBaseJobStub.php @@ -10,6 +10,10 @@ if (!class_exists(Abstract_Base_Job::class)) { abstract class Abstract_Base_Job { + protected int $retry_attempts = 3; + + protected array $retry_backoff = [60, 300, 900]; + public function __construct(protected array $payload = []) {} public function get_payload_value(string $key): mixed @@ -27,6 +31,18 @@ protected function success(array $data = []): array return ['success' => true, 'data' => $data]; } + // Mirrors upstream's limit check and backoff lookup; the filters + // and non-retryable exception list are left out. + public function should_retry($exception, $attempt) + { + return $attempt < $this->retry_attempts; + } + + public function get_retry_delay($attempt) + { + return $this->retry_backoff[$attempt - 1] ?? min(2 ** $attempt * 60, 3600); + } + abstract public function get_job_type(); abstract public function execute();