-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
fix: enable retry concurrency limit for readFile() #19077
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for docs-eslint canceled.
|
lib/eslint/eslint.js
Outdated
const controller = new AbortController(); | ||
const retryCodes = new Set(["ENFILE", "EMFILE"]); | ||
const retrier = new Retrier(error => retryCodes.has(error.code)); | ||
const retrier = new Retrier(error => retryCodes.has(error.code), { signal: controller.signal, concurrency: 100 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I set concurrency to 100 to match @fasttime's prototype. I tried with values from 100-1000 and found performance was basically the same. We can always tweak this later if we the need.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
If possible, would like @fasttime to verify before today's release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT, thanks!
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
Updated
@humanwhocodes/retry
and set concurrency to 100.fixes #18977
Is there anything you'd like reviewers to focus on?
I didn't add any new tests for this because I couldn't come up with one that failed without it. Functionally, this should result in no difference to the end user and assuming our existing tests pass, I think we're okay.