-
Notifications
You must be signed in to change notification settings - Fork 407
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
Use exponential backoff with jitter on 429 #110
Use exponential backoff with jitter on 429 #110
Conversation
23702f6
to
f0669fb
Compare
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.
Looks good other than my small comments!
lib/run_action.js
Outdated
@@ -6,7 +6,21 @@ var objectToQueryParamString = require('./object_to_query_param_string'); | |||
// This will become require('xhr') in the browser. | |||
var request = require('request'); | |||
|
|||
/* | |||
* "Full Jitter" algorithm taken from https://aws.amazon.com/blogs/architecture/exponential-backoff-and-jitter/ |
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.
Nit: because this is just one line, we can use a //
comment.
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.
👍
lib/run_action.js
Outdated
return runActionImpl(base, method, path, queryParams, bodyData, callback, 0); | ||
} | ||
|
||
function runActionImpl(base, method, path, queryParams, bodyData, callback, numAttempts) { |
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.
Because we only call runAction
in one place (from Base.prototype.runAction
), I'd prefer to make runAction
take numAttempts
and call it properply from Base.prototype.runAction
.
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.
👍
This can happen if there are too many clients trying to access the same base. To accomodate this, use exponential backoff instead of a fixed backoff.
f0669fb
to
a572e2c
Compare
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.
Looks good!
How is the retry mechanism handle the sequencing of requests? Is it somewhat unpredictable or it will always be retried in the order when it's initially fired, during the period of rate limit? |
@kaz875 When a request (such as a request to create a record) fails with a 429 status code, it will be retried. As far as Airtable.js is concerned, other requests are not taken into account, and we rely on the JavaScript event loop to handle this. |
This can happen if there are too many clients trying to access the same base.
To accomodate this, use exponential backoff instead of a fixed backoff.