Storage: Add Truncated exponential backoff for connection reset #5
Conversation
|
@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
|
There are no tests for the added code which will reduce the coverage. |
Will be adding tests once approach and implementation is confirmed. |
|
The approach should be fine as long as it's properly tested. |
Ok, I'll add the tests. |
|
@ mf2199 please review the code. Unit tested. |
Issue [5879]
Cloud Functions & Storage: fails intermittently with ProtocolError + ConnectionResetError