Skip to content

Check the configuration cache only in the main process - #8851

Merged
samsonasik merged 2 commits into
rectorphp:mainfrom
SanderMuller:worker-skip-config-check
Oct 26, 2026
Merged

samsonasik merged 2 commits into
rectorphp:mainfrom
SanderMuller:worker-skip-config-check

Conversation

@SanderMuller

@SanderMuller SanderMuller commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #8850.

Each parallel worker runs the configuration check when it boots. If its hash differs from the main process, it clears the whole cache while other workers write to it. On 2.8.0 that sometimes crashed the run. Now only the main process runs the check, before the workers start.

Tested on CodeIgniter4 with 2.8.0 without #8850: a warm run takes about 0.9 s instead of 10 s.

The new workflow runs the compiled rector/rector:dev-main twice on a small project and fails if the second run clears the cache. It runs after each merge and daily, because the compiled build only exists after a merge. On my fork it fails with 2.8.0 and passes with 2.7.0 and dev-main: https://github.com/SanderMuller/rector-src/actions/runs/40331633254

The parallel workers share the cache directory of the main process.
A worker that sees a changed configuration clears the whole cache,
and deletes files that the other workers write at the same time.
The second run must keep the cache. On 2.8.0 it did not.
@samsonasik

Copy link
Copy Markdown
Member

@SanderMuller not yet, feel free to add it 👍

@SanderMuller

Copy link
Copy Markdown
Contributor Author

@SanderMuller not yet, feel free to add it 👍

Added!

@samsonasik samsonasik left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, the existing 2.8.1 already patch race cache condition internally on SimpleParameterProvider

so this patch release can wait for include with other patches :)

@samsonasik
samsonasik merged commit 635bff7 into rectorphp:main Oct 26, 2026
50 checks passed
@samsonasik

Copy link
Copy Markdown
Member

Let's give it a try, thank you @SanderMuller

@TomasVotruba

Copy link
Copy Markdown
Member

Thank you 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants