Give the worker the same NDJSON frame limit as the main process - #8489
Open
webard wants to merge 2 commits into
Open
Give the worker the same NDJSON frame limit as the main process#8489webard wants to merge 2 commits into
webard wants to merge 2 commits into
Conversation
Member
|
I'm not sure I understand this change. |
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.
A job chunk larger than 64 KB is dropped on arrival at the worker. The run then reports success, having analysed almost nothing:
Nine seconds, empty stderr, exit code 0 - and 6 files analysed out of 12 492. Counted by the entries written to the cache directory, because the console output gives no way to tell.
Root cause
The two ends of the connection disagree about how large a frame may be.
src/Parallel/Application/ParallelFileProcessor.php(main process) asks for 4 MB:src/Console/Command/WorkerCommand.php(worker) omits$maxlengthand gets theclue/ndjson-reactdefault of 64 KB:A job frame carries one chunk of absolute file paths, so its size follows path length, not file count. The ceiling is therefore project-specific and moves on its own: a deeper directory tree, a longer checkout prefix (a git worktree sits some forty characters below the main clone) or simply more files per directory all lower it.
Reproduction
12 762 files, 14 cores, PHP 8.5.10, average absolute path length 115 characters. Each timing repeated in both orders to rule out page-cache effects.
jobSize[OK] Rector is done!, exit 0The cliff is arithmetic, not load-dependent. The worst 600-path window in this project encodes to 64 868 B against the 65 536 B default - 1.01×, right at the edge, which is why 300 passes and 600 does not. Raising
timeoutSecondsto 900 changes nothing (still 6 files), so it is not the job timeout.To reproduce on any project: pick
jobSizesuch thatjobSize × (average absolute path length + 3)exceeds 65 536, then compare the number of cache entries written against the file count.Why this is not just "pick a smaller jobSize"
Two reasons.
First, there is no signal. The run is green, stderr is empty, and the progress bar reaches 100%. #8894 reported this same symptom in 2024 - "Rectorphp silently exits before completing the run", exit code 0, tied by the reporter to
jobSize- and was closed the same day for lack of a reproducible case. The concern raised there still stands: a CI pipeline treats that exit code as proof the code was refactored.Second, the default
jobSizeof 16 leaves a lot on the table - 461 s against 124 s in the table above, for identical work - so raising it is a reasonable thing for a user to try, and today that is a loaded gun.Precedent
Rector's parallel scheduling is derived from PHPStan (
ScheduleFactorysays so). PHPStan sets the limit on both ends from a single configurable value:ParallelAnalyser(main):maxlength: $this->decoderBufferSizeWorkerRunner(worker):maxlength: $this->decoderBufferSizeconf/config.neon:parallel: buffer: 134217728 # 128 MBRector's worker limit is currently 64× smaller than its own main process and 2048× smaller than PHPStan's default.