fs: add synchronous retries to rimraf #30785

cjihrig · 2019-12-04T04:59:14Z

This PR builds on #30784 and #30783.

The third commit in this PR gives the synchronous version of rimraf the same linear retry logic as the asynchronous version. Prior to this commit, sync rimraf kept retrying the operation as soon as possible until maxRetries was reached.

Fixes: #30580
Refs: #30569

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

nodejs-github-bot · 2019-12-07T04:15:01Z

CI: https://ci.nodejs.org/job/node-test-pull-request/27440/ 💚


        fs: add synchronous retries to rimraf

This commit gives the synchronous version of rimraf the same linear retry logic as the asynchronous version. Prior to this commit, sync rimraf kept retrying the operation as soon as possible until maxRetries was reached.


        fixup! fs: add synchronous retries to rimraf

nodejs-github-bot · 2019-12-08T14:56:04Z

CI: https://ci.nodejs.org/job/node-test-pull-request/27488/


        fs: reduce unnecessary sync rimraf retries

rimraf should only retry if certain errors are encountered. Additionally, there is no point sleeping if an error occurs on the last try.

nodejs-github-bot · 2019-12-08T15:52:43Z

CI: https://ci.nodejs.org/job/node-test-pull-request/27489/

nodejs-github-bot · 2019-12-08T16:57:44Z

CI: https://ci.nodejs.org/job/node-test-pull-request/27493/

nodejs-github-bot · 2019-12-08T17:15:48Z

CI: https://ci.nodejs.org/job/node-test-pull-request/27494/

nodejs-github-bot · 2019-12-08T17:19:11Z

CI: https://ci.nodejs.org/job/node-test-pull-request/27495/

nodejs-github-bot · 2019-12-08T17:52:41Z

CI: https://ci.nodejs.org/job/node-test-pull-request/27498/

cjihrig · 2019-12-08T17:54:24Z

@Trott I don't think the CI is going to succeed until the out of space error in the git-nodesource-update-reference job is addressed.

Trott · 2019-12-08T17:57:35Z

@Trott I don't think the CI is going to succeed until the out of space error in the git-nodesource-update-reference job is addressed.

Don't confuse me with logic or data. You can't stop me from indiscriminately pressing "Resume Build" over and over.

cjihrig · 2019-12-08T19:27:09Z

OK, maybe I was wrong. The latest CI run failed on ARM, but only because of the git-nodesource-update-reference job. Does that mean this can land or what?

nodejs-github-bot · 2019-12-09T01:31:08Z

CI: https://ci.nodejs.org/job/node-test-pull-request/27506/ 💚


        fs: add synchronous retries to rimraf

This commit gives the synchronous version of rimraf the same linear retry logic as the asynchronous version. Prior to this commit, sync rimraf kept retrying the operation as soon as possible until maxRetries was reached. PR-URL: #30785 Fixes: #30580 Refs: #30569 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>


        fs: reduce unnecessary sync rimraf retries

rimraf should only retry if certain errors are encountered. Additionally, there is no point sleeping if an error occurs on the last try. PR-URL: #30785 Fixes: #30580 Refs: #30569 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>

danbev · 2019-12-09T06:24:49Z

Landed in 26991d0, and d7b8ae7.


        fs: add synchronous retries to rimraf

This commit gives the synchronous version of rimraf the same linear retry logic as the asynchronous version. Prior to this commit, sync rimraf kept retrying the operation as soon as possible until maxRetries was reached. PR-URL: #30785 Fixes: #30580 Refs: #30569 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>


        fs: reduce unnecessary sync rimraf retries

rimraf should only retry if certain errors are encountered. Additionally, there is no point sleeping if an error occurs on the last try. PR-URL: #30785 Fixes: #30580 Refs: #30569 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>


        fs: add synchronous retries to rimraf

This commit gives the synchronous version of rimraf the same linear retry logic as the asynchronous version. Prior to this commit, sync rimraf kept retrying the operation as soon as possible until maxRetries was reached. PR-URL: #30785 Fixes: #30580 Refs: #30569 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>


        fs: reduce unnecessary sync rimraf retries

rimraf should only retry if certain errors are encountered. Additionally, there is no point sleeping if an error occurs on the last try. PR-URL: #30785 Fixes: #30580 Refs: #30569 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>


        fs: add synchronous retries to rimraf

This commit gives the synchronous version of rimraf the same linear retry logic as the asynchronous version. Prior to this commit, sync rimraf kept retrying the operation as soon as possible until maxRetries was reached. PR-URL: #30785 Fixes: #30580 Refs: #30569 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>


        fs: reduce unnecessary sync rimraf retries

rimraf should only retry if certain errors are encountered. Additionally, there is no point sleeping if an error occurs on the last try. PR-URL: #30785 Fixes: #30580 Refs: #30569 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>


        fs: add synchronous retries to rimraf

