Storage: Add Truncated exponential backoff for connection reset - #5
Storage: Add Truncated exponential backoff for connection reset#5sangramql wants to merge 7 commits into
Conversation
sangramql
commented
Jul 14, 2019
@mf2199 please review |
| for retry in range(self.retries): | ||
| try: | ||
| time.sleep(min(random.random() + 2 ** (retry - 1), 32)) | ||
| api_response = client._connection.api_request( |
There was a problem hiding this comment.
Why do we need a random number here? Why can it not be a fixed number, say, 1? Also, consider an extreme case when random.randon() == retry == 0. Then there will be no backoff whatsoever.
There was a problem hiding this comment.
Why do we need a random number here? Why can it not be a fixed number, say, 1?
Random number of milliseconds less than or equal to 1000. This helps to avoid cases where many clients get synchronized by some situation and all retry at once, sending requests in synchronized waves. The value of random_number_milliseconds is recalculated after each retry request.
Also, consider an extreme case when random.randon() == retry == 0. Then there will be no backoff whatsoever.
The wait time is min((random_number_milliseconds + (2^n)), maximum_backoff), with n incremented by 1 for each iteration (request).
So if random == retry == 0, there would be 1 secs delay, since 2**0 = 1
mf2199
commented
Jul 14, 2019
There are no tests for the added code which will reduce the coverage. |
sangramql
commented
Jul 15, 2019
Will be adding tests once approach and implementation is confirmed. |
mf2199
commented
Jul 15, 2019
The approach should be fine as long as it's properly tested. |
sangramql
commented
Jul 15, 2019
Ok, I'll add the tests. |
sangramql
commented
Jul 16, 2019
@ mf2199 please review the code. Unit tested. |
Issue [5879]
Cloud Functions & Storage: fails intermittently with ProtocolError + ConnectionResetError