Provide backoff for retryable aws requests and the option to adjust the cf status check interval via an environment variable

[philiiiiiipp]

What did you implement

Closes #6980

I did create that inside the github console, given the simple logic I do believe that its fine. But those might also be famous last words!

Is this ready for review?: YES
Is it a breaking change?: NO

[codecov-io]

Codecov Report

Merging #6981 into master will increase coverage by 0.07%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #6981      +/-   ##
=========================================
+ Coverage   88.42%   88.5%   +0.07%     
=========================================
  Files         229     229              
  Lines        8416    8428      +12     
=========================================
+ Hits         7442    7459      +17     
+ Misses        974     969       -5

Impacted Files	Coverage Δ
lib/plugins/aws/provider/awsProvider.js	90.94% <100%> (+0.3%)	⬆️
lib/plugins/aws/lib/monitorStack.js	100% <100%> (ø)	⬆️
lib/utils/downloadTemplateFromRepo.js	96.22% <0%> (-0.04%)	⬇️
.../aws/package/compile/events/apiGateway/lib/cors.js	100% <0%> (ø)	⬆️
...kage/compile/events/apiGateway/lib/method/index.js	100% <0%> (ø)	⬆️
...rces/resources/apiGatewayCloudWatchRole/handler.js	0% <0%> (ø)	⬆️
...aws/package/compile/events/websockets/lib/stage.js	100% <0%> (ø)	⬆️
...s/customResources/resources/eventBridge/handler.js	0% <0%> (ø)	⬆️
lib/plugins/aws/customResources/resources/utils.js	21.21% <0%> (ø)	⬆️
...stomResources/resources/cognitoUserPool/handler.js	0% <0%> (ø)	⬆️
... and 11 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d33ee52...69465f1. Read the comment docs.

[medikoo]

Thank you @philiiiiiipp !

Still let's first clarify use case at #6980, before moving forward with this

[medikoo]

Thank you @philiiiiiipp In general looks good. I've added few minor remarks

[philiiiiiipp]

I somehow cannot answer on your comments anymore. I believe that last unresolved one was about the backoff.

In my opinion its a good pattern to have, but having a jitter in combination with the ability to increase the interval at the other place would suffice for my use case at least. I am not that knowledgeable in the codebase though, so if you prefer not having an increasing backoff, I am cool with that too.

[medikoo]

In my opinion its a good pattern to have

Can you elaborate why you see it useful in that case?

To me backoff serves well cases where we deal with a long wait time, and there's no point to check often. While here we deal with cases where there are short requests, and issue is that in some cases they're issued at very same time, so with fixed frequency they clash. In such case jitter seems the only solution that could indeed help,

I would like to avoid adding functionalities that do not serve real purpose (do not complicate things when it's not justified)

[philiiiiiipp]

Right, we are doing requests against an aws endpoint, which might be congested, not only because of sls, but in general because there are many other clients trying to reach it. So if you want to allow the endpoint to recover it makes sense to slowly back off until your retries are used up.

https://docs.aws.amazon.com/general/latest/gr/api-retries.html

Like I said I am not married to this, I am more eager to get the jitter and the custom frequency option in, in order to stop using our forked version. So if you feel that this makes more harm than good I will remove it and we can always revisit it at a later stage.

[medikoo]

So if you feel that this makes more harm than good I will remove it

My feeling is that adding extra 5s every time, in this specific case, will rather bring experience downgrade than upgrade.

I would also narrow wait range to something as 4-7s. The way it's configured now is 5-10s, so again that will make waits noticeably longer from what we have in master.

Technically I think it'll be good to focus in this PR purely on adding jitter, and maintain interval time on similar level as we have now.

The other reason I'm reluctant to make any bigger changes here, is that I don't remember having any other reports, that concerns that retry logic. So ideal approach seems to fix it for you by adding a jitter, but at same time maintain similar interval times, to keep experience not different for all other framework users.

[philiiiiiipp]

I would also narrow wait range to something as 4-7s. The way it's configured now is 5-10s, so again that will make waits noticeably longer from what we have in master.

Technically I think it'll be good to focus in this PR purely on adding jitter, and maintain interval time on similar level as we have now.

The other reason I'm reluctant to make any bigger changes here, is that I don't remember having any other reports, that concerns that retry logic. So ideal approach seems to fix it for you by adding a jitter, but at same time maintain similar interval times, to keep experience not different for all other framework users.

Right, that makes sense, I adjusted the interval to 4-7 seconds and removed the increasing backoff.

[pmuens]

Thanks for working on this @philiiiiiipp and thanks for reviewing @medikoo 👍

I just ran Prettier to make the build pass on Travis. Other than that I looked through the code and it looks good!

@medikoo could you do a final review and merge once you think all your concerns were addressed?

[medikoo]

Thank you @philiiiiiipp !