From f5bc9536d937f54b81212bbced2fce86dbee29e5 Mon Sep 17 00:00:00 2001 From: Muhamad Raza Haider Date: Sat, 24 Oct 2026 10:24:01 -0700 Subject: [PATCH 1/2] fix: reload worker after exactly maxJobs requests, not one later Closes #967 --- CHANGELOG.md | 1 + src/Reboot/Strategy/MaxJobsRebootStrategy.php | 5 ++++- tests/RebootStrategyTest.php | 9 +++++---- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7275723..07d0fd37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `MaxJobsRebootStrategy::shouldReboot()` now reloads the worker after exactly `requests` requests instead of one request later (`>=` instead of `>`): the strategy is consulted after each request is handled, so the old check served `maxJobs + 1` requests before reloading, contradicting the docs (#967) - `SfxDownloader::fetch()` now also fails fast with the directory-permissions message when a cached zip artifact sits in a destination directory that is no longer writable, instead of failing late per entry inside the extraction (a read-only directory that already holds every entry still re-extracts in place); a cached archive that does not open still reaches the poisoned-artifact cleanup path (#1052) - `StaticFilesMiddleware` now answers `HEAD` on a static file with headers only (`Content-Length`, `Content-Type`, `Accept-Ranges`, `ETag`, `Last-Modified`, `Cache-Control`, as for `GET`) and no body, and still returns 304 on a matching `If-None-Match`; methods other than `GET` and `HEAD` (`POST`, `PUT`, `DELETE`, ...) are passed to the application instead of returning the file, so the denylist and allowlist 404 answers now apply to `GET` and `HEAD` only (#903) - The CI matrix `sed` rewrite now matches digits in package names (`[a-z0-9-]+` instead of `[a-z-]+`), so a future `symfony/foo2bar`-style package is not silently skipped by the "Update Symfony constraints" step; `GithubWorkflowsTest` now replays the rewrite character class against every `symfony/*` key in `composer.json` (#1060) diff --git a/src/Reboot/Strategy/MaxJobsRebootStrategy.php b/src/Reboot/Strategy/MaxJobsRebootStrategy.php index 64b6a304..39258c04 100644 --- a/src/Reboot/Strategy/MaxJobsRebootStrategy.php +++ b/src/Reboot/Strategy/MaxJobsRebootStrategy.php @@ -20,6 +20,9 @@ public function __construct( public function shouldReboot(): bool { - return ++$this->jobsCount > $this->maxJobs; + // shouldReboot() is consulted after each request is handled, so + // the Nth request must trigger the reload (issue #967): with `>` + // the worker served maxJobs + 1 requests before reloading. + return ++$this->jobsCount >= $this->maxJobs; } } diff --git a/tests/RebootStrategyTest.php b/tests/RebootStrategyTest.php index d57b37bd..235c6c1a 100644 --- a/tests/RebootStrategyTest.php +++ b/tests/RebootStrategyTest.php @@ -29,7 +29,7 @@ public function testMaxJobsRebootStrategyReturnsFalseUntilLimit(): void $this->assertFalse($strategy->shouldReboot()); $this->assertFalse($strategy->shouldReboot()); - $this->assertFalse($strategy->shouldReboot()); + // Issue #967: the 3rd request triggers the reload, not the 4th. $this->assertTrue($strategy->shouldReboot()); } @@ -60,7 +60,7 @@ public function testMaxJobsRebootStrategyWithZeroDispersion(): void } } - $this->assertSame(6, $rebootAt, 'Should reboot after exactly 5 jobs with 0% dispersion'); + $this->assertSame(5, $rebootAt, 'Should reboot after exactly 5 jobs with 0% dispersion'); } public function testMaxJobsRebootStrategyWithSeededRandomizer(): void @@ -79,8 +79,9 @@ public function testMaxJobsRebootStrategyWithSeededRandomizer(): void } } - // With seeded randomizer, we should reboot at exactly 91 (90 + 1st job) - $this->assertSame(91, $rebootAt, 'Should reboot at deterministic job count with seeded randomizer'); + // With seeded randomizer, we should reboot at exactly 90: the + // drawn limit is 90 and the 90th job triggers the reload (issue #967). + $this->assertSame(90, $rebootAt, 'Should reboot at deterministic job count with seeded randomizer'); } public function testExceptionRebootStrategyReturnsFalseInitially(): void From 7a97d76cc501af6cee286e51e914ecb615277c7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Thu, 29 Oct 2026 20:40:41 +0100 Subject: [PATCH 2/2] docs: clarify worker reload threshold (#967) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07d0fd37..9d8788f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- `MaxJobsRebootStrategy::shouldReboot()` now reloads the worker after exactly `requests` requests instead of one request later (`>=` instead of `>`): the strategy is consulted after each request is handled, so the old check served `maxJobs + 1` requests before reloading, contradicting the docs (#967) +- `MaxJobsRebootStrategy::shouldReboot()` now requests a reload at the selected job limit, not one request later. With zero dispersion, the limit equals `requests` (#967) - `SfxDownloader::fetch()` now also fails fast with the directory-permissions message when a cached zip artifact sits in a destination directory that is no longer writable, instead of failing late per entry inside the extraction (a read-only directory that already holds every entry still re-extracts in place); a cached archive that does not open still reaches the poisoned-artifact cleanup path (#1052) - `StaticFilesMiddleware` now answers `HEAD` on a static file with headers only (`Content-Length`, `Content-Type`, `Accept-Ranges`, `ETag`, `Last-Modified`, `Cache-Control`, as for `GET`) and no body, and still returns 304 on a matching `If-None-Match`; methods other than `GET` and `HEAD` (`POST`, `PUT`, `DELETE`, ...) are passed to the application instead of returning the file, so the denylist and allowlist 404 answers now apply to `GET` and `HEAD` only (#903) - The CI matrix `sed` rewrite now matches digits in package names (`[a-z0-9-]+` instead of `[a-z-]+`), so a future `symfony/foo2bar`-style package is not silently skipped by the "Update Symfony constraints" step; `GithubWorkflowsTest` now replays the rewrite character class against every `symfony/*` key in `composer.json` (#1060)