Implementing PhpRedis client #36
Conversation
jeffreyzant
force-pushed
the
phpredis
branch
4 times, most recently
from
7897b9e to
028b794
Compare
jeffreyzant
force-pushed
the
phpredis
branch
4 times, most recently
from
83b0947 to
11a7bc9
Compare
jeffreyzant
changed the title
jeffreyzant
changed the title
|
|
||
| // The default amount of time (in milliseconds) that the client waits before | ||
| // retrying the connection attempt. | ||
| 'connector_retry_wait' => 1000, |
There was a problem hiding this comment.
How do these configuration paramters play together with retry_limit and retry_wait? Would it be possible to allow the PhpRedisConnection to reuse the connector logic (and retry configuration) of PhpRedisConnector?
There was a problem hiding this comment.
These settings allow for a different configuration on first connection. So the normal retry_limit and retry_wait are used for reconnecting after a failure. The connector_ options are used when creating the connection for the first time.
I've combined some of the retry logic, but I can't seem to find a way to simplify this. Maybe you have some ideas about it?
There was a problem hiding this comment.
I think the settings are fine, just a bit hard to understand in combination with the other ones (which have quite similar names too). Explaining their difference from the similar named settings is probably all you can do at this point.
|
@Namoshek thanks for your reply, I will try to work on your comments next week. We are already using this PR in our Kubernetes cluster :) |
|
Hi! Any ETA when this will be merge to main tree and tag released ? |
|
@cyrossignol can we see this merged |
| public function connect(array $servers, array $options = [ ]) | ||
| { | ||
| // Set the initial Sentinel servers. | ||
| $this->servers = array_map(function ($server) { |
There was a problem hiding this comment.
We should extract the options key before setting the servers or it can result trying to connect to the wrong hostname here: https://github.com/monospice/laravel-redis-sentinel-drivers/pull/36/files#diff-3bf85f59585710cebd9ae7787d8b7ffb7df273ce8ffe2e72053ef96b4bb81ccaR157
|
|
||
| // Create a client by calling the Sentinel servers | ||
| $connector = function () use ($options) { | ||
| return $this->createClientWithSentinel($options); |
There was a problem hiding this comment.
you are using $options instead of $clientOpts
This PR implements the PhpRedis client for Redis Sentinel. It wraps all methods with a retryOnFailure and creates a new Redis client when the connection fails.