-
-
Notifications
You must be signed in to change notification settings - Fork 993
Add retry logic to support temporary redis cluster failure (\Predis\C… #732
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
Add retry logic to support temporary redis cluster failure (\Predis\C… #732
Conversation
…onnection\Aggregate\RedisCluster)
82b1826 to
6f6c785
Compare
|
No worries. @nrk Any input on this? |
|
Hi, |
1 similar comment
|
Hi, |
|
@digital2real Danielle seems MIA. Can you add unit tests for your changes that cover all new scenarios? |
| } | ||
|
|
||
| ++$retries; | ||
| sleep(2^$retries); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation seems off.
| { | ||
| $failure = false; | ||
|
|
||
| $retries= 0 ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs formatting.
| $connection->disconnect(); | ||
|
|
||
| $this->remove($connection); | ||
| if (is_a($response, '\\Predis\\Response\\Error')){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use instanceof please.
| throw new ServerException($message) ; | ||
| } | ||
| } | ||
| } catch (ConnectionException|ServerException $exception) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this syntax is compatible with olde PHP versions.
|
Feel free to re-submit to the 2.0 |
This changes are trying to complete the retry logic in order to increase the tolerance to a temporary unavailability of a redis cluster.
(Sorry for previous PR #731 on main)