EC2MetadataCredentials and ECSCredentials retry #1114

LiuJoyceC · 2016-08-26T17:33:58Z

Adds retry to sourcing credentials for IAM Roles in the EC2 Metadata Service or in ECS. By default will retry up to 3 times, with exponential backoff. Also for ECSCredentials, this PR will allow one asynchronous request to serve multiple refresh calls, so that at most only one request will be in flight at any point. Resolves #692

/cc: @chrisradek

…Credentials. Adds queuing of refresh callbacks to ECSCredentials to reduce the number of asynchronous requests to retrieve credentials.

coveralls · 2016-08-26T17:37:49Z

Coverage increased (+0.3%) to 89.198% when pulling 44fb727 on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

coveralls · 2016-08-26T17:37:49Z

Coverage increased (+0.3%) to 89.198% when pulling 44fb727 on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

coveralls · 2016-08-26T17:37:49Z

Coverage increased (+0.3%) to 89.198% when pulling 44fb727 on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

chrisradek · 2016-08-29T15:42:08Z

lib/config.js

@@ -100,7 +100,7 @@ require('./credentials/credential_provider_chain');
 *     Currently supported options are:
 *
 *     * **base** [Integer] &mdash; The base number of milliseconds to use in the
- *       exponential backoff for operation retries. Defaults to 100 ms.
+ *       exponential backoff for operation retries. Defaults to 30 ms.


The default used to be 30 ms but was changed to 100 ms with the configurable retry delays.

…es retry delay util function to use default 100ms base retry delay.

coveralls · 2016-08-30T00:29:36Z

Coverage increased (+0.3%) to 89.198% when pulling bb6745d on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

coveralls · 2016-08-30T00:29:36Z

Coverage increased (+0.3%) to 89.198% when pulling bb6745d on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

coveralls · 2016-08-30T00:29:36Z

Coverage increased (+0.3%) to 89.198% when pulling bb6745d on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

coveralls · 2016-08-30T00:30:59Z

Coverage increased (+0.3%) to 89.198% when pulling 90b410b on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

coveralls · 2016-08-30T19:13:13Z

Coverage increased (+0.3%) to 89.198% when pulling ea30bd7 on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

coveralls · 2016-08-30T21:29:26Z

Coverage increased (+0.3%) to 89.198% when pulling 1917d31 on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

chrisradek · 2016-08-31T17:16:54Z

lib/credentials/ecs_credentials.js

  },

  /**
+   * @api private
+   */
+  refreshQueue: [],


Why is the refresh queue only implemented for ECS credentials and not EC2 credentials? EC2 credentials has the same issue.

EC2MetdataCredentials already had this feature, through the MetadataService. The refreshQueue on the ECSCredentials is analogous to the loadCredentialsCallbacks on the MetadataService. They're structured slightly differently but work the same way. (refreshQueue has to retain references to the instances of ECSCredentials because the data isn't given to the callback the way it is in MetadataService).

Ah, so EC2 already supported this, excellent.

…des and retry-after headers

coveralls · 2016-09-01T02:45:56Z

Coverage increased (+0.3%) to 89.219% when pulling 39705b1 on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

chrisradek · 2016-09-01T18:03:40Z

lib/util.js

