Skip to content
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

Proposal: changes to rimraf retry options #30580

Closed
cjihrig opened this issue Nov 21, 2019 · 0 comments
Closed

Proposal: changes to rimraf retry options #30580

cjihrig opened this issue Nov 21, 2019 · 0 comments

Comments

@cjihrig
Copy link
Contributor

cjihrig commented Nov 21, 2019

The recursive rmdir() currently supports two options:

  • emfileWait - When EMFILE (and soon ENFILE) is encountered, the operation is retried. The retry logic involves creating a series of timers, where the delay increases by 1ms until emfileWait is reached. This is only used in the asynchronous rimraf, and once it supports ENFILE, the name isn't really accurate anymore. This currently defaults to 1000.
  • maxBusyTries - This option is used when EBUSY, ENOTEMPTY, or EPERM is encountered. This retry logic involves creating a series of maxBusyTries timers, where the delay increases by 100ms. This is also only used in the asynchronous rimraf. This currently defaults to 3.

I'm proposing the following changes (note that rimraf is still experimental):

  • Rename maxBusyTries to maxRetries and make the default 0. By making the default 0, I think we can start applying this to the synchronous rimraf. I think retries should be opt-in, particularly for synchronous operations, which can block the event loop for a while. I also think that the async and sync versions should have consistent handling of retry logic.
  • Remove emfileWait completely, and use the same retry logic consistently.
  • Introduce a retryDelay option that specifies the amount of time to wait between retry attempts.
cjihrig added a commit to cjihrig/node that referenced this issue Nov 27, 2019
This is part of reworking the rimraf retry logic.

Refs: nodejs#30580
PR-URL: nodejs#30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
cjihrig added a commit to cjihrig/node that referenced this issue Nov 27, 2019
This commit makes retries an opt-in feature by defaulting
to no automatic retries. This will be particularly important
once synchronous operations can sleep between attempts.

Refs: nodejs#30580
PR-URL: nodejs#30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
cjihrig added a commit to cjihrig/node that referenced this issue Nov 27, 2019
This commit removes the emfileWait option. EMFILE errors are
now handled the same as any other retriable error.

Refs: nodejs#30580
PR-URL: nodejs#30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
cjihrig added a commit to cjihrig/node that referenced this issue Nov 27, 2019
This commit adds a retryDelay option to rimraf which configures
the amount of time between retry operations.

Refs: nodejs#30580
PR-URL: nodejs#30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
cjihrig added a commit to cjihrig/node that referenced this issue Nov 27, 2019
Co-authored-by: Thang Tran <trankimthang279@gmail.com>
Fixes: nodejs#30482
Refs: nodejs#30499
Refs: nodejs#30580
PR-URL: nodejs#30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Nov 30, 2019
This is part of reworking the rimraf retry logic.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Nov 30, 2019
This commit makes retries an opt-in feature by defaulting
to no automatic retries. This will be particularly important
once synchronous operations can sleep between attempts.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Nov 30, 2019
This commit removes the emfileWait option. EMFILE errors are
now handled the same as any other retriable error.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Nov 30, 2019
This commit adds a retryDelay option to rimraf which configures
the amount of time between retry operations.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Nov 30, 2019
Co-authored-by: Thang Tran <trankimthang279@gmail.com>
Fixes: #30482
Refs: #30499
Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@danbev danbev closed this as completed in 26991d0 Dec 9, 2019
danbev pushed a commit that referenced this issue Dec 9, 2019
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>
targos pushed a commit that referenced this issue Dec 9, 2019
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>
targos pushed a commit that referenced this issue Dec 9, 2019
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>
targos pushed a commit that referenced this issue Jan 13, 2020
This is part of reworking the rimraf retry logic.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jan 13, 2020
This commit makes retries an opt-in feature by defaulting
to no automatic retries. This will be particularly important
once synchronous operations can sleep between attempts.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jan 13, 2020
This commit removes the emfileWait option. EMFILE errors are
now handled the same as any other retriable error.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jan 13, 2020
This commit adds a retryDelay option to rimraf which configures
the amount of time between retry operations.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jan 13, 2020
Co-authored-by: Thang Tran <trankimthang279@gmail.com>
Fixes: #30482
Refs: #30499
Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jan 14, 2020
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>
targos pushed a commit that referenced this issue Jan 14, 2020
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>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
This is part of reworking the rimraf retry logic.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
This commit makes retries an opt-in feature by defaulting
to no automatic retries. This will be particularly important
once synchronous operations can sleep between attempts.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
This commit removes the emfileWait option. EMFILE errors are
now handled the same as any other retriable error.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
This commit adds a retryDelay option to rimraf which configures
the amount of time between retry operations.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
Co-authored-by: Thang Tran <trankimthang279@gmail.com>
Fixes: #30482
Refs: #30499
Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
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>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant