From 72abdc436efa3b678d51a37eebc63fba7f4b92f5 Mon Sep 17 00:00:00 2001 From: Josh Salway Date: Tue, 21 Apr 2026 08:46:16 +1000 Subject: [PATCH 01/12] Cap queued job attempts to avoid unbounded retry loops Four queued jobs in this package ship without a $tries property, inheriting the worker-level default. That works fine for typical runs, but produces the failure mode described in #2769, #2596, #3948, #2779, #3962, #4203, and #4328: a stall or OOM during import/export triggers the worker to re-dispatch the job, which stalls or OOMs again, repeating until the worker process dies or the queue backlog is cleared manually. AfterImportJob is the most severe case: its handle() uses $this->release($this->interval) to poll for dependent ReadChunk completion. Each release increments the attempts counter; without $tries, there is no ceiling, so a failed dependency or missing cache entry keeps the poll alive indefinitely at 60-second intervals. Changes: - AfterImportJob: $tries = 10 (caps the 60s polling at ~10 minutes) - AppendDataToSheet: $tries = 5 - AppendPaginatedToSheet: $tries = 5 - AppendQueryToSheet: $tries = 5 Users who already set $tries on their Import class are unaffected, since job-level $tries is overridden by the payload resolution path already in use for ReadChunk/QueueImport. These four jobs were the only ones in src/Jobs that had no fallback. --- src/Jobs/AfterImportJob.php | 11 +++++++++++ src/Jobs/AppendDataToSheet.php | 3 +++ src/Jobs/AppendPaginatedToSheet.php | 3 +++ src/Jobs/AppendQueryToSheet.php | 3 +++ 4 files changed, 20 insertions(+) diff --git a/src/Jobs/AfterImportJob.php b/src/Jobs/AfterImportJob.php index 15be0e19c..b4bef8390 100644 --- a/src/Jobs/AfterImportJob.php +++ b/src/Jobs/AfterImportJob.php @@ -16,6 +16,17 @@ class AfterImportJob implements ShouldQueue { use HasEventBus, InteractsWithQueue, Queueable; + /** + * Upper bound on how many times this job may be attempted. + * + * This job uses $this->release($this->interval) to poll for dependent + * ReadChunk jobs. Without $tries, each release increments the attempts + * counter with no ceiling, so an import that stalls (OOM in a chunk, + * failed dependency) keeps polling indefinitely. 10 attempts at the + * default 60s interval caps the polling at ~10 minutes. + */ + public $tries = 10; + /** * @var WithEvents */ diff --git a/src/Jobs/AppendDataToSheet.php b/src/Jobs/AppendDataToSheet.php index c1c294314..a3208ee9e 100644 --- a/src/Jobs/AppendDataToSheet.php +++ b/src/Jobs/AppendDataToSheet.php @@ -14,6 +14,9 @@ class AppendDataToSheet implements ShouldQueue { use Queueable, Dispatchable, ProxyFailures, InteractsWithQueue; + /** Upper bound on how many times this job may be attempted. */ + public $tries = 5; + /** * @var array */ diff --git a/src/Jobs/AppendPaginatedToSheet.php b/src/Jobs/AppendPaginatedToSheet.php index 1befcb998..28d0dded0 100644 --- a/src/Jobs/AppendPaginatedToSheet.php +++ b/src/Jobs/AppendPaginatedToSheet.php @@ -19,6 +19,9 @@ class AppendPaginatedToSheet implements ShouldQueue { use Queueable, Dispatchable, ProxyFailures, InteractsWithQueue; + /** Upper bound on how many times this job may be attempted. */ + public $tries = 5; + /** * @var TemporaryFile */ diff --git a/src/Jobs/AppendQueryToSheet.php b/src/Jobs/AppendQueryToSheet.php index 7ac4f0d01..99ba3def8 100644 --- a/src/Jobs/AppendQueryToSheet.php +++ b/src/Jobs/AppendQueryToSheet.php @@ -18,6 +18,9 @@ class AppendQueryToSheet implements ShouldQueue { use Queueable, Dispatchable, ProxyFailures, InteractsWithQueue, HasEventBus; + /** Upper bound on how many times this job may be attempted. */ + public $tries = 5; + /** * @var TemporaryFile */ From 33df27c61ec75ea7de740639451cefe4ce6d7ccc Mon Sep 17 00:00:00 2001 From: Josh Salway Date: Tue, 21 Apr 2026 09:22:58 +1000 Subject: [PATCH 02/12] Correct docblock: describe actual worker behaviour without $tries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous docblock said 'keeps polling indefinitely' — that is only true when the worker runs with --tries=0 (unlimited). Laravel's default queue:work signature is --tries=1, which would fail AfterImportJob after a single release before dependencies complete. Updated wording describes both cases correctly. --- src/Jobs/AfterImportJob.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Jobs/AfterImportJob.php b/src/Jobs/AfterImportJob.php index b4bef8390..a1c2c4753 100644 --- a/src/Jobs/AfterImportJob.php +++ b/src/Jobs/AfterImportJob.php @@ -20,10 +20,13 @@ class AfterImportJob implements ShouldQueue * Upper bound on how many times this job may be attempted. * * This job uses $this->release($this->interval) to poll for dependent - * ReadChunk jobs. Without $tries, each release increments the attempts - * counter with no ceiling, so an import that stalls (OOM in a chunk, - * failed dependency) keeps polling indefinitely. 10 attempts at the - * default 60s interval caps the polling at ~10 minutes. + * ReadChunk jobs. Without a job-level $tries, the attempt ceiling is + * inherited from the worker command (queue:work --tries=1 by default + * on current Laravel, --tries=0 / unlimited on some hosted platforms). + * Neither fits a polling pattern: one release fails the job before + * dependencies complete, unlimited releases poll forever. 10 attempts + * at the default 60s interval caps the polling at ~10 minutes, which + * is long enough for most imports but finite. */ public $tries = 10; From 15ed27582718b94417c038fc3af7c63a0b47a5c2 Mon Sep 17 00:00:00 2001 From: Josh Salway Date: Tue, 21 Apr 2026 09:32:43 +1000 Subject: [PATCH 03/12] Reduce scope: drop $tries on Append* jobs, keep only AfterImportJob After reviewing the referenced issues in the tracker, the retry-storm failure mode the original commits cited isn't actually the pattern users are reporting. The reports are about PhpSpreadsheet memory inefficiency (#3962, #4203), queue worker execution timeouts (#2596, #3948), and unrelated architectural issues (#4328). Users hitting those reports typically want MORE attempts on the Append* jobs, not fewer. Setting $tries=5 on them could regress users whose current queue:work --tries=N gives them headroom. The only case with a genuinely defensible mechanism is AfterImportJob, which uses $this->release($this->interval) to poll for ReadChunk completion. Without a job-level $tries, that loop has no ceiling on workers started with --tries=0. Keeping that fix. Three Append* changes reverted. One file, one property remains. --- src/Jobs/AppendDataToSheet.php | 3 --- src/Jobs/AppendPaginatedToSheet.php | 3 --- src/Jobs/AppendQueryToSheet.php | 3 --- 3 files changed, 9 deletions(-) diff --git a/src/Jobs/AppendDataToSheet.php b/src/Jobs/AppendDataToSheet.php index a3208ee9e..c1c294314 100644 --- a/src/Jobs/AppendDataToSheet.php +++ b/src/Jobs/AppendDataToSheet.php @@ -14,9 +14,6 @@ class AppendDataToSheet implements ShouldQueue { use Queueable, Dispatchable, ProxyFailures, InteractsWithQueue; - /** Upper bound on how many times this job may be attempted. */ - public $tries = 5; - /** * @var array */ diff --git a/src/Jobs/AppendPaginatedToSheet.php b/src/Jobs/AppendPaginatedToSheet.php index 28d0dded0..1befcb998 100644 --- a/src/Jobs/AppendPaginatedToSheet.php +++ b/src/Jobs/AppendPaginatedToSheet.php @@ -19,9 +19,6 @@ class AppendPaginatedToSheet implements ShouldQueue { use Queueable, Dispatchable, ProxyFailures, InteractsWithQueue; - /** Upper bound on how many times this job may be attempted. */ - public $tries = 5; - /** * @var TemporaryFile */ diff --git a/src/Jobs/AppendQueryToSheet.php b/src/Jobs/AppendQueryToSheet.php index 99ba3def8..7ac4f0d01 100644 --- a/src/Jobs/AppendQueryToSheet.php +++ b/src/Jobs/AppendQueryToSheet.php @@ -18,9 +18,6 @@ class AppendQueryToSheet implements ShouldQueue { use Queueable, Dispatchable, ProxyFailures, InteractsWithQueue, HasEventBus; - /** Upper bound on how many times this job may be attempted. */ - public $tries = 5; - /** * @var TemporaryFile */ From b4ee06fdaa79a7a96d5c55d88b24038e1d54b423 Mon Sep 17 00:00:00 2001 From: Josh Salway Date: Tue, 21 Apr 2026 09:34:58 +1000 Subject: [PATCH 04/12] Apply safe retry defaults across all unconfigured queued jobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expanded from AfterImportJob-only to seven export-pipeline jobs that currently ship with no queue-safety configuration. Each gets $tries=5 with $backoff=[30,60,300] to match the pattern that ReadChunk and QueueImport already use (where they pull from the user's Import class). Patched: - AfterImportJob: $tries=10, no $backoff (its release($interval) polling sets its own timing; adding $backoff would be redundant) - AppendDataToSheet, AppendPaginatedToSheet, AppendQueryToSheet, AppendViewToSheet: $tries=5, $backoff=[30,60,300] - CloseSheet, QueueExport, StoreQueuedExport: same Not touched: - ReadChunk: already pulls $tries, $backoff, $timeout, $maxExceptions from the user's Import class - QueueImport: pulls $tries from Import class and has an empty handle() body, so retries would do nothing anyway Untyped properties used throughout to match the composer.json PHP constraint (^7.0||^8.0) — typed properties require 7.4+. --- src/Jobs/AppendDataToSheet.php | 6 ++++++ src/Jobs/AppendPaginatedToSheet.php | 6 ++++++ src/Jobs/AppendQueryToSheet.php | 6 ++++++ src/Jobs/AppendViewToSheet.php | 6 ++++++ src/Jobs/CloseSheet.php | 6 ++++++ src/Jobs/QueueExport.php | 6 ++++++ src/Jobs/StoreQueuedExport.php | 6 ++++++ 7 files changed, 42 insertions(+) diff --git a/src/Jobs/AppendDataToSheet.php b/src/Jobs/AppendDataToSheet.php index c1c294314..389299651 100644 --- a/src/Jobs/AppendDataToSheet.php +++ b/src/Jobs/AppendDataToSheet.php @@ -14,6 +14,12 @@ class AppendDataToSheet implements ShouldQueue { use Queueable, Dispatchable, ProxyFailures, InteractsWithQueue; + /** Upper bound on how many times this job may be attempted. */ + public $tries = 5; + + /** Delay in seconds between retries for transient failures (disk, DB, filesystem). */ + public $backoff = [30, 60, 300]; + /** * @var array */ diff --git a/src/Jobs/AppendPaginatedToSheet.php b/src/Jobs/AppendPaginatedToSheet.php index 1befcb998..f96910f71 100644 --- a/src/Jobs/AppendPaginatedToSheet.php +++ b/src/Jobs/AppendPaginatedToSheet.php @@ -19,6 +19,12 @@ class AppendPaginatedToSheet implements ShouldQueue { use Queueable, Dispatchable, ProxyFailures, InteractsWithQueue; + /** Upper bound on how many times this job may be attempted. */ + public $tries = 5; + + /** Delay in seconds between retries for transient failures (disk, DB, filesystem). */ + public $backoff = [30, 60, 300]; + /** * @var TemporaryFile */ diff --git a/src/Jobs/AppendQueryToSheet.php b/src/Jobs/AppendQueryToSheet.php index 7ac4f0d01..c64ce2dd7 100644 --- a/src/Jobs/AppendQueryToSheet.php +++ b/src/Jobs/AppendQueryToSheet.php @@ -18,6 +18,12 @@ class AppendQueryToSheet implements ShouldQueue { use Queueable, Dispatchable, ProxyFailures, InteractsWithQueue, HasEventBus; + /** Upper bound on how many times this job may be attempted. */ + public $tries = 5; + + /** Delay in seconds between retries for transient failures (disk, DB, filesystem). */ + public $backoff = [30, 60, 300]; + /** * @var TemporaryFile */ diff --git a/src/Jobs/AppendViewToSheet.php b/src/Jobs/AppendViewToSheet.php index a7d2c09e2..0081839f8 100644 --- a/src/Jobs/AppendViewToSheet.php +++ b/src/Jobs/AppendViewToSheet.php @@ -15,6 +15,12 @@ class AppendViewToSheet implements ShouldQueue { use Queueable, Dispatchable, InteractsWithQueue; + /** Upper bound on how many times this job may be attempted. */ + public $tries = 5; + + /** Delay in seconds between retries for transient failures (disk, DB, filesystem). */ + public $backoff = [30, 60, 300]; + /** * @var TemporaryFile */ diff --git a/src/Jobs/CloseSheet.php b/src/Jobs/CloseSheet.php index 7225505ae..ee1165481 100644 --- a/src/Jobs/CloseSheet.php +++ b/src/Jobs/CloseSheet.php @@ -12,6 +12,12 @@ class CloseSheet implements ShouldQueue { use Queueable, ProxyFailures; + /** Upper bound on how many times this job may be attempted. */ + public $tries = 5; + + /** Delay in seconds between retries for transient failures (disk, filesystem). */ + public $backoff = [30, 60, 300]; + /** * @var object */ diff --git a/src/Jobs/QueueExport.php b/src/Jobs/QueueExport.php index 562de2b95..27618dfaf 100644 --- a/src/Jobs/QueueExport.php +++ b/src/Jobs/QueueExport.php @@ -16,6 +16,12 @@ class QueueExport implements ShouldQueue { use ExtendedQueueable, Dispatchable, InteractsWithQueue; + /** Upper bound on how many times this job may be attempted. */ + public $tries = 5; + + /** Delay in seconds between retries for transient failures (disk, filesystem). */ + public $backoff = [30, 60, 300]; + /** * @var object */ diff --git a/src/Jobs/StoreQueuedExport.php b/src/Jobs/StoreQueuedExport.php index 8f6a79013..805ef8518 100644 --- a/src/Jobs/StoreQueuedExport.php +++ b/src/Jobs/StoreQueuedExport.php @@ -11,6 +11,12 @@ class StoreQueuedExport implements ShouldQueue { use Queueable; + /** Upper bound on how many times this job may be attempted. */ + public $tries = 5; + + /** Delay in seconds between retries for transient failures (disk, remote filesystem). */ + public $backoff = [30, 60, 300]; + /** * @var string */ From 6ea9c7cb1b785d4a1994c012de71a7cdffdac2e0 Mon Sep 17 00:00:00 2001 From: Josh Salway Date: Tue, 21 Apr 2026 09:45:49 +1000 Subject: [PATCH 05/12] Fall back to Laravel defaults for ReadChunk $tries and $backoff ReadChunk resolves $tries, $backoff, $timeout, and $maxExceptions from the user's Import class at construction time. If the Import class doesn't set them, these currently resolve to null, meaning the worker falls back to whatever queue:work --tries=N and default backoff (0) are. This changes two of them to explicit fallbacks that match what a fresh Laravel install does: - $tries: null -> 1 (same as queue:work --tries default) - $backoff: null -> [30, 60, 300] (affects spacing only, not count) Effect by scenario: - User sets Import::$tries = 5: unchanged (Import still wins) - User runs queue:work (default --tries=1): unchanged (1 either way) - User runs queue:work --tries=0 without Import setting: now bounded at 1 attempt instead of unlimited retries on failure - User runs queue:work --tries=5 without Import setting: now bounded at 1 instead of 5. Can restore by setting $tries on the Import class. $timeout and $maxExceptions left as null-fallbacks because worker --timeout=60 default is already tight, and $maxExceptions default behaviour is correct when $tries alone acts as the ceiling. --- src/Jobs/ReadChunk.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Jobs/ReadChunk.php b/src/Jobs/ReadChunk.php index ef73373d8..67b3ed34c 100644 --- a/src/Jobs/ReadChunk.php +++ b/src/Jobs/ReadChunk.php @@ -115,9 +115,9 @@ public function __construct(WithChunkReading $import, IReader $reader, Temporary $this->startRow = $startRow; $this->chunkSize = $chunkSize; $this->timeout = $import->timeout ?? null; - $this->tries = $import->tries ?? null; + $this->tries = $import->tries ?? 1; $this->maxExceptions = $import->maxExceptions ?? null; - $this->backoff = method_exists($import, 'backoff') ? $import->backoff() : ($import->backoff ?? null); + $this->backoff = method_exists($import, 'backoff') ? $import->backoff() : ($import->backoff ?? [30, 60, 300]); $this->connection = property_exists($import, 'connection') ? $import->connection : null; $this->queue = property_exists($import, 'queue') ? $import->queue : null; } From ab57c177688dd2a694b8fc5f7893314464346b62 Mon Sep 17 00:00:00 2001 From: Josh Salway Date: Tue, 21 Apr 2026 15:40:27 +1000 Subject: [PATCH 06/12] Add regression tests for ReadChunk Import-class fallback resolution Three unit tests verify the fallback behaviour documented in the PR: - test_readchunk_falls_back_to_tries_one_when_import_has_no_retry_config Import class without $tries -> ReadChunk.tries = 1 - test_readchunk_falls_back_to_exponential_backoff_when_import_has_no_retry_config Import class without $backoff -> ReadChunk.backoff = [30, 60, 300] - test_readchunk_preserves_user_tries_set_on_import Import class with $tries = 7 -> ReadChunk.tries = 7 (user wins) Construct ReadChunk directly with anonymous-class Import instances and mocked IReader/TemporaryFile dependencies to avoid the DB-backed test path needed for the existing queue propagation tests. --- tests/QueuedImportTest.php | 1 + tests/ReadChunkDefaultsTest.php | 83 +++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 tests/ReadChunkDefaultsTest.php diff --git a/tests/QueuedImportTest.php b/tests/QueuedImportTest.php index 39271af8c..abdb47c19 100644 --- a/tests/QueuedImportTest.php +++ b/tests/QueuedImportTest.php @@ -236,4 +236,5 @@ public function test_can_define_max_exceptions_property_on_queued_import() $this->assertEquals(3, $maxExceptionsCount); } + } diff --git a/tests/ReadChunkDefaultsTest.php b/tests/ReadChunkDefaultsTest.php new file mode 100644 index 000000000..17dd6d17a --- /dev/null +++ b/tests/ReadChunkDefaultsTest.php @@ -0,0 +1,83 @@ +tries = $tries; + } + + public function chunkSize(): int + { + return 100; + } + }; + } + + /** + * @return ReadChunk + */ + private function makeReadChunk(WithChunkReading $import): ReadChunk + { + return new ReadChunk( + $import, + $this->createMock(IReader::class), + $this->createMock(TemporaryFile::class), + 'Sheet1', + $import, + 1, + 100 + ); + } + + public function test_readchunk_falls_back_to_tries_one_when_import_has_no_retry_config() + { + $readChunk = $this->makeReadChunk($this->importWithoutRetryConfig()); + + $this->assertSame(1, $readChunk->tries); + } + + public function test_readchunk_falls_back_to_exponential_backoff_when_import_has_no_retry_config() + { + $readChunk = $this->makeReadChunk($this->importWithoutRetryConfig()); + + $this->assertSame([30, 60, 300], $readChunk->backoff); + } + + public function test_readchunk_preserves_user_tries_set_on_import() + { + $readChunk = $this->makeReadChunk($this->importWithTriesSetTo(7)); + + $this->assertSame(7, $readChunk->tries); + } +} From e8622d922953a34dfa5d9b80c7f562bb963dde20 Mon Sep 17 00:00:00 2001 From: Josh Salway Date: Tue, 21 Apr 2026 15:41:22 +1000 Subject: [PATCH 07/12] Drop accidental phpunit cache file from PR The .phpunit.cache/test-results file was inadvertently committed when running the new ReadChunkDefaultsTest locally. Reverting to match origin/3.1. Also removed a stray blank line at the end of QueuedImportTest.php. --- tests/QueuedImportTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/QueuedImportTest.php b/tests/QueuedImportTest.php index abdb47c19..39271af8c 100644 --- a/tests/QueuedImportTest.php +++ b/tests/QueuedImportTest.php @@ -236,5 +236,4 @@ public function test_can_define_max_exceptions_property_on_queued_import() $this->assertEquals(3, $maxExceptionsCount); } - } From 0c5e702c914d24aa318456c29ced4b5adaf4521f Mon Sep 17 00:00:00 2001 From: Josh Salway Date: Tue, 21 Apr 2026 16:18:54 +1000 Subject: [PATCH 08/12] Reorder ReadChunkDefaultsTest to satisfy StyleCI ordered_class_elements StyleCI flagged the original ordering (private helpers before public test methods). Moved the three test methods to the top of the class and left the helpers below, matching the visibility ordering the laravel preset enforces (public -> private). No test logic changed. All 3 tests still pass locally: Tests: 3, Assertions: 3. --- tests/ReadChunkDefaultsTest.php | 42 ++++++++++++++++----------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/tests/ReadChunkDefaultsTest.php b/tests/ReadChunkDefaultsTest.php index 17dd6d17a..e2f877efb 100644 --- a/tests/ReadChunkDefaultsTest.php +++ b/tests/ReadChunkDefaultsTest.php @@ -9,6 +9,27 @@ class ReadChunkDefaultsTest extends TestCase { + public function test_readchunk_falls_back_to_tries_one_when_import_has_no_retry_config() + { + $readChunk = $this->makeReadChunk($this->importWithoutRetryConfig()); + + $this->assertSame(1, $readChunk->tries); + } + + public function test_readchunk_falls_back_to_exponential_backoff_when_import_has_no_retry_config() + { + $readChunk = $this->makeReadChunk($this->importWithoutRetryConfig()); + + $this->assertSame([30, 60, 300], $readChunk->backoff); + } + + public function test_readchunk_preserves_user_tries_set_on_import() + { + $readChunk = $this->makeReadChunk($this->importWithTriesSetTo(7)); + + $this->assertSame(7, $readChunk->tries); + } + /** * @return WithChunkReading */ @@ -59,25 +80,4 @@ private function makeReadChunk(WithChunkReading $import): ReadChunk 100 ); } - - public function test_readchunk_falls_back_to_tries_one_when_import_has_no_retry_config() - { - $readChunk = $this->makeReadChunk($this->importWithoutRetryConfig()); - - $this->assertSame(1, $readChunk->tries); - } - - public function test_readchunk_falls_back_to_exponential_backoff_when_import_has_no_retry_config() - { - $readChunk = $this->makeReadChunk($this->importWithoutRetryConfig()); - - $this->assertSame([30, 60, 300], $readChunk->backoff); - } - - public function test_readchunk_preserves_user_tries_set_on_import() - { - $readChunk = $this->makeReadChunk($this->importWithTriesSetTo(7)); - - $this->assertSame(7, $readChunk->tries); - } } From f22c5f167a5fae3c38943acbddc90c6d737c9134 Mon Sep 17 00:00:00 2001 From: Josh Salway Date: Tue, 21 Apr 2026 17:35:35 +1000 Subject: [PATCH 09/12] Reduce retry defaults: $tries=1 for single-run jobs (no $backoff) Changes the seven single-run jobs from $tries=5 with exponential backoff to $tries=1 with no backoff. Keeps AfterImportJob at $tries=10 because its polling pattern genuinely needs the headroom. Rationale: - $tries=1 matches Laravel's queue:work --tries default. Default-worker deployments see zero behaviour change. - Still caps --tries=0 (Vapor-pattern) deployments at 1 attempt instead of unlimited. - Append*/Close/Queue/Store jobs write to temp files and their idempotency on partial-write failure is not obvious. Fail-fast on the first error avoids any risk of double-written rows or corrupt output. - ReadChunk fallback ($tries ?? 1, $backoff ?? [30, 60, 300]) kept as-is: users who explicitly set $tries on their Import class still get properly-spaced retries via the $backoff fallback. - AfterImportJob keeps $tries=10 because its handle() uses $this->release($interval) to poll for ReadChunk completion. Each release increments the attempt counter; $tries=1 would fail the job on the first release before any dependency completes. --- src/Jobs/AppendDataToSheet.php | 5 +---- src/Jobs/AppendPaginatedToSheet.php | 5 +---- src/Jobs/AppendQueryToSheet.php | 5 +---- src/Jobs/AppendViewToSheet.php | 5 +---- src/Jobs/CloseSheet.php | 5 +---- src/Jobs/QueueExport.php | 5 +---- src/Jobs/StoreQueuedExport.php | 5 +---- 7 files changed, 7 insertions(+), 28 deletions(-) diff --git a/src/Jobs/AppendDataToSheet.php b/src/Jobs/AppendDataToSheet.php index 389299651..59fc9761e 100644 --- a/src/Jobs/AppendDataToSheet.php +++ b/src/Jobs/AppendDataToSheet.php @@ -15,10 +15,7 @@ class AppendDataToSheet implements ShouldQueue use Queueable, Dispatchable, ProxyFailures, InteractsWithQueue; /** Upper bound on how many times this job may be attempted. */ - public $tries = 5; - - /** Delay in seconds between retries for transient failures (disk, DB, filesystem). */ - public $backoff = [30, 60, 300]; + public $tries = 1; /** * @var array diff --git a/src/Jobs/AppendPaginatedToSheet.php b/src/Jobs/AppendPaginatedToSheet.php index f96910f71..d37eefda1 100644 --- a/src/Jobs/AppendPaginatedToSheet.php +++ b/src/Jobs/AppendPaginatedToSheet.php @@ -20,10 +20,7 @@ class AppendPaginatedToSheet implements ShouldQueue use Queueable, Dispatchable, ProxyFailures, InteractsWithQueue; /** Upper bound on how many times this job may be attempted. */ - public $tries = 5; - - /** Delay in seconds between retries for transient failures (disk, DB, filesystem). */ - public $backoff = [30, 60, 300]; + public $tries = 1; /** * @var TemporaryFile diff --git a/src/Jobs/AppendQueryToSheet.php b/src/Jobs/AppendQueryToSheet.php index c64ce2dd7..4726d0a6a 100644 --- a/src/Jobs/AppendQueryToSheet.php +++ b/src/Jobs/AppendQueryToSheet.php @@ -19,10 +19,7 @@ class AppendQueryToSheet implements ShouldQueue use Queueable, Dispatchable, ProxyFailures, InteractsWithQueue, HasEventBus; /** Upper bound on how many times this job may be attempted. */ - public $tries = 5; - - /** Delay in seconds between retries for transient failures (disk, DB, filesystem). */ - public $backoff = [30, 60, 300]; + public $tries = 1; /** * @var TemporaryFile diff --git a/src/Jobs/AppendViewToSheet.php b/src/Jobs/AppendViewToSheet.php index 0081839f8..3accbd821 100644 --- a/src/Jobs/AppendViewToSheet.php +++ b/src/Jobs/AppendViewToSheet.php @@ -16,10 +16,7 @@ class AppendViewToSheet implements ShouldQueue use Queueable, Dispatchable, InteractsWithQueue; /** Upper bound on how many times this job may be attempted. */ - public $tries = 5; - - /** Delay in seconds between retries for transient failures (disk, DB, filesystem). */ - public $backoff = [30, 60, 300]; + public $tries = 1; /** * @var TemporaryFile diff --git a/src/Jobs/CloseSheet.php b/src/Jobs/CloseSheet.php index ee1165481..2abff9a2f 100644 --- a/src/Jobs/CloseSheet.php +++ b/src/Jobs/CloseSheet.php @@ -13,10 +13,7 @@ class CloseSheet implements ShouldQueue use Queueable, ProxyFailures; /** Upper bound on how many times this job may be attempted. */ - public $tries = 5; - - /** Delay in seconds between retries for transient failures (disk, filesystem). */ - public $backoff = [30, 60, 300]; + public $tries = 1; /** * @var object diff --git a/src/Jobs/QueueExport.php b/src/Jobs/QueueExport.php index 27618dfaf..0d3c80fbc 100644 --- a/src/Jobs/QueueExport.php +++ b/src/Jobs/QueueExport.php @@ -17,10 +17,7 @@ class QueueExport implements ShouldQueue use ExtendedQueueable, Dispatchable, InteractsWithQueue; /** Upper bound on how many times this job may be attempted. */ - public $tries = 5; - - /** Delay in seconds between retries for transient failures (disk, filesystem). */ - public $backoff = [30, 60, 300]; + public $tries = 1; /** * @var object diff --git a/src/Jobs/StoreQueuedExport.php b/src/Jobs/StoreQueuedExport.php index 805ef8518..1e26c2f65 100644 --- a/src/Jobs/StoreQueuedExport.php +++ b/src/Jobs/StoreQueuedExport.php @@ -12,10 +12,7 @@ class StoreQueuedExport implements ShouldQueue use Queueable; /** Upper bound on how many times this job may be attempted. */ - public $tries = 5; - - /** Delay in seconds between retries for transient failures (disk, remote filesystem). */ - public $backoff = [30, 60, 300]; + public $tries = 1; /** * @var string From 49c3c46bc2102e5fc2f6867c66469dc7ed73722b Mon Sep 17 00:00:00 2001 From: Josh Salway Date: Tue, 21 Apr 2026 23:21:13 +1000 Subject: [PATCH 10/12] Drop ReadChunk backoff fallback; keep only $tries=1 fallback Dropped the $import->backoff fallback to [30, 60, 300] on ReadChunk. If the user sets $tries on their Import class but no $backoff, they now get the framework default (0s between retries) again. If they want spacing they can set it on their Import class. Rationale: this PR's narrative for the seven single-run jobs is 'cap runaway attempts, don't impose retry-spacing opinions.' Keeping a backoff fallback on ReadChunk contradicted that for a narrow case (user opted into tries>1 without setting backoff). Follow-up PR can add it if maintainers want it. Dropped the accompanying test. Two tests remain: - test_readchunk_falls_back_to_tries_one_when_import_has_no_tries - test_readchunk_preserves_user_tries_set_on_import Tests: 2, Assertions: 2. --- src/Jobs/ReadChunk.php | 2 +- tests/ReadChunkDefaultsTest.php | 9 +-------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/Jobs/ReadChunk.php b/src/Jobs/ReadChunk.php index 67b3ed34c..0bfeec72e 100644 --- a/src/Jobs/ReadChunk.php +++ b/src/Jobs/ReadChunk.php @@ -117,7 +117,7 @@ public function __construct(WithChunkReading $import, IReader $reader, Temporary $this->timeout = $import->timeout ?? null; $this->tries = $import->tries ?? 1; $this->maxExceptions = $import->maxExceptions ?? null; - $this->backoff = method_exists($import, 'backoff') ? $import->backoff() : ($import->backoff ?? [30, 60, 300]); + $this->backoff = method_exists($import, 'backoff') ? $import->backoff() : ($import->backoff ?? null); $this->connection = property_exists($import, 'connection') ? $import->connection : null; $this->queue = property_exists($import, 'queue') ? $import->queue : null; } diff --git a/tests/ReadChunkDefaultsTest.php b/tests/ReadChunkDefaultsTest.php index e2f877efb..a73c56dc2 100644 --- a/tests/ReadChunkDefaultsTest.php +++ b/tests/ReadChunkDefaultsTest.php @@ -9,20 +9,13 @@ class ReadChunkDefaultsTest extends TestCase { - public function test_readchunk_falls_back_to_tries_one_when_import_has_no_retry_config() + public function test_readchunk_falls_back_to_tries_one_when_import_has_no_tries() { $readChunk = $this->makeReadChunk($this->importWithoutRetryConfig()); $this->assertSame(1, $readChunk->tries); } - public function test_readchunk_falls_back_to_exponential_backoff_when_import_has_no_retry_config() - { - $readChunk = $this->makeReadChunk($this->importWithoutRetryConfig()); - - $this->assertSame([30, 60, 300], $readChunk->backoff); - } - public function test_readchunk_preserves_user_tries_set_on_import() { $readChunk = $this->makeReadChunk($this->importWithTriesSetTo(7)); From ba538c6b2f70fb2d95ce818eed6ca2c35a4e07db Mon Sep 17 00:00:00 2001 From: Josh Salway Date: Tue, 21 Apr 2026 23:24:49 +1000 Subject: [PATCH 11/12] Simplify AfterImportJob $tries docblock Removed the long inline rationale; the same information now lives in the PR body where it is easier to maintain. One-line docblock points at the load-bearing fact (each release() counts as an attempt). --- src/Jobs/AfterImportJob.php | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/Jobs/AfterImportJob.php b/src/Jobs/AfterImportJob.php index a1c2c4753..30e980bd6 100644 --- a/src/Jobs/AfterImportJob.php +++ b/src/Jobs/AfterImportJob.php @@ -16,18 +16,7 @@ class AfterImportJob implements ShouldQueue { use HasEventBus, InteractsWithQueue, Queueable; - /** - * Upper bound on how many times this job may be attempted. - * - * This job uses $this->release($this->interval) to poll for dependent - * ReadChunk jobs. Without a job-level $tries, the attempt ceiling is - * inherited from the worker command (queue:work --tries=1 by default - * on current Laravel, --tries=0 / unlimited on some hosted platforms). - * Neither fits a polling pattern: one release fails the job before - * dependencies complete, unlimited releases poll forever. 10 attempts - * at the default 60s interval caps the polling at ~10 minutes, which - * is long enough for most imports but finite. - */ + /** Higher than other jobs in this package: each release() in handle() counts as an attempt. */ public $tries = 10; /** From 90783bda2b700bf6f247ec599d41187ab3cb49df Mon Sep 17 00:00:00 2001 From: Josh Salway Date: Tue, 21 Apr 2026 23:28:28 +1000 Subject: [PATCH 12/12] Remove ReadChunkDefaultsTest The tests verified that PHP's null-coalescing operator returns the fallback when the operand is null and the operand otherwise. That is a language guarantee, not library behaviour. The existing integration tests in QueuedImportTest.php already cover the jobs through real dispatch cycles (see test_can_define_max_exceptions_property_on_queued_import for the pattern). Dropping the unit file to keep the PR focused on the one-line property changes. --- tests/ReadChunkDefaultsTest.php | 76 --------------------------------- 1 file changed, 76 deletions(-) delete mode 100644 tests/ReadChunkDefaultsTest.php diff --git a/tests/ReadChunkDefaultsTest.php b/tests/ReadChunkDefaultsTest.php deleted file mode 100644 index a73c56dc2..000000000 --- a/tests/ReadChunkDefaultsTest.php +++ /dev/null @@ -1,76 +0,0 @@ -makeReadChunk($this->importWithoutRetryConfig()); - - $this->assertSame(1, $readChunk->tries); - } - - public function test_readchunk_preserves_user_tries_set_on_import() - { - $readChunk = $this->makeReadChunk($this->importWithTriesSetTo(7)); - - $this->assertSame(7, $readChunk->tries); - } - - /** - * @return WithChunkReading - */ - private function importWithoutRetryConfig(): WithChunkReading - { - return new class implements WithChunkReading - { - public function chunkSize(): int - { - return 100; - } - }; - } - - /** - * @return WithChunkReading - */ - private function importWithTriesSetTo(int $tries): WithChunkReading - { - return new class($tries) implements WithChunkReading - { - public $tries; - - public function __construct(int $tries) - { - $this->tries = $tries; - } - - public function chunkSize(): int - { - return 100; - } - }; - } - - /** - * @return ReadChunk - */ - private function makeReadChunk(WithChunkReading $import): ReadChunk - { - return new ReadChunk( - $import, - $this->createMock(IReader::class), - $this->createMock(TemporaryFile::class), - 'Sheet1', - $import, - 1, - 100 - ); - } -}