Add retry logic to support temporary redis cluster failure #788
Conversation
|
|
||
| ++$retries; | ||
|
|
||
| sleep(2**$retries); |
There was a problem hiding this comment.
Sleeping for 2s seems aggressive, can we make this more granular.
There was a problem hiding this comment.
My last change introduce exponential backoff retries and $minRetryAfter
| private $strategy; | ||
| private $connections; | ||
| private $retryLimit = 5; | ||
| private $minRetryAfter = 1; // seconds |
There was a problem hiding this comment.
Let's call it $retryAfter and make it milliseconds, seconds is too coarse.
There was a problem hiding this comment.
its a float : you can use 0.2 if you want (usleep is used)
There was a problem hiding this comment.
I used the name $minRetryAfter to emphasize that it is a starting value and not a fixed value: the value is incremented exponentially.
tillkruss
left a comment
There was a problem hiding this comment.
I've made some changes. LMK if this is good for you.
$minRetryAfter cant be changed if needed with RedisCluster::setMinRetryAfter
…Connection\ConnectionException
735b46d to
0d24fb8
Compare
|
Hi tillkruss, |
This changes are trying to complete the retry logic in order to increase the tolerance to a temporary unavailability of a redis cluster.
Tests are slow (retries...), so I added the to group "slow":
vendor/bin/phpunit --group slow --filter RedisClusterTestRelated to PR #732