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

Added exponential backoff of calls to api gateway api #1645

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

drem-darios
Copy link

Starts at 2 seconds and retries 5 times while backing off exponentially.

Q A
πŸ› Bug fix? yes
πŸš€ New feature? no
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues #1344
❓ Documentation no

Description

Added exponential backoff of calls to api gateway api to avoid TooManyCallsException. Calls that fail will sleep for 2 seconds before retrying. Every failure up to the max retries (default 5) takes exponentially more time than the previous sleep time.

You can see here: https://docs.aws.amazon.com/apigateway/latest/developerguide/limits.html that API Gateway allows 5 requests every 2 seconds per account for GetResources and I was hitting that limit pretty frequently. There is also a 10 request per second limit across all API Gateway management operations. To work around those limits, I added some code to the api_gateway_repository.go file that would exponentially backoff the requests in the case that we received a "TooManyRequestsException" error. I set the bar at 2 seconds since that was the limit we were hitting. Also, I had to add this logic to every function making a request to API Gateway since any of them could trigger the total operation limitation. (e.g. GetRestApisPage reaches the limit, then a call to say GetAccount will trigger the throttle).

This logic should probably be implemented in other places/repositories too...

Starts at 2 seconds and retries 5 times while backing off exponentially.
@drem-darios drem-darios requested a review from a team as a code owner March 18, 2023 04:00
@drem-darios drem-darios requested review from eliecharra and removed request for a team March 18, 2023 04:00
@CLAassistant
Copy link

CLAassistant commented Mar 18, 2023

CLA assistant check
All committers have signed the CLA.

@codecov-commenter
Copy link

Codecov Report

Merging #1645 (38ad4d2) into main (a4cb2eb) will decrease coverage by 1.63%.
The diff coverage is n/a.

πŸ“£ This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1645      +/-   ##
==========================================
- Coverage   79.64%   78.02%   -1.63%     
==========================================
  Files         229      222       -7     
  Lines        7183     7022     -161     
==========================================
- Hits         5721     5479     -242     
- Misses       1268     1345      +77     
- Partials      194      198       +4     

see 26 files with indirect coverage changes

@mfractal
Copy link

mfractal commented Jul 3, 2023

when is this planned to get merged ? been hitting 'ThrottlingException: Rate exceeded' all the time recently running driftctl

@markus-geiger
Copy link

markus-geiger commented Nov 6, 2023

I merged here https://github.com/markus-geiger/driftctl it runs longer but still I do get:

Throttling: Rate exceeded
	status code: 400, request id: a01c4539-04c7-47ec-ab6f-aa52b31f0768

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 this pull request may close these issues.

5 participants