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.

Sign up for GitHub

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

Jump to bottom

Fix handling of retry-able requests 429 and 503 #200

Closed
nvthongswansea opened this issue Apr 27, 2021 · 3 comments
Closed

Fix handling of retry-able requests 429 and 503 #200

nvthongswansea opened this issue Apr 27, 2021 · 3 comments
Assignees
@nvthongswansea
Labels
bug Something isn't working enhancement New feature or request

Comments

@nvthongswansea
Copy link
Member

A rate-limited request should be retried until the rate-limit is reset and the request is processed successfully. If the request is delayed again (due to rate-limit is reached again), keep retrying.
@nvthongswansea nvthongswansea self-assigned this Apr 27, 2021
@nvthongswansea
Copy link
Member Author

@bkircher any thoughts?

@bkircher
Copy link
Contributor

bkircher commented Apr 28, 2021 •
edited
Loading

Yip, this is how it should be. Remember that polling on GET /requests/<id> also count.

@bkircher
Copy link
Contributor

Yes. In gridscale/terraform-provider-gridscale#157 discussion, we found that gsclient-go handling of retries is totally wrong here.

As per API doc, 429 should re-tried indefinitely, blocking (given proper handling of signals like SIGINT and friends) until RateLimit-Reset is reached.

503 should also be retried but blocking on Retry-Aftervalue. There a max_n_retries value might make sense, so maybe try 5 times per default on 503 and give up after that.

As for

  • 409
  • 424 and
  • 500

Those should not be retried at all.

@bkircher bkircher added bug Something isn't working enhancement New feature or request labels Apr 29, 2021
@bkircher bkircher changed the title Retrying rate-limited requests should not be limited to N times. Fix handling of retry-able requests 429 and 503 Apr 29, 2021
@nvthongswansea nvthongswansea mentioned this issue May 5, 2021
Merged
bkircher pushed a commit to gridscale/terraform-provider-gridscale that referenced this issue May 9, 2021
@nvthongswansea
Allow customizing request delay and maximum number of retries
1887dbc 
Allow setting delay between requests and the maximum number
of retries. The default no. of retries is set to 1.

With this commit, TF provider reads two new environment
variables:
- GRIDSCALE_TF_REQUEST_DELAY_INTERVAL
- GRIDSCALE_TF_MAX_N_RETRIES

References:
- gridscale/gsclient-go#200
bkircher pushed a commit that referenced this issue May 9, 2021
@nvthongswansea
Fix handling of retry-able requests 429 and 503
e207b7d 
Only 503 and 429 are retriable. Changes:

See issue #200.

Requests with 503 response code will be retried. If Retry-After response header is defined (x seconds), the next retry will be executed after x seconds. If that header is not included, the next retry depends on settings of gsclient-go (delayIntervalMilliSecs and maxNumberOfRetries). When the retry count of a request reaches maxNumberOfRetries, gsclient-go return the last request's error.

Requests with 429 (rate-limit is reached) response code will also be retried when the rate-limit is reset. If the rate-limit is reached again, retry again in when the rate-limit is reset. There is no maxNumberOfRetries for 429 request retry.

NOTE: if the context is Done() before retrying (or during retrying). An error caused by the expired context is returned.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nvthongswansea nvthongswansea

Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bkircher @nvthongswansea