+          if (statusCode < 300) {
+            cb(null, data);
+          } else {
+            var retryAfter = parseInt(httpResponse.headers['retry-after']) * 1000 || 0;


Minor thing, but might want to specify that the int is a decimal number when parsing parseInt(str, 10) for a little extra safety.

Good catch, I'll modify it.

coveralls · 2016-09-01T22:03:20Z

Coverage increased (+0.3%) to 89.219% when pulling 783a1ab on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

coveralls · 2016-09-02T01:06:52Z

Coverage increased (+0.3%) to 89.219% when pulling 0c52b9f on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

coveralls · 2016-09-06T23:22:17Z

Coverage increased (+0.3%) to 89.219% when pulling e25adce on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

coveralls · 2016-09-06T23:22:17Z

Coverage increased (+0.3%) to 89.219% when pulling e25adce on LiuJoyceC:metadataServiceRetry into 7e3c279 on aws:master.

chrisradek · 2016-09-08T06:50:36Z

References: #1114, #1123, #603, #696

lock · 2019-09-28T12:56:58Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

LiuJoyceC added 2 commits August 26, 2016 00:45

Adds retry with exponential backoff to EC2MetadataCredentials and ECS…

ec1e757

…Credentials. Adds queuing of refresh callbacks to ECSCredentials to reduce the number of asynchronous requests to retrieve credentials.

Remove unnecessary lines of code in unit tests

44fb727

awsdave added the in progress label Aug 26, 2016

LiuJoyceC assigned chrisradek Aug 26, 2016

chrisradek reviewed Aug 29, 2016
View reviewed changes

LiuJoyceC added 3 commits August 29, 2016 17:14

Modifies tests to be compatible with Node 0.10

bb6745d

Reverts documentation back to saying 100ms as base retry delay. Updat…

90b410b

…es retry delay util function to use default 100ms base retry delay.

Updates unit test

ea30bd7

Adds description to changelog for Metadata Service retry.

1917d31

chrisradek reviewed Aug 31, 2016
View reviewed changes

Adds retrying to MetadataService and ECSCredentials for 5xx status co…

39705b1

…des and retry-after headers

chrisradek reviewed Sep 1, 2016
View reviewed changes

Restricts retrying for 400 error codes to just 429

783a1ab

Corrects unit test

0c52b9f

Minor update

e25adce

LiuJoyceC merged commit 9f08143 into aws:master Sep 8, 2016

awsdave removed the in progress label Sep 8, 2016

chrisradek added a commit that referenced this pull request Sep 9, 2016

Tag release v2.6.0

8fd2060

References: #1114, #1123, #603, #696

springmeyer mentioned this pull request Mar 20, 2017

Require at least aws-sdk >= 2.6.0 mapbox/tilelive-s3#92

Merged

alexdrans mentioned this pull request Aug 18, 2017

Upgrading aws-sdk dependency bbc/sqs-consumer#90

Merged

lock bot locked as resolved and limited conversation to collaborators Sep 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EC2MetadataCredentials and ECSCredentials retry #1114

EC2MetadataCredentials and ECSCredentials retry #1114

LiuJoyceC commented Aug 26, 2016

coveralls commented Aug 26, 2016 •

edited

Loading

coveralls commented Aug 26, 2016

coveralls commented Aug 26, 2016

chrisradek Aug 29, 2016

coveralls commented Aug 30, 2016

coveralls commented Aug 30, 2016 •

edited

Loading

coveralls commented Aug 30, 2016

coveralls commented Aug 30, 2016 •

edited

Loading

coveralls commented Aug 30, 2016 •

edited

Loading

coveralls commented Aug 30, 2016 •

edited

Loading

chrisradek Aug 31, 2016

LiuJoyceC Aug 31, 2016 •

edited

Loading

chrisradek Aug 31, 2016

coveralls commented Sep 1, 2016 •

edited

Loading

chrisradek Sep 1, 2016

LiuJoyceC Sep 6, 2016

coveralls commented Sep 1, 2016 •

edited

Loading

coveralls commented Sep 2, 2016 •

edited

Loading

coveralls commented Sep 6, 2016 •

edited

Loading

coveralls commented Sep 6, 2016 •

edited

Loading

chrisradek commented Sep 8, 2016

lock bot commented Sep 28, 2019

EC2MetadataCredentials and ECSCredentials retry #1114

EC2MetadataCredentials and ECSCredentials retry #1114

Conversation

LiuJoyceC commented Aug 26, 2016

coveralls commented Aug 26, 2016 • edited Loading

coveralls commented Aug 26, 2016

coveralls commented Aug 26, 2016

chrisradek Aug 29, 2016

Choose a reason for hiding this comment

coveralls commented Aug 30, 2016

coveralls commented Aug 30, 2016 • edited Loading

coveralls commented Aug 30, 2016

coveralls commented Aug 30, 2016 • edited Loading

coveralls commented Aug 30, 2016 • edited Loading

coveralls commented Aug 30, 2016 • edited Loading

chrisradek Aug 31, 2016

Choose a reason for hiding this comment

LiuJoyceC Aug 31, 2016 • edited Loading

Choose a reason for hiding this comment

chrisradek Aug 31, 2016

Choose a reason for hiding this comment

coveralls commented Sep 1, 2016 • edited Loading

chrisradek Sep 1, 2016

Choose a reason for hiding this comment

LiuJoyceC Sep 6, 2016

Choose a reason for hiding this comment

coveralls commented Sep 1, 2016 • edited Loading

coveralls commented Sep 2, 2016 • edited Loading

coveralls commented Sep 6, 2016 • edited Loading

coveralls commented Sep 6, 2016 • edited Loading

chrisradek commented Sep 8, 2016

lock bot commented Sep 28, 2019

coveralls commented Aug 26, 2016 •

edited

Loading

coveralls commented Aug 30, 2016 •

edited

Loading

coveralls commented Aug 30, 2016 •

edited

Loading

coveralls commented Aug 30, 2016 •

edited

Loading

coveralls commented Aug 30, 2016 •

edited

Loading

LiuJoyceC Aug 31, 2016 •

edited

Loading

coveralls commented Sep 1, 2016 •

edited

Loading

coveralls commented Sep 1, 2016 •

edited

Loading

coveralls commented Sep 2, 2016 •

edited

Loading

coveralls commented Sep 6, 2016 •

edited

Loading

coveralls commented Sep 6, 2016 •

edited

Loading