fix: retry filesystem operations on EAGAIN #9959
Conversation
|
💖 Thanks for opening this pull request! 💖 |
Qubasa
force-pushed
the
eagain_fix
branch
2 times, most recently
from
df6f811 to
b74a8b0
Compare
Qubasa
changed the title
|
Doesn't https://www.npmjs.com/package/graceful-fs already do the retries? Which we use for these functions. |
|
@zkochan I just looked into the libraries source code https://github.com/isaacs/node-graceful-fs/blob/main/graceful-fs.js the first one being it only patches the async versions of the |
|
@zkochan Could you help me out in fixing the CI? I can't see the error messages causing the failure and I don't seem to be able to run the tests locally on a clean branch without failing. I tried running |
|
What about fs-extra? Does that retry it? |
|
The tests are failing in @pnpm/fs.indexed-pkg-importer, which has a lot of mocking. You probably need to update the mocks of some fs functions. |
|
@zkochan No sadly fs-extra does not handle EAGAIN :( |
How can I run these specific tests? |
|
Qubasa
force-pushed
the
eagain_fix
branch
2 times, most recently
from
2b4c4dc to
3c35875
Compare
|
@zkochan Thanks! The tests should pass now! |
Qubasa
force-pushed
the
eagain_fix
branch
2 times, most recently
from
d0d259a to
6f56fa0
Compare
|
@zkochan Wohps my bad, there was type lint ignore that got removed, but now it should pass ;) |
fs/hard-link-dir/src/index.ts
Outdated
| const linkSyncWithRetry = withEagainRetry(fs.linkSync) | ||
| const copyFileWithRetry = withEagainRetry(fs.copyFileSync) | ||
| const mkdirSyncWithRetry = withEagainRetry(fs.mkdirSync) |
There was a problem hiding this comment.
why not just moving these to graceful-fs exporting the functions with retries by default?
There was a problem hiding this comment.
@zkochan Good point, changed it accordingly.
filesystem operations can raise EAGAIN to tell the application to try again later. This is especially often the case under ZFS. fix: move wrapped functions to graceful-fs directly
fs/graceful-fs/src/index.ts
Outdated
| export const mkdirSyncWithRetry = withEagainRetry(fs.mkdirSync) | ||
| export const writeFileWithRetry = withEagainRetry(fs.writeFileSync) | ||
| export const linkSyncWithRetry = withEagainRetry(fs.linkSync) | ||
| export const copyFileWithRetry = withEagainRetry(fs.copyFileSync) |
There was a problem hiding this comment.
why not
| export const copyFileWithRetry = withEagainRetry(fs.copyFileSync) | |
| export const copyFileWithRetry = withEagainRetry(gfs.copyFileSync) |
which has some retries as well, which may be useful
fs/graceful-fs/src/index.ts
Outdated
|
|
||
| export default { // eslint-disable-line | ||
| copyFile: promisify(gfs.copyFile), | ||
| copyFileSync: gfs.copyFileSync, |
There was a problem hiding this comment.
just replace this. No need to have copyFileWithRetry separately.
|
Congrats on merging your first pull request! 🎉🎉🎉 |
3 participants
Filesystem operation can return EAGAIN to tell the applicaiton to try again later.
This PR implements a retry logic to filesystem calls with an exponential backoff. Meaning if a
fs.function failed with EAGAIN it will retry up to 15 times with increasing wait times inbetween, the maximum wait time being 300 miliseconds.This error seems to be raised often on ZFS.
This PR fixes
ERR_PNPM_EAGAIN EAGAIN: resource temporarily unavailablehoppscotch/hoppscotch#4960I successfully tested this PR on ZFS with Nix for the NuschtOS project https://github.com/NuschtOS/search