[codex] Make Swoole worker recycling cooperative#4
Open
adhikjoshi wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR changes HTTP worker max-request recycling from Swoole's built-in
max_requestpath to an application-level cooperative recycle check that runs only after a request coroutine has unregistered itself.Root cause
Production evidence showed a worker recycle overlapping an in-flight
/api/v6/image_editing/qwen_editrequest. The worker logged graceful shutdown, but Nginx still sawrecv() failed (104: Connection reset by peer)from the upstream and returned 502. In coroutine mode, Swoole's built-in HTTP worker recycling can close the worker while another coroutine response is still being read upstream.What changed
max_requestandmax_request_graceto0, even if low-level Swoole options try to re-enable them.task_max_requestandtask_max_request_graceunchanged.WorkerState.StopWorkerIfMaxRequestsExceeded, which requests worker stop only when the max-request threshold has been reached andMonitor::getActiveRequestCount()is0.finallyblock after timer cleanup and request coroutine unregister.bin/WorkerState.phpso typed source classes and the new action can resolve it outside the Swoole entrypoint's manual require path.Validation
/opt/homebrew/Cellar/[email protected]/8.3.29/bin/php vendor/bin/phpunit129 tests, 974 assertionspassed. PHPUnit reports 11 existing deprecations./opt/homebrew/Cellar/[email protected]/8.3.29/bin/php vendor/bin/phpunit tests/Feature/SwooleCooperativeRecycleIntegrationTest.php3 tests, 107 assertionspassed./opt/homebrew/Cellar/[email protected]/8.3.29/bin/php vendor/bin/phpstan analyse src/Swoole/Actions/StopWorkerIfMaxRequestsExceeded.php --level=8 --memory-limit=1G --no-progresspassed./opt/homebrew/Cellar/[email protected]/8.3.29/bin/php /opt/homebrew/bin/composer validate --strictpassed.php vendor/bin/phpunit tests/Unit/StopWorkerIfMaxRequestsExceededTest.phpwith the default non-Swoole PHP binary passed.Notes for review
A broader PHPStan run over touched existing command/handler files still reports pre-existing baseline issues unrelated to this PR, including
HigherOrderTapProxy::start(),OnWorkerStart::bootWorker()missing a static-analysis return,Redis::purge(), and narrowedfunction_exists()checks. The new action itself is clean at PHPStan level 8.