Skip to content

fix: reload worker after exactly maxJobs requests, not one later - #1108

Merged
s2x merged 4 commits into
crazy-goat:masterfrom
rootsiemen:pr-20261024-workerman-967
Oct 29, 2026
Merged

s2x merged 4 commits into
crazy-goat:masterfrom
rootsiemen:pr-20261024-workerman-967

Conversation

@rootsiemen

Copy link
Copy Markdown
Contributor

Closes #967

Problem

MaxJobsRebootStrategy::shouldReboot() used ++$this->jobsCount > $this->maxJobs. Since the strategy is consulted after each request is handled (HttpRequestHandler, after terminate/close), a worker served maxJobs + 1 requests before reloading — confirmed by the maintainer's repro in the issue (requests: 2 served 3 requests per worker). The docs already say the worker reloads after N requests.

Fix

  • shouldReboot() now uses >=, so the Nth request triggers the reload. No doc change was needed: docs/reload-strategies.md ("reloads after about requests requests"), docs/configuration.md and the ConfigurationTreeBuilder info text already describe the N-request behaviour ("about" covers the dispersion range).

Tests

  • testMaxJobsRebootStrategyReturnsFalseUntilLimit (maxJobs 3): now false, false, true — the 3rd request triggers the reload.
  • testMaxJobsRebootStrategyWithZeroDispersion (maxJobs 5): reboot point corrected from 6 to 5, matching its own assertion message.
  • testMaxJobsRebootStrategyWithSeededRandomizer: drawn limit 90 now reboots at the 90th call, not the 91st.
  • The dispersion-range test is unchanged and still passes (first reload still within the window).

Note: this environment has no PHP runtime, so the PHPUnit suite could not be run here — the change is a one-operator fix plus test-expectation updates, and CI's test job is the verification step. Please let me know if CI flags anything.

@s2x
s2x merged commit d17e371 into crazy-goat:master Oct 29, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] max_requests reloads one request later than the docs say

2 participants