This commit gives the synchronous version of rimraf the same linear retry logic as the asynchronous version. Prior to this commit, sync rimraf kept retrying the operation as soon as possible until maxRetries was reached. PR-URL: nodejs#30785 Fixes: nodejs#30580 Refs: nodejs#30569 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>


        fs: reduce unnecessary sync rimraf retries

rimraf should only retry if certain errors are encountered. Additionally, there is no point sleeping if an error occurs on the last try. PR-URL: nodejs#30785 Fixes: nodejs#30580 Refs: nodejs#30569 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>

nodejs-github-bot added the lib / src label Dec 4, 2019

cjihrig mentioned this pull request Dec 4, 2019

util: add util.sleep() #30784

Closed

4 of 4 tasks complete

Trott approved these changes Dec 4, 2019

View changes

addaleax added the blocked label Dec 4, 2019

cjihrig force-pushed the cjihrig:sync-retry branch Dec 7, 2019

cjihrig removed the blocked label Dec 7, 2019

Trott added the author ready label Dec 7, 2019

Trott mentioned this pull request Dec 7, 2019

test: harden tmpdir/rmdirSync error handling #30517

Closed

2 of 2 tasks complete

bnoordhuis reviewed Dec 8, 2019

View changes

lib/internal/fs/rimraf.js Outdated Show resolved Hide resolved

Trott removed the author ready label Dec 8, 2019

cjihrig mentioned this pull request Dec 8, 2019

test: work around ENOTEMPTY when cleaning tmp dir #30849

Closed

Trott added the author ready label Dec 8, 2019

cjihrig added 2 commits Dec 4, 2019

fixup! fs: add synchronous retries to rimraf

Verified

This commit was signed with a verified signature.

cjihrig Colin Ihrig

GPG key ID: 7434390BDBE9B9C5 Learn about signing commits

Loading status checks…

8c2ca28

cjihrig force-pushed the cjihrig:sync-retry branch to 8c2ca28 Dec 8, 2019

richardlau approved these changes Dec 8, 2019

View changes

danbev closed this Dec 9, 2019

cjihrig deleted the cjihrig:sync-retry branch Dec 9, 2019

MylesBorins mentioned this pull request Dec 13, 2019

v13.4.0 proposal #30937

Merged

targos mentioned this pull request Jan 15, 2020

v12.15.0 release proposal #31368

Closed

MylesBorins mentioned this pull request Feb 8, 2020

v12.16.0 proposal #31691

Merged

nodejs / node

fs: add synchronous retries to rimraf #30785

fs: add synchronous retries to rimraf #30785

cjihrig commented Dec 4, 2019 •

edited

This comment was marked as outdated.

nodejs-github-bot commented Dec 7, 2019 •

edited by cjihrig

This comment was marked as outdated.

nodejs-github-bot commented Dec 8, 2019

This comment was marked as outdated.

nodejs-github-bot commented Dec 8, 2019

This comment was marked as outdated.

nodejs-github-bot commented Dec 8, 2019

This comment was marked as outdated.

nodejs-github-bot commented Dec 8, 2019

This comment was marked as outdated.

nodejs-github-bot commented Dec 8, 2019

This comment has been minimized.

nodejs-github-bot commented Dec 8, 2019

This comment has been minimized.

cjihrig commented Dec 8, 2019

This comment has been minimized.

Trott commented Dec 8, 2019

This comment has been minimized.

cjihrig commented Dec 8, 2019

This comment has been minimized.

nodejs-github-bot commented Dec 9, 2019 •

edited by cjihrig

This comment has been minimized.

danbev commented Dec 9, 2019

nodejs / node

Join GitHub today

fs: add synchronous retries to rimraf #30785

fs: add synchronous retries to rimraf #30785

Conversation

cjihrig commented Dec 4, 2019 • edited

Checklist

This comment was marked as outdated.

nodejs-github-bot commented Dec 7, 2019 • edited by cjihrig

This comment was marked as outdated.

nodejs-github-bot commented Dec 8, 2019

This comment was marked as outdated.

nodejs-github-bot commented Dec 8, 2019

This comment was marked as outdated.

nodejs-github-bot commented Dec 8, 2019

This comment was marked as outdated.

nodejs-github-bot commented Dec 8, 2019

This comment was marked as outdated.

nodejs-github-bot commented Dec 8, 2019

This comment has been minimized.

nodejs-github-bot commented Dec 8, 2019

This comment has been minimized.

cjihrig commented Dec 8, 2019

This comment has been minimized.

Trott commented Dec 8, 2019

This comment has been minimized.

cjihrig commented Dec 8, 2019

This comment has been minimized.

nodejs-github-bot commented Dec 9, 2019 • edited by cjihrig

This comment has been minimized.

danbev commented Dec 9, 2019

cjihrig commented Dec 4, 2019 •

edited

nodejs-github-bot commented Dec 7, 2019 •

edited by cjihrig

nodejs-github-bot commented Dec 9, 2019 •

edited by cjihrig