-
Notifications
You must be signed in to change notification settings - Fork 69
fix(cli): requests get throttled when there is a large number of stacks #927
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
fix(cli): requests get throttled when there is a large number of stacks #927
Conversation
Limit concurrency to 20.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #927 +/- ##
==========================================
+ Coverage 83.45% 83.95% +0.49%
==========================================
Files 71 71
Lines 10401 10401
Branches 1317 1324 +7
==========================================
+ Hits 8680 8732 +52
+ Misses 1680 1628 -52
Partials 41 41
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| }; | ||
| }; | ||
|
|
||
| const limit = pLimit(20); |
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.
Still seems like a high number?
Are the retry settings configured thoroughly enough for the CFN client?
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 is the current retry strategy for the CFN client:
new ConfiguredRetryStrategy(11, (attempt: number) => 1000 * (2 ** attempt))I ran an experiment with this strategy, against an account with 342 templates. I didn't start getting throttling errors until concurrency = 47. So, 20 might be playing it too safely, actually.
Just as a sanity check, without any retry strategy, even with concurrency = 2, I would sometimes get a throttling error.
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.
SGTM, thanks!
The CLI is making an unlimited number of concurrent
GetTemplaterequests. When there is a large number of stacks, CloudFormation may throttle the requests.Limit concurrency to 20.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license