From 9079c94f17e6a33f3941bb86e0769c81ad6c7893 Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Fri, 9 Apr 2021 17:39:02 +0200 Subject: [PATCH 01/18] implementing phpredis --- src/Configuration/HostNormalizer.php | 2 +- src/Connections/PhpRedisConnection.php | 109 ++++++++++++++++ src/Connectors/PhpRedisConnector.php | 118 ++++++++++++++++++ src/Manager/VersionedRedisSentinelManager.php | 2 + 4 files changed, 230 insertions(+), 1 deletion(-) create mode 100644 src/Connections/PhpRedisConnection.php create mode 100644 src/Connectors/PhpRedisConnector.php diff --git a/src/Configuration/HostNormalizer.php b/src/Configuration/HostNormalizer.php index 4a012ae..c4b1f89 100644 --- a/src/Configuration/HostNormalizer.php +++ b/src/Configuration/HostNormalizer.php @@ -62,7 +62,7 @@ class HostNormalizer public static function normalizeConnections(array $connections) { foreach ($connections as $name => $connection) { - if ($name === 'options' || $name === 'clusters') { + if ($name === 'client' || $name === 'options' || $name === 'clusters') { continue; } diff --git a/src/Connections/PhpRedisConnection.php b/src/Connections/PhpRedisConnection.php new file mode 100644 index 0000000..350def5 --- /dev/null +++ b/src/Connections/PhpRedisConnection.php @@ -0,0 +1,109 @@ + + * @license See LICENSE file + * @link https://github.com/monospice/laravel-redis-sentinel-drivers + */ +class PhpRedisConnection extends LaravelPhpRedisConnection +{ + /** + * The number of times the client attempts to retry a command when it fails + * to connect to a Redis instance behind Sentinel. + * + * @var int + */ + protected $retryLimit = 20; + + /** + * The time in milliseconds to wait before the client retries a failed + * command. + * + * @var int + */ + protected $retryWait = 1000; + + /** + * Create a new PhpRedis connection. + * + * @param \Redis $client + * @param callable|null $connector + * @param array $sentinelOptions + * @return void + */ + public function __construct($client, callable $connector = null, array $sentinelOptions = []) + { + parent::__construct($client, $connector); + + $this->retryLimit = (int) ($sentinelOptions['retry_limit'] ?? 20); + $this->retryWait = (int) ($sentinelOptions['retry_wait'] ?? 1000); + } + + /** + * Execute commands in a transaction. + * + * @param callable|null $callback + * @return \Redis|array + */ + public function transaction(callable $callback = null) + { + return $this->retryOnFailure(function () use ($callback) { + $transaction = $this->client()->multi(); + + return is_null($callback) + ? $transaction + : tap($transaction, $callback)->exec(); + }); + } + /** + * Attempt to retry the provided operation when the client fails to connect + * to a Redis server. + * + * We adapt Predis' Sentinel connection failure handling logic here to + * reproduce the high-availability mode provided by the actual client. To + * work around "aggregate" connection limitations in Predis, this class + * provides methods that don't use the high-level Sentinel connection API + * of Predis directly, so it needs to handle connection failures itself. + * + * @param callable $callback The operation to execute. + * + * @return mixed The result of the first successful attempt. + * + * @throws RedisException After exhausting the allowed number of + * attempts to reconnect. + */ + protected function retryOnFailure(callable $callback) + { + $attempts = 0; + + do { + try { + return $callback(); + } catch (RedisException $exception) { + $this->client->close(); + + usleep($this->retryWait * 1000); + + $this->client = $this->connector(); + + $attempts++; + } + } while ($attempts <= $this->retryLimit); + + throw $exception; + } +} diff --git a/src/Connectors/PhpRedisConnector.php b/src/Connectors/PhpRedisConnector.php new file mode 100644 index 0000000..295128c --- /dev/null +++ b/src/Connectors/PhpRedisConnector.php @@ -0,0 +1,118 @@ + + * @license See LICENSE file + * @link http://github.com/monospice/laravel-redis-sentinel-drivers + */ +class PhpRedisConnector extends LaravelPhpRedisConnector +{ + /** + * Holds the current sentinel servers. + * + * @var array + */ + protected $servers = []; + + /** + * Configuration options specific to Sentinel connection operation + * + * We cannot pass these options as an array to the Predis client. + * Instead, we'll set them on the connection directly using methods + * provided by the SentinelReplication class of the Predis package. + * + * @var array + */ + protected $sentinelKeys = [ + 'sentinel_timeout' => null, + 'retry_wait' => null, + 'retry_limit' => null, + 'update_sentinels' => null, + + 'sentinel_persistent' => null, + 'sentinel_read_timeout' => null, + ]; + + /** + * Create a new Redis Sentinel connection from the provided configuration + * options + * + * @param array $server The client configuration for the connection + * @param array $options The global client options shared by all Sentinel + * connections + * + * @return PredisConnection The Sentinel connection containing a configured + * Predis Client + */ + public function connect(array $servers, array $options = [ ]) + { + // Merge the global options shared by all Sentinel connections with + // connection-specific options + $clientOpts = array_merge($options, Arr::pull($servers, 'options', [ ])); + + // Extract the array of Sentinel connection options from the rest of + // the client options + $sentinelOpts = array_intersect_key($clientOpts, $this->sentinelKeys); + + // Filter the Sentinel connection options elements from the client + // options array + $clientOpts = array_diff_key($clientOpts, $this->sentinelKeys); + + // Create a client by calling the Sentinel servers + $connector = function () use ($servers, $options) { + return $this->createClientWithSentinel($servers, $options); + }; + + return new PhpRedisConnection($connector(), $connector, $sentinelOpts); + } + + /** + * Create the Redis client instance. + * + * @param array $servers + * @param array $options + * @return \Redis + */ + protected function createClientWithSentinel(array $servers, array $options) + { + shuffle($servers); + + foreach ($servers as $server) { + $sentinel = new RedisSentinel( + $server['host'] ?? 'localhost', + $server['port'] ?? 26739, + $options['sentinel_timeout'] ?? 0, + $options['sentinel_persistent'] ?? null, + $options['retry_wait'] ?? 0, + $options['sentinel_read_timeout'] ?? 0, + ); + + // @TODO update_sentinels + // $this->servers = $sentinel->sentinels($options['service'] ?? 'mymaster'); + // var_dump($sentinel->sentinels($options['service'] ?? 'mymaster')); + // var_dump($sentinel->masters()); + + $master = $sentinel->getMasterAddrByName($options['service'] ?? 'mymaster'); + if ($master !== false) { + $config['host'] = $master[0]; + $config['port'] = $master[1]; + + var_dump($config); + + return $this->createClient($config); + } + } + } +} diff --git a/src/Manager/VersionedRedisSentinelManager.php b/src/Manager/VersionedRedisSentinelManager.php index 241a5bb..00453a6 100644 --- a/src/Manager/VersionedRedisSentinelManager.php +++ b/src/Manager/VersionedRedisSentinelManager.php @@ -70,6 +70,8 @@ protected function connector() switch ($this->driver) { case 'predis': return new Connectors\PredisConnector(); + case 'phpredis': + return new Connectors\PhpRedisConnector(); } throw new InvalidArgumentException( From 9c0093ddf376f4a3b4612a0a295d0c9a0c0d0ab5 Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Fri, 9 Apr 2021 18:12:52 +0200 Subject: [PATCH 02/18] implementing update_sentinels --- src/Connectors/PhpRedisConnector.php | 68 +++++++++++++++++++--------- 1 file changed, 47 insertions(+), 21 deletions(-) diff --git a/src/Connectors/PhpRedisConnector.php b/src/Connectors/PhpRedisConnector.php index 295128c..fa9f687 100644 --- a/src/Connectors/PhpRedisConnector.php +++ b/src/Connectors/PhpRedisConnector.php @@ -7,6 +7,7 @@ use Illuminate\Redis\Connectors\PhpRedisConnector as LaravelPhpRedisConnector; use Monospice\LaravelRedisSentinel\Connections\PhpRedisConnection; use RedisSentinel; +use RedisException; /** * Initializes PhpRedis Client instances for Redis Sentinel connections @@ -24,7 +25,7 @@ class PhpRedisConnector extends LaravelPhpRedisConnector * * @var array */ - protected $servers = []; + protected $servers; /** * Configuration options specific to Sentinel connection operation @@ -58,6 +59,9 @@ class PhpRedisConnector extends LaravelPhpRedisConnector */ public function connect(array $servers, array $options = [ ]) { + // Set the initial Sentinel servers. + $this->servers = $servers; + // Merge the global options shared by all Sentinel connections with // connection-specific options $clientOpts = array_merge($options, Arr::pull($servers, 'options', [ ])); @@ -71,47 +75,69 @@ public function connect(array $servers, array $options = [ ]) $clientOpts = array_diff_key($clientOpts, $this->sentinelKeys); // Create a client by calling the Sentinel servers - $connector = function () use ($servers, $options) { - return $this->createClientWithSentinel($servers, $options); + $connector = function () use ($options) { + return $this->createClientWithSentinel($options); }; return new PhpRedisConnection($connector(), $connector, $sentinelOpts); } /** - * Create the Redis client instance. + * Create the Redis client instance * * @param array $servers * @param array $options * @return \Redis */ - protected function createClientWithSentinel(array $servers, array $options) + protected function createClientWithSentinel(array $options) { + $servers = $this->servers; + shuffle($servers); - foreach ($servers as $server) { + foreach ($servers as $idx => $server) { + $host = $server['host'] ?? 'localhost'; + $port = $server['port'] ?? 26739; + $service = $options['service'] ?? 'mymaster'; + $sentinel = new RedisSentinel( - $server['host'] ?? 'localhost', - $server['port'] ?? 26739, + $host, + $port, $options['sentinel_timeout'] ?? 0, $options['sentinel_persistent'] ?? null, $options['retry_wait'] ?? 0, $options['sentinel_read_timeout'] ?? 0, ); - // @TODO update_sentinels - // $this->servers = $sentinel->sentinels($options['service'] ?? 'mymaster'); - // var_dump($sentinel->sentinels($options['service'] ?? 'mymaster')); - // var_dump($sentinel->masters()); - - $master = $sentinel->getMasterAddrByName($options['service'] ?? 'mymaster'); - if ($master !== false) { - $config['host'] = $master[0]; - $config['port'] = $master[1]; - - var_dump($config); - - return $this->createClient($config); + try { + if (($options['update_sentinels'] ?? false) === true) { + $this->servers = array_merge( + [ + [ + 'host' => $host, + 'port' => $port, + ] + ], array_map(fn ($sentinel) => [ + 'host' => $sentinel['ip'], + 'port' => $sentinel['port'], + ], $sentinel->sentinels($service)) + ); + } + + $master = $sentinel->getMasterAddrByName($service); + if (is_array($master) && count($master)) { + $config['host'] = $master[0]; + $config['port'] = $master[1]; + + // @TODO rewrite this config for auth. + + return $this->createClient($config); + } + } catch (RedisException $e) { + // Only throw the exception if the last server can't connect + if ($idx === count($servers) - 1) { + throw $e; + } } } } From 56662b2ba98ae46044b7c43129fbe0ede758b095 Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Sun, 11 Apr 2021 13:57:07 +0200 Subject: [PATCH 03/18] wrap all phpredis methods with retryOnFailure --- src/Connections/PhpRedisConnection.php | 143 ++++++++++++++++++++++--- src/Connectors/PhpRedisConnector.php | 36 +++---- 2 files changed, 144 insertions(+), 35 deletions(-) diff --git a/src/Connections/PhpRedisConnection.php b/src/Connections/PhpRedisConnection.php index 350def5..b95b61c 100644 --- a/src/Connections/PhpRedisConnection.php +++ b/src/Connections/PhpRedisConnection.php @@ -2,6 +2,7 @@ namespace Monospice\LaravelRedisSentinel\Connections; +use Closure; use Illuminate\Redis\Connections\PhpRedisConnection as LaravelPhpRedisConnection; use Redis; use RedisException; @@ -9,13 +10,12 @@ /** * Executes Redis commands using the PhpRedis client. * - * This package extends Laravel's PhpRedisConnection class to work around issues - * experienced when using the PhpRedis client to send commands over "aggregate" - * connections (in this case, Sentinel connections). + * This package extends Laravel's PhpRedisConnection class to wrap all command + * methods with a retryOnFailure method. * * @category Package * @package Monospice\LaravelRedisSentinel - * @author @pdbreen, Cy Rossignol + * @author Jeffrey Zant * @license See LICENSE file * @link https://github.com/monospice/laravel-redis-sentinel-drivers */ @@ -54,7 +54,79 @@ public function __construct($client, callable $connector = null, array $sentinel } /** - * Execute commands in a transaction. + * {@inheritdoc} in addition retry on client failure. + * + * @param mixed $cursor + * @param array $options + * @return mixed + */ + public function scan($cursor, $options = []) + { + return $this->retryOnFailure(function () use ($cursor, $options) { + return parent::scan($cursor, $options); + }); + } + + /** + * {@inheritdoc} in addition retry on client failure. + * + * @param string $key + * @param mixed $cursor + * @param array $options + * @return mixed + */ + public function zscan($key, $cursor, $options = []) + { + return $this->retryOnFailure(function () use ($key, $cursor, $options) { + parent::zscan($key, $cursor, $options); + }); + } + + /** + * {@inheritdoc} in addition retry on client failure. + * + * @param string $key + * @param mixed $cursor + * @param array $options + * @return mixed + */ + public function hscan($key, $cursor, $options = []) + { + return $this->retryOnFailure(function () use ($key, $cursor, $options) { + parent::hscan($key, $cursor, $options); + }); + } + + /** + * {@inheritdoc} in addition retry on client failure. + * + * @param string $key + * @param mixed $cursor + * @param array $options + * @return mixed + */ + public function sscan($key, $cursor, $options = []) + { + return $this->retryOnFailure(function () use ($key, $cursor, $options) { + parent::sscan($key, $cursor, $options); + }); + } + + /** + * {@inheritdoc} in addition retry on client failure. + * + * @param callable|null $callback + * @return \Redis|array + */ + public function pipeline(callable $callback = null) + { + return $this->retryOnFailure(function () use ($callback) { + return parent::pipeline($callback); + }); + } + + /** + * {@inheritdoc} in addition retry on client failure. * * @param callable|null $callback * @return \Redis|array @@ -62,25 +134,57 @@ public function __construct($client, callable $connector = null, array $sentinel public function transaction(callable $callback = null) { return $this->retryOnFailure(function () use ($callback) { - $transaction = $this->client()->multi(); + return parent::transaction($callback); + }); + } - return is_null($callback) - ? $transaction - : tap($transaction, $callback)->exec(); + /** + * {@inheritdoc} in addition retry on client failure. + * + * @param array|string $channels + * @param \Closure $callback + * @return void + */ + public function subscribe($channels, Closure $callback) + { + return $this->retryOnFailure(function () use ($channels, $callback) { + return parent::subscribe($channels, $callback); }); } + + /** + * {@inheritdoc} in addition retry on client failure. + * + * @param array|string $channels + * @param \Closure $callback + * @return void + */ + public function psubscribe($channels, Closure $callback) + { + return $this->retryOnFailure(function () use ($channels, $callback) { + return parent::psubscribe($channels, $callback); + }); + } + + /** + * {@inheritdoc} in addition retry on client failure. + * + * @param string $method + * @param array $parameters + * @return mixed + */ + public function command($method, array $parameters = []) + { + return $this->retryOnFailure(function () use ($method, $parameters) { + return parent::command($method, $parameters); + }); + } + /** * Attempt to retry the provided operation when the client fails to connect * to a Redis server. * - * We adapt Predis' Sentinel connection failure handling logic here to - * reproduce the high-availability mode provided by the actual client. To - * work around "aggregate" connection limitations in Predis, this class - * provides methods that don't use the high-level Sentinel connection API - * of Predis directly, so it needs to handle connection failures itself. - * * @param callable $callback The operation to execute. - * * @return mixed The result of the first successful attempt. * * @throws RedisException After exhausting the allowed number of @@ -98,7 +202,12 @@ protected function retryOnFailure(callable $callback) usleep($this->retryWait * 1000); - $this->client = $this->connector(); + try { + $this->client = $this->connector(); + } catch (RedisException $e) { + // Ignore the the creation of a new client gets an exception. + // If this exception isn't caught the retry will stop. + } $attempts++; } diff --git a/src/Connectors/PhpRedisConnector.php b/src/Connectors/PhpRedisConnector.php index fa9f687..cde93cf 100644 --- a/src/Connectors/PhpRedisConnector.php +++ b/src/Connectors/PhpRedisConnector.php @@ -3,7 +3,6 @@ namespace Monospice\LaravelRedisSentinel\Connectors; use Illuminate\Support\Arr; -use Monospice\LaravelRedisSentinel\Connections\PredisConnection; use Illuminate\Redis\Connectors\PhpRedisConnector as LaravelPhpRedisConnector; use Monospice\LaravelRedisSentinel\Connections\PhpRedisConnection; use RedisSentinel; @@ -14,7 +13,7 @@ * * @category Package * @package Monospice\LaravelRedisSentinel - * @author Cy Rossignol + * @author Jeffrey Zant * @license See LICENSE file * @link http://github.com/monospice/laravel-redis-sentinel-drivers */ @@ -30,9 +29,10 @@ class PhpRedisConnector extends LaravelPhpRedisConnector /** * Configuration options specific to Sentinel connection operation * - * We cannot pass these options as an array to the Predis client. + * @TODO rewrite doc. + * We cannot pass these options as an array to the PhpRedis client. * Instead, we'll set them on the connection directly using methods - * provided by the SentinelReplication class of the Predis package. + * provided by the SentinelReplication class of the PhpRedis package. * * @var array */ @@ -54,8 +54,8 @@ class PhpRedisConnector extends LaravelPhpRedisConnector * @param array $options The global client options shared by all Sentinel * connections * - * @return PredisConnection The Sentinel connection containing a configured - * Predis Client + * @return PhpRedisConnection The Sentinel connection containing a configured + * PhpRedis Client */ public function connect(array $servers, array $options = [ ]) { @@ -95,7 +95,7 @@ protected function createClientWithSentinel(array $options) shuffle($servers); - foreach ($servers as $idx => $server) { + foreach ($servers as $server) { $host = $server['host'] ?? 'localhost'; $port = $server['port'] ?? 26739; $service = $options['service'] ?? 'mymaster'; @@ -125,20 +125,20 @@ protected function createClientWithSentinel(array $options) } $master = $sentinel->getMasterAddrByName($service); - if (is_array($master) && count($master)) { - $config['host'] = $master[0]; - $config['port'] = $master[1]; - - // @TODO rewrite this config for auth. - - return $this->createClient($config); + if (! is_array($master) || ! count($master)) { + throw new RedisException(sprintf('No master found for service "%s".', $service)); } + + return $this->createClient(array_merge( + $options['parameters'] ?? [], + $server, + ['host' => $master[0], 'port' => $master[1]] + )); } catch (RedisException $e) { - // Only throw the exception if the last server can't connect - if ($idx === count($servers) - 1) { - throw $e; - } + // } } + + throw new RedisException('Could not create a client for the configured Sentinel servers.'); } } From ef688e177943e79cfad03780c2d048808ff0f2cf Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Mon, 12 Apr 2021 10:33:19 +0200 Subject: [PATCH 04/18] removing null coalescing operators --- src/Connections/PhpRedisConnection.php | 4 ++-- src/Connectors/PhpRedisConnector.php | 26 +++++++++++++++++--------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/Connections/PhpRedisConnection.php b/src/Connections/PhpRedisConnection.php index b95b61c..ac2bce8 100644 --- a/src/Connections/PhpRedisConnection.php +++ b/src/Connections/PhpRedisConnection.php @@ -49,8 +49,8 @@ public function __construct($client, callable $connector = null, array $sentinel { parent::__construct($client, $connector); - $this->retryLimit = (int) ($sentinelOptions['retry_limit'] ?? 20); - $this->retryWait = (int) ($sentinelOptions['retry_wait'] ?? 1000); + $this->retryLimit = (int) (isset($sentinelOptions['retry_limit']) ? $sentinelOptions['retry_limit'] : 20); + $this->retryWait = (int) (isset($sentinelOptions['retry_wait']) ? $sentinelOptions['retry_wait'] : 1000); } /** diff --git a/src/Connectors/PhpRedisConnector.php b/src/Connectors/PhpRedisConnector.php index cde93cf..fd4b74f 100644 --- a/src/Connectors/PhpRedisConnector.php +++ b/src/Connectors/PhpRedisConnector.php @@ -93,24 +93,30 @@ protected function createClientWithSentinel(array $options) { $servers = $this->servers; + // Shuffle the servers to perform some loadbalancing. shuffle($servers); + // Try to connect to any of the servers. foreach ($servers as $server) { - $host = $server['host'] ?? 'localhost'; - $port = $server['port'] ?? 26739; - $service = $options['service'] ?? 'mymaster'; + $host = isset($server['host']) ? $server['host'] : 'localhost'; + $port = isset($server['port']) ? $server['port'] : 26739; + $service = isset($options['service']) ? $options['service'] : 'mymaster'; + // Create a connection to the Sentinel instance. $sentinel = new RedisSentinel( $host, $port, - $options['sentinel_timeout'] ?? 0, - $options['sentinel_persistent'] ?? null, - $options['retry_wait'] ?? 0, - $options['sentinel_read_timeout'] ?? 0, + isset($options['sentinel_timeout']) ? $options['sentinel_timeout'] : 0, + isset($options['sentinel_persistent']) ? $options['sentinel_persistent'] : null, + isset($options['retry_wait']) ? $options['retry_wait'] : 0, + isset($options['sentinel_read_timeout']) ? $options['sentinel_read_timeout'] : 0, ); try { - if (($options['update_sentinels'] ?? false) === true) { + // Check if the Sentinel server list needs to be updated. + // Put the current server and the other sentinels in the server list. + $updateSentinels = isset($options['update_sentinels']) ? $options['update_sentinels'] : false; + if ($updateSentinels === true) { $this->servers = array_merge( [ [ @@ -124,13 +130,15 @@ protected function createClientWithSentinel(array $options) ); } + // Lookup the master node. $master = $sentinel->getMasterAddrByName($service); if (! is_array($master) || ! count($master)) { throw new RedisException(sprintf('No master found for service "%s".', $service)); } + // Create a PhpRedis client for the selected master node. return $this->createClient(array_merge( - $options['parameters'] ?? [], + isset($options['parameters']) ? $options['parameters'] : [], $server, ['host' => $master[0], 'port' => $master[1]] )); From 81e95f953808ea5960547c06ea844fae7f1abcb6 Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Mon, 12 Apr 2021 12:33:41 +0200 Subject: [PATCH 05/18] phpredis integration tests --- src/Connections/PhpRedisConnection.php | 7 +- src/Connectors/PhpRedisConnector.php | 46 ++++-- src/Exceptions/RetryRedisException.php | 10 ++ .../Connections/PhpRedisConnectionTest.php | 153 ++++++++++++++++++ 4 files changed, 200 insertions(+), 16 deletions(-) create mode 100644 src/Exceptions/RetryRedisException.php create mode 100644 tests/Integration/Connections/PhpRedisConnectionTest.php diff --git a/src/Connections/PhpRedisConnection.php b/src/Connections/PhpRedisConnection.php index ac2bce8..4e8dbdf 100644 --- a/src/Connections/PhpRedisConnection.php +++ b/src/Connections/PhpRedisConnection.php @@ -2,6 +2,7 @@ namespace Monospice\LaravelRedisSentinel\Connections; +use Monospice\LaravelRedisSentinel\Exceptions\RetryRedisException; use Closure; use Illuminate\Redis\Connections\PhpRedisConnection as LaravelPhpRedisConnection; use Redis; @@ -203,7 +204,7 @@ protected function retryOnFailure(callable $callback) usleep($this->retryWait * 1000); try { - $this->client = $this->connector(); + $this->client = $this->connector ? call_user_func($this->connector) : $this->client; } catch (RedisException $e) { // Ignore the the creation of a new client gets an exception. // If this exception isn't caught the retry will stop. @@ -211,8 +212,8 @@ protected function retryOnFailure(callable $callback) $attempts++; } - } while ($attempts <= $this->retryLimit); + } while ($attempts < $this->retryLimit); - throw $exception; + throw new RetryRedisException(sprintf('Reached the reconnect limit of %d attempts', $attempts)); } } diff --git a/src/Connectors/PhpRedisConnector.php b/src/Connectors/PhpRedisConnector.php index fd4b74f..c55e9b2 100644 --- a/src/Connectors/PhpRedisConnector.php +++ b/src/Connectors/PhpRedisConnector.php @@ -29,10 +29,8 @@ class PhpRedisConnector extends LaravelPhpRedisConnector /** * Configuration options specific to Sentinel connection operation * - * @TODO rewrite doc. - * We cannot pass these options as an array to the PhpRedis client. - * Instead, we'll set them on the connection directly using methods - * provided by the SentinelReplication class of the PhpRedis package. + * Some of the Sentinel configuration options can be entered in this class. + * The retry_wait and retry_limit values are passed to the connection. * * @var array */ @@ -60,7 +58,9 @@ class PhpRedisConnector extends LaravelPhpRedisConnector public function connect(array $servers, array $options = [ ]) { // Set the initial Sentinel servers. - $this->servers = $servers; + $this->servers = array_map(function ($server) { + return $this->formatServer($server); + }, $servers); // Merge the global options shared by all Sentinel connections with // connection-specific options @@ -92,6 +92,10 @@ public function connect(array $servers, array $options = [ ]) protected function createClientWithSentinel(array $options) { $servers = $this->servers; + $timeout = isset($options['sentinel_timeout']) ? $options['sentinel_timeout'] : 0; + $persistent = isset($options['sentinel_peristent']) ? $options['sentinel_peristent'] : null; + $retryWait = isset($options['retry_wait']) ? $options['retry_wait'] : 0; + $readTimeout = isset($options['sentinel_read_timeout']) ? $options['sentinel_read_timeout'] : 0; // Shuffle the servers to perform some loadbalancing. shuffle($servers); @@ -103,14 +107,7 @@ protected function createClientWithSentinel(array $options) $service = isset($options['service']) ? $options['service'] : 'mymaster'; // Create a connection to the Sentinel instance. - $sentinel = new RedisSentinel( - $host, - $port, - isset($options['sentinel_timeout']) ? $options['sentinel_timeout'] : 0, - isset($options['sentinel_persistent']) ? $options['sentinel_persistent'] : null, - isset($options['retry_wait']) ? $options['retry_wait'] : 0, - isset($options['sentinel_read_timeout']) ? $options['sentinel_read_timeout'] : 0, - ); + $sentinel = new RedisSentinel($host, $port, $timeout, $persistent, $retryWait, $readTimeout); try { // Check if the Sentinel server list needs to be updated. @@ -149,4 +146,27 @@ protected function createClientWithSentinel(array $options) throw new RedisException('Could not create a client for the configured Sentinel servers.'); } + + /** + * Format a server. + * + * @param array $server + * @return array + * + * @throws RedisException + */ + private function formatServer($server) + { + if (is_string($server)) { + list($host, $port) = explode(':', $server); + + return ['host' => $host, 'port' => $port]; + } + + if (! is_array($server)) { + throw new RedisException('Could not format the server definition.'); + } + + return $server; + } } diff --git a/src/Exceptions/RetryRedisException.php b/src/Exceptions/RetryRedisException.php new file mode 100644 index 0000000..d32736f --- /dev/null +++ b/src/Exceptions/RetryRedisException.php @@ -0,0 +1,10 @@ +subject = $this->makeClient(); + } + + /** + * Run this cleanup after each test. + * + * @return void + */ + public function tearDown() + { + parent::tearDown(); + + Mockery::close(); + } + + public function testAllowsTransactionsOnAggregateConnection() + { + $transaction = $this->subject->transaction(); + + $this->assertInstanceOf(Redis::class, $transaction); + } + + public function testExecutesCommandsInTransaction() + { + $result = $this->subject->transaction(function ($trans) { + $trans->set('test-key', 'test value'); + $trans->get('test-key'); + }); + + $this->assertCount(2, $result); + $this->assertTrue($result[0]); + $this->assertEquals('test value', $result[1]); + $this->assertRedisKeyEquals('test-key', 'test value'); + } + + public function testExecutesTransactionsOnMaster() + { + $expectedSubset = ['role' => 'master']; + + $info = $this->subject->transaction(function ($transaction) { + $transaction->info(); + }); + + $this->assertArraySubset($expectedSubset, $info[0]); + } + + public function testAbortsTransactionOnException() + { + $exception = null; + + try { + $this->subject->transaction(function ($trans) { + $trans->set('test-key', 'test value'); + throw new DummyException(); + }); + } catch (DummyException $exception) { + // With PHPUnit, we need to wrap the throwing block to perform + // assertions afterward. + } + + $this->assertNotNull($exception); + $this->assertRedisKeyEquals('test-key', null); + } + + public function testRetriesTransactionWhenConnectionFails() + { + $this->expectException(RetryRedisException::class); + + $expectedRetries = 2; + + $this->subject = $this->makeClient($expectedRetries, 0); // retry immediately + + $this->subject->transaction(function () { + throw new RedisException(); + }); + } + + public function testCanReconnectWhenConnectionFails() + { + $retries = 3; + $attempts = 0; + + $this->subject = $this->makeClient($retries, 0); // retry immediately + + $this->subject->transaction(function ($trans) use (&$attempts, $retries) { + $attempts++; + + if ($attempts < $retries) { + throw new RedisException(); + } else { + $trans->set('test-key', 'test value'); + } + }); + + $this->assertGreaterThan(1, $attempts, 'First try does not count.'); + $this->assertRedisKeyEquals('test-key', 'test value'); + } + + /** + * Initialize a PhpRedis client using the test connection configuration + * that can verify connectivity failure handling. + * + * @param int|null $retryLimit + * @param int|null $retryWait + * @return Redis A client instance for the subject under test. + */ + protected function makeClient(int $retryLimit = null, int $retryWait = null) + { + $connector = new PhpRedisConnector(); + + $options = $this->config['options']; + if ($retryLimit !== null) { + $options['retry_limit'] = $retryLimit; + } + + if ($retryWait !== null) { + $options['retry_wait'] = $retryWait; + } + + return $connector->connect($this->config['default'], $options); + } +} From dec46c3f6f7e0ffa9e63ffcba51013bf73ce587e Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Mon, 12 Apr 2021 13:05:32 +0200 Subject: [PATCH 06/18] improving readability and integration tests --- src/Connectors/PhpRedisConnector.php | 65 ++++++++++++------- .../Connections/PhpRedisConnectionTest.php | 10 +-- tests/Unit/Configuration/LoaderTest.php | 4 +- .../Unit/Connections/PredisConnectionTest.php | 2 +- .../VersionedRedisSentinelManagerTest.php | 9 ++- .../Unit/RedisSentinelServiceProviderTest.php | 3 +- 6 files changed, 55 insertions(+), 38 deletions(-) diff --git a/src/Connectors/PhpRedisConnector.php b/src/Connectors/PhpRedisConnector.php index c55e9b2..33b4ab5 100644 --- a/src/Connectors/PhpRedisConnector.php +++ b/src/Connectors/PhpRedisConnector.php @@ -5,6 +5,7 @@ use Illuminate\Support\Arr; use Illuminate\Redis\Connectors\PhpRedisConnector as LaravelPhpRedisConnector; use Monospice\LaravelRedisSentinel\Connections\PhpRedisConnection; +use Redis; use RedisSentinel; use RedisException; @@ -85,9 +86,8 @@ public function connect(array $servers, array $options = [ ]) /** * Create the Redis client instance * - * @param array $servers * @param array $options - * @return \Redis + * @return Redis */ protected function createClientWithSentinel(array $options) { @@ -96,12 +96,13 @@ protected function createClientWithSentinel(array $options) $persistent = isset($options['sentinel_peristent']) ? $options['sentinel_peristent'] : null; $retryWait = isset($options['retry_wait']) ? $options['retry_wait'] : 0; $readTimeout = isset($options['sentinel_read_timeout']) ? $options['sentinel_read_timeout'] : 0; + $parameters = isset($options['parameters']) ? $options['parameters'] : []; // Shuffle the servers to perform some loadbalancing. shuffle($servers); // Try to connect to any of the servers. - foreach ($servers as $server) { + foreach ($servers as $idx => $server) { $host = isset($server['host']) ? $server['host'] : 'localhost'; $port = isset($server['port']) ? $server['port'] : 26739; $service = isset($options['service']) ? $options['service'] : 'mymaster'; @@ -114,43 +115,56 @@ protected function createClientWithSentinel(array $options) // Put the current server and the other sentinels in the server list. $updateSentinels = isset($options['update_sentinels']) ? $options['update_sentinels'] : false; if ($updateSentinels === true) { - $this->servers = array_merge( - [ - [ - 'host' => $host, - 'port' => $port, - ] - ], array_map(fn ($sentinel) => [ - 'host' => $sentinel['ip'], - 'port' => $sentinel['port'], - ], $sentinel->sentinels($service)) - ); + $this->updateSentinels($sentinel, $host, $port, $service); } // Lookup the master node. $master = $sentinel->getMasterAddrByName($service); - if (! is_array($master) || ! count($master)) { + if (is_array($master) && ! count($master)) { throw new RedisException(sprintf('No master found for service "%s".', $service)); } // Create a PhpRedis client for the selected master node. - return $this->createClient(array_merge( - isset($options['parameters']) ? $options['parameters'] : [], - $server, - ['host' => $master[0], 'port' => $master[1]] - )); + return $this->createClient( + array_merge($parameters, $server, ['host' => $master[0], 'port' => $master[1]]) + ); } catch (RedisException $e) { - // + // Rethrow the expection when the last server is reached. + if ($idx === count($servers) - 1) { + throw $e; + } } } + } - throw new RedisException('Could not create a client for the configured Sentinel servers.'); + /** + * Update the list With sentinel servers. + * + * @param RedisSentinel $sentinel + * @param string $currentHost + * @param int $currentPort + * @param string $service + * @return void + */ + private function updateSentinels(RedisSentinel $sentinel, string $currentHost, int $currentPort, string $service) + { + $this->servers = array_merge( + [ + [ + 'host' => $currentHost, + 'port' => $currentPort, + ] + ], array_map(fn ($sentinel) => [ + 'host' => $sentinel['ip'], + 'port' => $sentinel['port'], + ], $sentinel->sentinels($service)) + ); } /** * Format a server. * - * @param array $server + * @param mixed $server * @return array * * @throws RedisException @@ -159,8 +173,11 @@ private function formatServer($server) { if (is_string($server)) { list($host, $port) = explode(':', $server); + if (! $host || ! $port) { + throw new RedisException('Could not format the server definition.'); + } - return ['host' => $host, 'port' => $port]; + return ['host' => $host, 'port' => (int) $port]; } if (! is_array($server)) { diff --git a/tests/Integration/Connections/PhpRedisConnectionTest.php b/tests/Integration/Connections/PhpRedisConnectionTest.php index 1c93a74..dc915f4 100644 --- a/tests/Integration/Connections/PhpRedisConnectionTest.php +++ b/tests/Integration/Connections/PhpRedisConnectionTest.php @@ -29,7 +29,7 @@ public function setUp() { parent::setUp(); - $this->subject = $this->makeClient(); + $this->subject = $this->makeConnection(); } /** @@ -99,7 +99,7 @@ public function testRetriesTransactionWhenConnectionFails() $expectedRetries = 2; - $this->subject = $this->makeClient($expectedRetries, 0); // retry immediately + $this->subject = $this->makeConnection($expectedRetries, 0); // retry immediately $this->subject->transaction(function () { throw new RedisException(); @@ -111,7 +111,7 @@ public function testCanReconnectWhenConnectionFails() $retries = 3; $attempts = 0; - $this->subject = $this->makeClient($retries, 0); // retry immediately + $this->subject = $this->makeConnection($retries, 0); // retry immediately $this->subject->transaction(function ($trans) use (&$attempts, $retries) { $attempts++; @@ -133,9 +133,9 @@ public function testCanReconnectWhenConnectionFails() * * @param int|null $retryLimit * @param int|null $retryWait - * @return Redis A client instance for the subject under test. + * @return PhpRedisConnection A client instance for the subject under test. */ - protected function makeClient(int $retryLimit = null, int $retryWait = null) + protected function makeConnection(int $retryLimit = null, int $retryWait = null) { $connector = new PhpRedisConnector(); diff --git a/tests/Unit/Configuration/LoaderTest.php b/tests/Unit/Configuration/LoaderTest.php index 5fc8a37..96ed1d9 100644 --- a/tests/Unit/Configuration/LoaderTest.php +++ b/tests/Unit/Configuration/LoaderTest.php @@ -499,9 +499,9 @@ public function testSkipsLoadingHorizonConfigurationIfCached() public function testDisallowsUsingNonexistantConnectionForHorizon() { - $this->config->set('horizon.use', 'not-a-connection'); + $this->expectException(UnexpectedValueException::class); - $this->setExpectedException(UnexpectedValueException::class); + $this->config->set('horizon.use', 'not-a-connection'); $this->loader->loadHorizonConfiguration(); } diff --git a/tests/Unit/Connections/PredisConnectionTest.php b/tests/Unit/Connections/PredisConnectionTest.php index a5e384e..c1c6e30 100644 --- a/tests/Unit/Connections/PredisConnectionTest.php +++ b/tests/Unit/Connections/PredisConnectionTest.php @@ -102,7 +102,7 @@ public function testSetsSentinelConnectionOptionsFromConfig() public function testDisallowsInvalidSentinelOptions() { - $this->setExpectedException(BadMethodCallException::class); + $this->expectException(BadMethodCallException::class); new PredisConnection($this->clientMock, [ 'not_an_option' => null ]); } diff --git a/tests/Unit/Manager/VersionedRedisSentinelManagerTest.php b/tests/Unit/Manager/VersionedRedisSentinelManagerTest.php index d6764b3..0a05005 100644 --- a/tests/Unit/Manager/VersionedRedisSentinelManagerTest.php +++ b/tests/Unit/Manager/VersionedRedisSentinelManagerTest.php @@ -2,7 +2,6 @@ namespace Monospice\LaravelRedisSentinel\Tests\Unit; -use Closure; use Illuminate\Contracts\Container\Container; use Illuminate\Contracts\Redis\Factory as RedisFactory; use Illuminate\Redis\RedisManager; @@ -118,23 +117,23 @@ public function testCreatesSingleClientsWithIndividualConfig() public function testDisallowsRedisClusterConnections() { - $this->setExpectedException(InvalidArgumentException::class); + $this->expectException(InvalidArgumentException::class); $this->subject->connection('clustered_connection'); } public function testFailsOnUndefinedConnection() { - $this->setExpectedException(InvalidArgumentException::class); + $this->expectException(InvalidArgumentException::class); $this->subject->connection('nonexistant_connection'); } public function testFailsOnUnsupportedClientDriver() { - $this->setExpectedException(InvalidArgumentException::class); + $this->expectException(InvalidArgumentException::class); - $manager = $this->makeSubject('phpredis', [ + $manager = $this->makeSubject('fakeredis', [ 'test_connection' => [ ], ]); diff --git a/tests/Unit/RedisSentinelServiceProviderTest.php b/tests/Unit/RedisSentinelServiceProviderTest.php index 1833453..e2c1fdc 100644 --- a/tests/Unit/RedisSentinelServiceProviderTest.php +++ b/tests/Unit/RedisSentinelServiceProviderTest.php @@ -200,10 +200,11 @@ public function testBootExtendsSessionHandlers() public function testWaitsForBoot() { + $this->expectException(\InvalidArgumentException::class); + $this->app->config->set('redis-sentinel.auto_boot', false); $this->provider->register(); - $this->setExpectedException(\InvalidArgumentException::class); // It didn't auto boot $this->assertNull($this->app->cache->store('redis-sentinel')); From d2a53a57b3e70574a0b3c15b5254783b9587e143 Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Mon, 12 Apr 2021 13:45:30 +0200 Subject: [PATCH 07/18] install pecl redis on travis --- .travis.yml | 5 ++++- src/Connections/PhpRedisConnection.php | 6 +++--- src/Connectors/PhpRedisConnector.php | 10 ++++++---- ...RetryRedisException.php => RedisRetryException.php} | 2 +- .../Integration/Connections/PhpRedisConnectionTest.php | 8 +++----- 5 files changed, 17 insertions(+), 14 deletions(-) rename src/Exceptions/{RetryRedisException.php => RedisRetryException.php} (68%) diff --git a/.travis.yml b/.travis.yml index 7db9ca1..d541d41 100644 --- a/.travis.yml +++ b/.travis.yml @@ -128,6 +128,9 @@ before_install: install: travis_retry composer install --prefer-dist --no-interaction --no-suggest -before_script: SUPERVISE=no travis_retry ./start-cluster.sh || { cat ./cluster/*.log; false; } +before_script: + - sudo pecl install redis + - echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"` + - SUPERVISE=no travis_retry ./start-cluster.sh || { cat ./cluster/*.log; false; } script: vendor/bin/phpunit --exclude-group ${LUMEN_VERSION:+laravel-only},${HORIZON:-horizon} diff --git a/src/Connections/PhpRedisConnection.php b/src/Connections/PhpRedisConnection.php index 4e8dbdf..5b85f83 100644 --- a/src/Connections/PhpRedisConnection.php +++ b/src/Connections/PhpRedisConnection.php @@ -2,7 +2,7 @@ namespace Monospice\LaravelRedisSentinel\Connections; -use Monospice\LaravelRedisSentinel\Exceptions\RetryRedisException; +use Monospice\LaravelRedisSentinel\Exceptions\RedisRetryException; use Closure; use Illuminate\Redis\Connections\PhpRedisConnection as LaravelPhpRedisConnection; use Redis; @@ -188,7 +188,7 @@ public function command($method, array $parameters = []) * @param callable $callback The operation to execute. * @return mixed The result of the first successful attempt. * - * @throws RedisException After exhausting the allowed number of + * @throws RedisRetryException After exhausting the allowed number of * attempts to reconnect. */ protected function retryOnFailure(callable $callback) @@ -214,6 +214,6 @@ protected function retryOnFailure(callable $callback) } } while ($attempts < $this->retryLimit); - throw new RetryRedisException(sprintf('Reached the reconnect limit of %d attempts', $attempts)); + throw new RedisRetryException(sprintf('Reached the reconnect limit of %d attempts', $attempts)); } } diff --git a/src/Connectors/PhpRedisConnector.php b/src/Connectors/PhpRedisConnector.php index 33b4ab5..90f3098 100644 --- a/src/Connectors/PhpRedisConnector.php +++ b/src/Connectors/PhpRedisConnector.php @@ -154,10 +154,12 @@ private function updateSentinels(RedisSentinel $sentinel, string $currentHost, i 'host' => $currentHost, 'port' => $currentPort, ] - ], array_map(fn ($sentinel) => [ - 'host' => $sentinel['ip'], - 'port' => $sentinel['port'], - ], $sentinel->sentinels($service)) + ], array_map(function ($sentinel) { + return [ + 'host' => $sentinel['ip'], + 'port' => $sentinel['port'], + ]; + }, $sentinel->sentinels($service)) ); } diff --git a/src/Exceptions/RetryRedisException.php b/src/Exceptions/RedisRetryException.php similarity index 68% rename from src/Exceptions/RetryRedisException.php rename to src/Exceptions/RedisRetryException.php index d32736f..571de18 100644 --- a/src/Exceptions/RetryRedisException.php +++ b/src/Exceptions/RedisRetryException.php @@ -4,7 +4,7 @@ use RedisException as PhpRedisException; -class RetryRedisException extends PhpRedisException +class RedisRetryException extends PhpRedisException { // } diff --git a/tests/Integration/Connections/PhpRedisConnectionTest.php b/tests/Integration/Connections/PhpRedisConnectionTest.php index dc915f4..0d41f0e 100644 --- a/tests/Integration/Connections/PhpRedisConnectionTest.php +++ b/tests/Integration/Connections/PhpRedisConnectionTest.php @@ -7,7 +7,7 @@ use Monospice\LaravelRedisSentinel\Connectors\PhpRedisConnector; use Monospice\LaravelRedisSentinel\Tests\Support\DummyException; use Monospice\LaravelRedisSentinel\Tests\Support\IntegrationTestCase; -use Monospice\LaravelRedisSentinel\Exceptions\RetryRedisException; +use Monospice\LaravelRedisSentinel\Exceptions\RedisRetryException; use Redis; use RedisException; @@ -95,11 +95,9 @@ public function testAbortsTransactionOnException() public function testRetriesTransactionWhenConnectionFails() { - $this->expectException(RetryRedisException::class); + $this->expectException(RedisRetryException::class); - $expectedRetries = 2; - - $this->subject = $this->makeConnection($expectedRetries, 0); // retry immediately + $this->subject = $this->makeConnection(1, 0); // retry once and immediately $this->subject->transaction(function () { throw new RedisException(); From 4f555d2ac03e8186ee122ad2fa3979ecde6b029c Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Mon, 12 Apr 2021 13:54:28 +0200 Subject: [PATCH 08/18] travis ci pecl without sudo --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index d541d41..f4202a5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -129,7 +129,7 @@ before_install: install: travis_retry composer install --prefer-dist --no-interaction --no-suggest before_script: - - sudo pecl install redis + - pecl install redis - echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"` - SUPERVISE=no travis_retry ./start-cluster.sh || { cat ./cluster/*.log; false; } From eff0fef41666eaa749df1ed0b444923ea2694cdd Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Mon, 12 Apr 2021 15:15:40 +0200 Subject: [PATCH 09/18] travis ci pecl only php 7 --- .travis.yml | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index f4202a5..42f23e6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -129,8 +129,16 @@ before_install: install: travis_retry composer install --prefer-dist --no-interaction --no-suggest before_script: - - pecl install redis - - echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"` + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.0" ]]; then printf "yes\n" | pecl install redis; fi + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.1" ]]; then printf "yes\n" | pecl install redis; fi + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.2" ]]; then printf "yes\n" | pecl install redis; fi + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.3" ]]; then printf "yes\n" | pecl install redis; fi + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.4" ]]; then printf "yes\n" | pecl install redis; fi + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.0" ]]; then echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"; fi` + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.1" ]]; then echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"; fi` + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.2" ]]; then echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"; fi` + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.3" ]]; then echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"; fi` + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.4" ]]; then echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"; fi` - SUPERVISE=no travis_retry ./start-cluster.sh || { cat ./cluster/*.log; false; } script: vendor/bin/phpunit --exclude-group ${LUMEN_VERSION:+laravel-only},${HORIZON:-horizon} From 923875ef1ad0cda9fcec17bbeb2e3f371b0bb9b3 Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Mon, 12 Apr 2021 18:23:21 +0200 Subject: [PATCH 10/18] travis ci pecl only php 7, edit install lines --- .travis.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 42f23e6..8f454fe 100644 --- a/.travis.yml +++ b/.travis.yml @@ -129,11 +129,11 @@ before_install: install: travis_retry composer install --prefer-dist --no-interaction --no-suggest before_script: - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.0" ]]; then printf "yes\n" | pecl install redis; fi - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.1" ]]; then printf "yes\n" | pecl install redis; fi - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.2" ]]; then printf "yes\n" | pecl install redis; fi - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.3" ]]; then printf "yes\n" | pecl install redis; fi - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.4" ]]; then printf "yes\n" | pecl install redis; fi + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.0" ]]; then pecl install -f redis; fi + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.1" ]]; then pecl install -f redis; fi + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.2" ]]; then pecl install -f redis; fi + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.3" ]]; then pecl install -f redis; fi + - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.4" ]]; then pecl install -f redis; fi - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.0" ]]; then echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"; fi` - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.1" ]]; then echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"; fi` - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.2" ]]; then echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"; fi` From 46cdf3ae66d4f59cd26f2de71285e5c5bac9b743 Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Mon, 12 Apr 2021 22:13:03 +0200 Subject: [PATCH 11/18] fixing travis pecl --- .travis.yml | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8f454fe..4cf0b2a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -125,20 +125,12 @@ before_install: - if [ -n "$LUMEN_VERSION" ]; then composer remove --dev --no-update "laravel/framework"; fi - if [ -n "$LUMEN_VERSION" ]; then composer require --no-update "laravel/lumen-framework:$LUMEN_VERSION"; fi - if [ -z "$HORIZON" ]; then composer remove --dev --no-update "laravel/horizon"; fi + - phpenv config-rm xdebug.ini + - echo "extension = redis.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini install: travis_retry composer install --prefer-dist --no-interaction --no-suggest before_script: - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.0" ]]; then pecl install -f redis; fi - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.1" ]]; then pecl install -f redis; fi - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.2" ]]; then pecl install -f redis; fi - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.3" ]]; then pecl install -f redis; fi - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.4" ]]; then pecl install -f redis; fi - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.0" ]]; then echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"; fi` - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.1" ]]; then echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"; fi` - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.2" ]]; then echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"; fi` - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.3" ]]; then echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"; fi` - - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.4" ]]; then echo "extension=redis.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"; fi` - SUPERVISE=no travis_retry ./start-cluster.sh || { cat ./cluster/*.log; false; } script: vendor/bin/phpunit --exclude-group ${LUMEN_VERSION:+laravel-only},${HORIZON:-horizon} From 03faa65c836b62274840424c9e092ac5359c47de Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Tue, 13 Apr 2021 13:01:18 +0200 Subject: [PATCH 12/18] check if pecl extesion is enabled for tests --- src/Connectors/PhpRedisConnector.php | 15 ++++++++++++++- .../Connections/PhpRedisConnectionTest.php | 17 +++++------------ 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/Connectors/PhpRedisConnector.php b/src/Connectors/PhpRedisConnector.php index 90f3098..867eddf 100644 --- a/src/Connectors/PhpRedisConnector.php +++ b/src/Connectors/PhpRedisConnector.php @@ -4,6 +4,7 @@ use Illuminate\Support\Arr; use Illuminate\Redis\Connectors\PhpRedisConnector as LaravelPhpRedisConnector; +use LogicException; use Monospice\LaravelRedisSentinel\Connections\PhpRedisConnection; use Redis; use RedisSentinel; @@ -88,10 +89,13 @@ public function connect(array $servers, array $options = [ ]) * * @param array $options * @return Redis + * + * @throws LogicException */ protected function createClientWithSentinel(array $options) { $servers = $this->servers; + $service = isset($options['service']) ? $options['service'] : 'mymaster'; $timeout = isset($options['sentinel_timeout']) ? $options['sentinel_timeout'] : 0; $persistent = isset($options['sentinel_peristent']) ? $options['sentinel_peristent'] : null; $retryWait = isset($options['retry_wait']) ? $options['retry_wait'] : 0; @@ -101,11 +105,20 @@ protected function createClientWithSentinel(array $options) // Shuffle the servers to perform some loadbalancing. shuffle($servers); + // Check if the redis extension is enabled. + if (! extension_loaded('redis')) { + throw new LogicException('Please make sure the PHP Redis extension is installed and enabled.'); + } + + // Check if the extension is up to date and contains RedisSentinel. + if (! class_exists(RedisSentinel::class)) { + throw new LogicException('Please make sure the PHP Redis extension is up to date.'); + } + // Try to connect to any of the servers. foreach ($servers as $idx => $server) { $host = isset($server['host']) ? $server['host'] : 'localhost'; $port = isset($server['port']) ? $server['port'] : 26739; - $service = isset($options['service']) ? $options['service'] : 'mymaster'; // Create a connection to the Sentinel instance. $sentinel = new RedisSentinel($host, $port, $timeout, $persistent, $retryWait, $readTimeout); diff --git a/tests/Integration/Connections/PhpRedisConnectionTest.php b/tests/Integration/Connections/PhpRedisConnectionTest.php index 0d41f0e..817f881 100644 --- a/tests/Integration/Connections/PhpRedisConnectionTest.php +++ b/tests/Integration/Connections/PhpRedisConnectionTest.php @@ -2,7 +2,6 @@ namespace Monospice\LaravelRedisSentinel\Tests\Integration\Connections; -use Mockery; use Monospice\LaravelRedisSentinel\Connections\PhpRedisConnection; use Monospice\LaravelRedisSentinel\Connectors\PhpRedisConnector; use Monospice\LaravelRedisSentinel\Tests\Support\DummyException; @@ -29,19 +28,13 @@ public function setUp() { parent::setUp(); - $this->subject = $this->makeConnection(); - } + if (! extension_loaded('redis')) { + $this->markTestSkipped('The redis extension is not installed. Please install the extension to enable '.__CLASS__); - /** - * Run this cleanup after each test. - * - * @return void - */ - public function tearDown() - { - parent::tearDown(); + return; + } - Mockery::close(); + $this->subject = $this->makeConnection(); } public function testAllowsTransactionsOnAggregateConnection() From 7681bcfb7addbcaf95c5eadcec0d489bcf540a60 Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Tue, 13 Apr 2021 13:07:59 +0200 Subject: [PATCH 13/18] improve tests skipping --- .travis.yml | 3 +- .../Connections/PhpRedisConnectionTest.php | 38 ++++++++++++++++++- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4cf0b2a..ccbf727 100644 --- a/.travis.yml +++ b/.travis.yml @@ -130,7 +130,6 @@ before_install: install: travis_retry composer install --prefer-dist --no-interaction --no-suggest -before_script: - - SUPERVISE=no travis_retry ./start-cluster.sh || { cat ./cluster/*.log; false; } +before_script: SUPERVISE=no travis_retry ./start-cluster.sh || { cat ./cluster/*.log; false; } script: vendor/bin/phpunit --exclude-group ${LUMEN_VERSION:+laravel-only},${HORIZON:-horizon} diff --git a/tests/Integration/Connections/PhpRedisConnectionTest.php b/tests/Integration/Connections/PhpRedisConnectionTest.php index 817f881..87f4d9a 100644 --- a/tests/Integration/Connections/PhpRedisConnectionTest.php +++ b/tests/Integration/Connections/PhpRedisConnectionTest.php @@ -29,8 +29,6 @@ public function setUp() parent::setUp(); if (! extension_loaded('redis')) { - $this->markTestSkipped('The redis extension is not installed. Please install the extension to enable '.__CLASS__); - return; } @@ -39,6 +37,12 @@ public function setUp() public function testAllowsTransactionsOnAggregateConnection() { + if (! extension_loaded('redis')) { + $this->markTestSkipped('The redis extension is not installed. Please install the extension to enable '.__CLASS__); + + return; + } + $transaction = $this->subject->transaction(); $this->assertInstanceOf(Redis::class, $transaction); @@ -46,6 +50,12 @@ public function testAllowsTransactionsOnAggregateConnection() public function testExecutesCommandsInTransaction() { + if (! extension_loaded('redis')) { + $this->markTestSkipped('The redis extension is not installed. Please install the extension to enable '.__CLASS__); + + return; + } + $result = $this->subject->transaction(function ($trans) { $trans->set('test-key', 'test value'); $trans->get('test-key'); @@ -59,6 +69,12 @@ public function testExecutesCommandsInTransaction() public function testExecutesTransactionsOnMaster() { + if (! extension_loaded('redis')) { + $this->markTestSkipped('The redis extension is not installed. Please install the extension to enable '.__CLASS__); + + return; + } + $expectedSubset = ['role' => 'master']; $info = $this->subject->transaction(function ($transaction) { @@ -70,6 +86,12 @@ public function testExecutesTransactionsOnMaster() public function testAbortsTransactionOnException() { + if (! extension_loaded('redis')) { + $this->markTestSkipped('The redis extension is not installed. Please install the extension to enable '.__CLASS__); + + return; + } + $exception = null; try { @@ -88,6 +110,12 @@ public function testAbortsTransactionOnException() public function testRetriesTransactionWhenConnectionFails() { + if (! extension_loaded('redis')) { + $this->markTestSkipped('The redis extension is not installed. Please install the extension to enable '.__CLASS__); + + return; + } + $this->expectException(RedisRetryException::class); $this->subject = $this->makeConnection(1, 0); // retry once and immediately @@ -99,6 +127,12 @@ public function testRetriesTransactionWhenConnectionFails() public function testCanReconnectWhenConnectionFails() { + if (! extension_loaded('redis')) { + $this->markTestSkipped('The redis extension is not installed. Please install the extension to enable '.__CLASS__); + + return; + } + $retries = 3; $attempts = 0; From 165ec41d80225188b970d47a55580fab3728b68a Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Tue, 13 Apr 2021 13:55:41 +0200 Subject: [PATCH 14/18] fixing travis ci --- .travis.yml | 55 +++++++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/.travis.yml b/.travis.yml index ccbf727..0aa57d0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,35 +3,35 @@ dist: trusty sudo: false php: - - 5.6 +# - 5.6 - 7.0 - - 7.1 - - 7.2 - - 7.3 - - 7.4 +# - 7.1 +# - 7.2 +# - 7.3 +# - 7.4 env: - - LARAVEL_VERSION=">=5.4.0 <5.4.20" - - LARAVEL_VERSION=">=5.4.20 <5.5.0" - - LARAVEL_VERSION="5.5.*" - - LARAVEL_VERSION="5.5.*" HORIZON=1 - - LARAVEL_VERSION="5.6.*" - - LARAVEL_VERSION="5.6.*" HORIZON=1 - - LARAVEL_VERSION="5.7.*" - - LARAVEL_VERSION="5.7.*" HORIZON=1 - - LARAVEL_VERSION="5.8.*" - - LARAVEL_VERSION="5.8.*" HORIZON=1 +# - LARAVEL_VERSION=">=5.4.0 <5.4.20" +# - LARAVEL_VERSION=">=5.4.20 <5.5.0" +# - LARAVEL_VERSION="5.5.*" +# - LARAVEL_VERSION="5.5.*" HORIZON=1 +# - LARAVEL_VERSION="5.6.*" +# - LARAVEL_VERSION="5.6.*" HORIZON=1 +# - LARAVEL_VERSION="5.7.*" +# - LARAVEL_VERSION="5.7.*" HORIZON=1 +# - LARAVEL_VERSION="5.8.*" +# - LARAVEL_VERSION="5.8.*" HORIZON=1 - LARAVEL_VERSION="6.*" - - LARAVEL_VERSION="6.*" HORIZON=1 - - LARAVEL_VERSION="7.*" - - LARAVEL_VERSION="7.*" HORIZON=1 - - LUMEN_VERSION="5.4.*" - - LUMEN_VERSION="5.5.*" - - LUMEN_VERSION="5.6.*" - - LUMEN_VERSION="5.7.*" - - LUMEN_VERSION="5.8.*" - - LUMEN_VERSION="6.*" - - LUMEN_VERSION="7.*" +# - LARAVEL_VERSION="6.*" HORIZON=1 +# - LARAVEL_VERSION="7.*" +# - LARAVEL_VERSION="7.*" HORIZON=1 +# - LUMEN_VERSION="5.4.*" +# - LUMEN_VERSION="5.5.*" +# - LUMEN_VERSION="5.6.*" +# - LUMEN_VERSION="5.7.*" +# - LUMEN_VERSION="5.8.*" +# - LUMEN_VERSION="6.*" +# - LUMEN_VERSION="7.*" matrix: exclude: @@ -125,8 +125,9 @@ before_install: - if [ -n "$LUMEN_VERSION" ]; then composer remove --dev --no-update "laravel/framework"; fi - if [ -n "$LUMEN_VERSION" ]; then composer require --no-update "laravel/lumen-framework:$LUMEN_VERSION"; fi - if [ -z "$HORIZON" ]; then composer remove --dev --no-update "laravel/horizon"; fi - - phpenv config-rm xdebug.ini - - echo "extension = redis.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini + - if [[ ${TRAVIS_PHP_VERSION:0:1} == "7" ]]; then printf "\n" | pecl install -f redis; fi + - if [[ ${TRAVIS_PHP_VERSION:0:1} == "7" ]]; then phpenv config-rm xdebug.ini; fi + - if [[ ${TRAVIS_PHP_VERSION:0:1} == "7" ]]; then echo "extension = redis.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini; fi install: travis_retry composer install --prefer-dist --no-interaction --no-suggest From dcd03708694e7c777e685d0e7ad1ade1d0c79092 Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Tue, 13 Apr 2021 15:10:48 +0200 Subject: [PATCH 15/18] fixing laravel 5 PhpRedisConnection --- .travis.yml | 52 +++++++++++++------------- src/Connections/PhpRedisConnection.php | 14 +++++++ 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0aa57d0..f11f563 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,35 +3,35 @@ dist: trusty sudo: false php: -# - 5.6 + - 5.6 - 7.0 -# - 7.1 -# - 7.2 -# - 7.3 -# - 7.4 + - 7.1 + - 7.2 + - 7.3 + - 7.4 env: -# - LARAVEL_VERSION=">=5.4.0 <5.4.20" -# - LARAVEL_VERSION=">=5.4.20 <5.5.0" -# - LARAVEL_VERSION="5.5.*" -# - LARAVEL_VERSION="5.5.*" HORIZON=1 -# - LARAVEL_VERSION="5.6.*" -# - LARAVEL_VERSION="5.6.*" HORIZON=1 -# - LARAVEL_VERSION="5.7.*" -# - LARAVEL_VERSION="5.7.*" HORIZON=1 -# - LARAVEL_VERSION="5.8.*" -# - LARAVEL_VERSION="5.8.*" HORIZON=1 + - LARAVEL_VERSION=">=5.4.0 <5.4.20" + - LARAVEL_VERSION=">=5.4.20 <5.5.0" + - LARAVEL_VERSION="5.5.*" + - LARAVEL_VERSION="5.5.*" HORIZON=1 + - LARAVEL_VERSION="5.6.*" + - LARAVEL_VERSION="5.6.*" HORIZON=1 + - LARAVEL_VERSION="5.7.*" + - LARAVEL_VERSION="5.7.*" HORIZON=1 + - LARAVEL_VERSION="5.8.*" + - LARAVEL_VERSION="5.8.*" HORIZON=1 - LARAVEL_VERSION="6.*" -# - LARAVEL_VERSION="6.*" HORIZON=1 -# - LARAVEL_VERSION="7.*" -# - LARAVEL_VERSION="7.*" HORIZON=1 -# - LUMEN_VERSION="5.4.*" -# - LUMEN_VERSION="5.5.*" -# - LUMEN_VERSION="5.6.*" -# - LUMEN_VERSION="5.7.*" -# - LUMEN_VERSION="5.8.*" -# - LUMEN_VERSION="6.*" -# - LUMEN_VERSION="7.*" + - LARAVEL_VERSION="6.*" HORIZON=1 + - LARAVEL_VERSION="7.*" + - LARAVEL_VERSION="7.*" HORIZON=1 + - LUMEN_VERSION="5.4.*" + - LUMEN_VERSION="5.5.*" + - LUMEN_VERSION="5.6.*" + - LUMEN_VERSION="5.7.*" + - LUMEN_VERSION="5.8.*" + - LUMEN_VERSION="6.*" + - LUMEN_VERSION="7.*" matrix: exclude: @@ -126,8 +126,6 @@ before_install: - if [ -n "$LUMEN_VERSION" ]; then composer require --no-update "laravel/lumen-framework:$LUMEN_VERSION"; fi - if [ -z "$HORIZON" ]; then composer remove --dev --no-update "laravel/horizon"; fi - if [[ ${TRAVIS_PHP_VERSION:0:1} == "7" ]]; then printf "\n" | pecl install -f redis; fi - - if [[ ${TRAVIS_PHP_VERSION:0:1} == "7" ]]; then phpenv config-rm xdebug.ini; fi - - if [[ ${TRAVIS_PHP_VERSION:0:1} == "7" ]]; then echo "extension = redis.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini; fi install: travis_retry composer install --prefer-dist --no-interaction --no-suggest diff --git a/src/Connections/PhpRedisConnection.php b/src/Connections/PhpRedisConnection.php index 5b85f83..5e32bf8 100644 --- a/src/Connections/PhpRedisConnection.php +++ b/src/Connections/PhpRedisConnection.php @@ -22,6 +22,15 @@ */ class PhpRedisConnection extends LaravelPhpRedisConnection { + /** + * The connection creation callback. + * + * Laravel 5 does not store the connector by default. + * + * @var callable + */ + protected $connector; + /** * The number of times the client attempts to retry a command when it fails * to connect to a Redis instance behind Sentinel. @@ -50,6 +59,11 @@ public function __construct($client, callable $connector = null, array $sentinel { parent::__construct($client, $connector); + // Set the connector when it is not set. Used for Laravel 5. + if (! $this->connector) { + $this->connector = $connector; + } + $this->retryLimit = (int) (isset($sentinelOptions['retry_limit']) ? $sentinelOptions['retry_limit'] : 20); $this->retryWait = (int) (isset($sentinelOptions['retry_wait']) ? $sentinelOptions['retry_wait'] : 1000); } From a60ce796189b9026105902463715dafd474281ff Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Wed, 14 Apr 2021 14:30:23 +0200 Subject: [PATCH 16/18] implementing retries on first connection --- README.md | 30 +++++++- src/Connections/PhpRedisConnection.php | 2 +- src/Connectors/PhpRedisConnector.php | 68 +++++++++++++++++-- .../Connectors/PhpRedisConnectorTest.php | 67 ++++++++++++++++++ 4 files changed, 159 insertions(+), 8 deletions(-) create mode 100644 tests/Integration/Connectors/PhpRedisConnectorTest.php diff --git a/README.md b/README.md index 3b1c6dc..ee51af7 100644 --- a/README.md +++ b/README.md @@ -59,10 +59,13 @@ Requirements - PHP 5.4 or greater - [Redis][redis] 2.8 or greater (for Sentinel support) - - [Predis][predis] 1.1 or greater (for Sentinel client support) - [Laravel][laravel] or [Lumen][lumen] 5.0 or greater (4.x doesn't support the required Predis version) +Driver options: + - [Predis][predis] 1.1 or greater (for Sentinel client support) + - [PhpRedis][php-redis] 5.3.4 or greater (for Sentinel client support) + **Note:** Laravel 5.4 introduced the ability to use the [PhpRedis][php-redis] extension as a Redis client for the framework. This package does not yet support the PhpRedis option. @@ -132,6 +135,7 @@ QUEUE_CONNECTION=redis-sentinel # Laravel >= 5.7 QUEUE_DRIVER=redis-sentinel # Laravel <= 5.6 REDIS_DRIVER=redis-sentinel +REDIS_CLIENT=predis REDIS_HOST=sentinel1.example.com, sentinel2.example.com, 10.0.0.1, etc. REDIS_PORT=26379 REDIS_SENTINEL_SERVICE=mymaster # or your Redis master group name @@ -409,6 +413,30 @@ for a single connection. The default values are shown below: ], ``` +The PhpRedis client supports extra options. The default values are shown below: + +```php +'options' => [ + ... + + // The default number of attempts to retry the connection if the inititial + // connection has failed. A value of 0 instructs the + // client to throw an exception after the first failed attempt, while a + // value of -1 causes the client to continue to retry commands indefinitely. + 'connector_retry_limit' => 20, + + // The default amount of time (in milliseconds) that the client waits before + // retrying the connection attempt. + 'connector_retry_wait' => 1000, + + // Sets the persistent option in the RedisSentinel class. + 'sentinel_persistent' => null, + + // Sets the read timeout option in the RedisSentinel class. 0 means unlimited. + 'sentinel_read_timeout' => 0, +], +``` + ### Broadcasting, Cache, Session, and Queue Drivers After configuring the Sentinel database connections, we can instruct Laravel to diff --git a/src/Connections/PhpRedisConnection.php b/src/Connections/PhpRedisConnection.php index 5e32bf8..0cd33a5 100644 --- a/src/Connections/PhpRedisConnection.php +++ b/src/Connections/PhpRedisConnection.php @@ -226,7 +226,7 @@ protected function retryOnFailure(callable $callback) $attempts++; } - } while ($attempts < $this->retryLimit); + } while ($attempts <= $this->retryLimit); throw new RedisRetryException(sprintf('Reached the reconnect limit of %d attempts', $attempts)); } diff --git a/src/Connectors/PhpRedisConnector.php b/src/Connectors/PhpRedisConnector.php index 867eddf..b65a27e 100644 --- a/src/Connectors/PhpRedisConnector.php +++ b/src/Connectors/PhpRedisConnector.php @@ -6,6 +6,7 @@ use Illuminate\Redis\Connectors\PhpRedisConnector as LaravelPhpRedisConnector; use LogicException; use Monospice\LaravelRedisSentinel\Connections\PhpRedisConnection; +use Monospice\LaravelRedisSentinel\Exceptions\RedisRetryException; use Redis; use RedisSentinel; use RedisException; @@ -28,6 +29,22 @@ class PhpRedisConnector extends LaravelPhpRedisConnector */ protected $servers; + /** + * The number of times the client attempts to retry a command when it fails + * to connect to a Redis instance behind Sentinel. + * + * @var int + */ + protected $retryLimit = 20; + + /** + * The time in milliseconds to wait before the client retries a failed + * command. + * + * @var int + */ + protected $retryWait = 1000; + /** * Configuration options specific to Sentinel connection operation * @@ -64,6 +81,10 @@ public function connect(array $servers, array $options = [ ]) return $this->formatServer($server); }, $servers); + // Set the connector options. + $this->retryLimit = (int) (isset($options['connector_retry_limit']) ? $options['connector_retry_limit'] : 20); + $this->retryWait = (int) (isset($options['connector_retry_wait']) ? $options['connector_retry_wait'] : 1000); + // Merge the global options shared by all Sentinel connections with // connection-specific options $clientOpts = array_merge($options, Arr::pull($servers, 'options', [ ])); @@ -81,7 +102,12 @@ public function connect(array $servers, array $options = [ ]) return $this->createClientWithSentinel($options); }; - return new PhpRedisConnection($connector(), $connector, $sentinelOpts); + // Create a connection and retry if this fails. + $connection = $this->retryOnFailure(function () use ($connector) { + return $connector(); + }); + + return new PhpRedisConnection($connection, $connector, $sentinelOpts); } /** @@ -138,9 +164,10 @@ protected function createClientWithSentinel(array $options) } // Create a PhpRedis client for the selected master node. - return $this->createClient( - array_merge($parameters, $server, ['host' => $master[0], 'port' => $master[1]]) - ); + return $this->createClient(array_merge($parameters, $server, [ + 'host' => $master[0], + 'port' => $master[1], + ])); } catch (RedisException $e) { // Rethrow the expection when the last server is reached. if ($idx === count($servers) - 1) { @@ -150,6 +177,35 @@ protected function createClientWithSentinel(array $options) } } + /** + * Retry the callback when a RedisException is catched. + * + * @param callable $callback The operation to execute. + * @return mixed The result of the first successful attempt. + * + * @throws RedisRetryException After exhausting the allowed number of + * attempts to connect. + */ + protected function retryOnFailure(callable $callback) + { + $attempts = 0; + $previous = null; + + do { + try { + return $callback(); + } catch (RedisException $exception) { + $previous = $exception; + + usleep($this->retryWait * 1000); + + $attempts++; + } + } while ($attempts < $this->retryLimit); + + throw new RedisRetryException(sprintf('Reached the connect limit of %d attempts', $attempts), 0, $previous); + } + /** * Update the list With sentinel servers. * @@ -159,7 +215,7 @@ protected function createClientWithSentinel(array $options) * @param string $service * @return void */ - private function updateSentinels(RedisSentinel $sentinel, string $currentHost, int $currentPort, string $service) + protected function updateSentinels(RedisSentinel $sentinel, string $currentHost, int $currentPort, string $service) { $this->servers = array_merge( [ @@ -184,7 +240,7 @@ private function updateSentinels(RedisSentinel $sentinel, string $currentHost, i * * @throws RedisException */ - private function formatServer($server) + protected function formatServer($server) { if (is_string($server)) { list($host, $port) = explode(':', $server); diff --git a/tests/Integration/Connectors/PhpRedisConnectorTest.php b/tests/Integration/Connectors/PhpRedisConnectorTest.php new file mode 100644 index 0000000..0f36bfb --- /dev/null +++ b/tests/Integration/Connectors/PhpRedisConnectorTest.php @@ -0,0 +1,67 @@ +markTestSkipped('The redis extension is not installed. Please install the extension to enable '.__CLASS__); + + return; + } + + $connector = new PhpRedisConnector(); + $client = $connector->connect($this->config['default'], $this->config['options']); + + $this->assertInstanceOf(PhpRedisConnection::class, $client); + } + + public function testRetriesTransactionWhenConnectionFails() + { + if (! extension_loaded('redis')) { + $this->markTestSkipped('The redis extension is not installed. Please install the extension to enable '.__CLASS__); + + return; + } + + $this->expectException(RedisRetryException::class); + + $connector = new PhpRedisConnector(); + + $servers = [ + [ + 'host' => '127.0.0.1', + 'port' => 1111, + ], + ]; + + $options = array_merge([ + 'connector_retry_limit' => 3, + 'connector_retry_wait' => 0, + ]); + + $connector = new PhpRedisConnector(); + $connector->connect($servers, $options); + } +} From e6e623a8c15117c188dca706777160fbecd76813 Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Wed, 12 May 2021 13:11:25 +0200 Subject: [PATCH 17/18] working on merge request feedback --- src/Configuration/HostNormalizer.php | 2 +- src/Connections/PhpRedisConnection.php | 44 ++++--- src/Connectors/PhpRedisConnector.php | 151 ++++++++++++++----------- 3 files changed, 105 insertions(+), 92 deletions(-) diff --git a/src/Configuration/HostNormalizer.php b/src/Configuration/HostNormalizer.php index c4b1f89..6c220bd 100644 --- a/src/Configuration/HostNormalizer.php +++ b/src/Configuration/HostNormalizer.php @@ -62,7 +62,7 @@ class HostNormalizer public static function normalizeConnections(array $connections) { foreach ($connections as $name => $connection) { - if ($name === 'client' || $name === 'options' || $name === 'clusters') { + if (in_array($name, ['client', 'options', 'clusters'])) { continue; } diff --git a/src/Connections/PhpRedisConnection.php b/src/Connections/PhpRedisConnection.php index 0cd33a5..a6415ad 100644 --- a/src/Connections/PhpRedisConnection.php +++ b/src/Connections/PhpRedisConnection.php @@ -2,9 +2,9 @@ namespace Monospice\LaravelRedisSentinel\Connections; -use Monospice\LaravelRedisSentinel\Exceptions\RedisRetryException; use Closure; use Illuminate\Redis\Connections\PhpRedisConnection as LaravelPhpRedisConnection; +use Monospice\LaravelRedisSentinel\Connectors\PhpRedisConnector; use Redis; use RedisException; @@ -27,7 +27,7 @@ class PhpRedisConnection extends LaravelPhpRedisConnection * * Laravel 5 does not store the connector by default. * - * @var callable + * @var callable|null */ protected $connector; @@ -64,8 +64,15 @@ public function __construct($client, callable $connector = null, array $sentinel $this->connector = $connector; } - $this->retryLimit = (int) (isset($sentinelOptions['retry_limit']) ? $sentinelOptions['retry_limit'] : 20); - $this->retryWait = (int) (isset($sentinelOptions['retry_wait']) ? $sentinelOptions['retry_wait'] : 1000); + // Set the retry limit. + if (isset($sentinelOptions['retry_limit']) && is_numeric($sentinelOptions['retry_limit'])) { + $this->retryLimit = (int) $sentinelOptions['retry_limit']; + } + + // Set the retry wait. + if (isset($sentinelOptions['retry_wait']) && is_numeric($sentinelOptions['retry_wait'])) { + $this->retryWait = (int) $sentinelOptions['retry_wait']; + } } /** @@ -201,33 +208,20 @@ public function command($method, array $parameters = []) * * @param callable $callback The operation to execute. * @return mixed The result of the first successful attempt. - * - * @throws RedisRetryException After exhausting the allowed number of - * attempts to reconnect. */ protected function retryOnFailure(callable $callback) { - $attempts = 0; + return PhpRedisConnector::retryOnFailure($callback, $this->retryLimit, $this->retryWait, function () { + $this->client->close(); - do { try { - return $callback(); - } catch (RedisException $exception) { - $this->client->close(); - - usleep($this->retryWait * 1000); - - try { - $this->client = $this->connector ? call_user_func($this->connector) : $this->client; - } catch (RedisException $e) { - // Ignore the the creation of a new client gets an exception. - // If this exception isn't caught the retry will stop. + if ($this->connector) { + $this->client = call_user_func($this->connector); } - - $attempts++; + } catch (RedisException $e) { + // Ignore when the creation of a new client gets an exception. + // If this exception isn't caught the retry will stop. } - } while ($attempts <= $this->retryLimit); - - throw new RedisRetryException(sprintf('Reached the reconnect limit of %d attempts', $attempts)); + }); } } diff --git a/src/Connectors/PhpRedisConnector.php b/src/Connectors/PhpRedisConnector.php index b65a27e..4fa18c0 100644 --- a/src/Connectors/PhpRedisConnector.php +++ b/src/Connectors/PhpRedisConnector.php @@ -35,7 +35,7 @@ class PhpRedisConnector extends LaravelPhpRedisConnector * * @var int */ - protected $retryLimit = 20; + protected $connectorRetryLimit = 20; /** * The time in milliseconds to wait before the client retries a failed @@ -43,7 +43,7 @@ class PhpRedisConnector extends LaravelPhpRedisConnector * * @var int */ - protected $retryWait = 1000; + protected $connectorRetryWait = 1000; /** * Configuration options specific to Sentinel connection operation @@ -63,6 +63,20 @@ class PhpRedisConnector extends LaravelPhpRedisConnector 'sentinel_read_timeout' => null, ]; + /** + * Instantiate the connector and check if the required extension is available. + */ + public function __construct() + { + if (! extension_loaded('redis')) { + throw new LogicException('Please make sure the PHP Redis extension is installed and enabled.'); + } + + if (! class_exists(RedisSentinel::class)) { + throw new LogicException('Please make sure the PHP Redis extension is up to date (5.3.4 or greater).'); + } + } + /** * Create a new Redis Sentinel connection from the provided configuration * options @@ -81,9 +95,15 @@ public function connect(array $servers, array $options = [ ]) return $this->formatServer($server); }, $servers); - // Set the connector options. - $this->retryLimit = (int) (isset($options['connector_retry_limit']) ? $options['connector_retry_limit'] : 20); - $this->retryWait = (int) (isset($options['connector_retry_wait']) ? $options['connector_retry_wait'] : 1000); + // Set the connector retry limit. + if (isset($options['connector_retry_limit']) && is_numeric($options['connector_retry_limit'])) { + $this->connectorRetryLimit = (int) $options['connector_retry_limit']; + } + + // Set the connector retry wait. + if (isset($options['connector_retry_wait']) && is_numeric($options['connector_retry_wait'])) { + $this->connectorRetryWait = (int) $options['connector_retry_wait']; + } // Merge the global options shared by all Sentinel connections with // connection-specific options @@ -103,9 +123,9 @@ public function connect(array $servers, array $options = [ ]) }; // Create a connection and retry if this fails. - $connection = $this->retryOnFailure(function () use ($connector) { + $connection = self::retryOnFailure(function () use ($connector) { return $connector(); - }); + }, $this->connectorRetryLimit, $this->connectorRetryWait); return new PhpRedisConnection($connection, $connector, $sentinelOpts); } @@ -120,92 +140,55 @@ public function connect(array $servers, array $options = [ ]) */ protected function createClientWithSentinel(array $options) { - $servers = $this->servers; - $service = isset($options['service']) ? $options['service'] : 'mymaster'; - $timeout = isset($options['sentinel_timeout']) ? $options['sentinel_timeout'] : 0; - $persistent = isset($options['sentinel_peristent']) ? $options['sentinel_peristent'] : null; - $retryWait = isset($options['retry_wait']) ? $options['retry_wait'] : 0; - $readTimeout = isset($options['sentinel_read_timeout']) ? $options['sentinel_read_timeout'] : 0; - $parameters = isset($options['parameters']) ? $options['parameters'] : []; - - // Shuffle the servers to perform some loadbalancing. - shuffle($servers); - - // Check if the redis extension is enabled. - if (! extension_loaded('redis')) { - throw new LogicException('Please make sure the PHP Redis extension is installed and enabled.'); - } + $serverConfigurations = $this->servers; + $clientConfiguration = isset($options['parameters']) ? $options['parameters'] : []; - // Check if the extension is up to date and contains RedisSentinel. - if (! class_exists(RedisSentinel::class)) { - throw new LogicException('Please make sure the PHP Redis extension is up to date.'); - } + $updateSentinels = isset($options['update_sentinels']) ? $options['update_sentinels'] : false; + $sentinelService = isset($options['service']) ? $options['service'] : 'mymaster'; + $sentinelTimeout = isset($options['sentinel_timeout']) ? $options['sentinel_timeout'] : 0; + $sentinelPersistent = isset($options['sentinel_persistent']) ? $options['sentinel_persistent'] : null; + $sentinelReadTimeout = isset($options['sentinel_read_timeout']) ? $options['sentinel_read_timeout'] : 0; + + // Shuffle the server configurations to perform some loadbalancing. + shuffle($serverConfigurations); // Try to connect to any of the servers. - foreach ($servers as $idx => $server) { - $host = isset($server['host']) ? $server['host'] : 'localhost'; - $port = isset($server['port']) ? $server['port'] : 26739; + foreach ($serverConfigurations as $idx => $serverConfiguration) { + $host = isset($serverConfiguration['host']) ? $serverConfiguration['host'] : 'localhost'; + $port = isset($serverConfiguration['port']) ? $serverConfiguration['port'] : 26379; - // Create a connection to the Sentinel instance. - $sentinel = new RedisSentinel($host, $port, $timeout, $persistent, $retryWait, $readTimeout); + // Create a connection to the Sentinel instance. Using a retry_interval of 0, retrying + // is done inside the PhpRedisConnection. Cannot seem to get the retry_interval to work: + // https://github.com/phpredis/phpredis/blob/37a90257d09b4efa75230769cf535484116b2b67/library.c#L343 + $sentinel = new RedisSentinel($host, $port, $sentinelTimeout, $sentinelPersistent, 0, $sentinelReadTimeout); try { // Check if the Sentinel server list needs to be updated. // Put the current server and the other sentinels in the server list. - $updateSentinels = isset($options['update_sentinels']) ? $options['update_sentinels'] : false; if ($updateSentinels === true) { - $this->updateSentinels($sentinel, $host, $port, $service); + $this->updateSentinels($sentinel, $host, $port, $sentinelService); } // Lookup the master node. - $master = $sentinel->getMasterAddrByName($service); + $master = $sentinel->getMasterAddrByName($sentinelService); if (is_array($master) && ! count($master)) { - throw new RedisException(sprintf('No master found for service "%s".', $service)); + throw new RedisException(sprintf('No master found for service "%s".', $sentinelService)); } // Create a PhpRedis client for the selected master node. - return $this->createClient(array_merge($parameters, $server, [ + return $this->createClient(array_merge($clientConfiguration, $serverConfiguration, [ 'host' => $master[0], 'port' => $master[1], ])); } catch (RedisException $e) { // Rethrow the expection when the last server is reached. - if ($idx === count($servers) - 1) { + if ($idx === count($serverConfigurations) - 1) { throw $e; } } } } - /** - * Retry the callback when a RedisException is catched. - * - * @param callable $callback The operation to execute. - * @return mixed The result of the first successful attempt. - * - * @throws RedisRetryException After exhausting the allowed number of - * attempts to connect. - */ - protected function retryOnFailure(callable $callback) - { - $attempts = 0; - $previous = null; - - do { - try { - return $callback(); - } catch (RedisException $exception) { - $previous = $exception; - - usleep($this->retryWait * 1000); - - $attempts++; - } - } while ($attempts < $this->retryLimit); - - throw new RedisRetryException(sprintf('Reached the connect limit of %d attempts', $attempts), 0, $previous); - } - /** * Update the list With sentinel servers. * @@ -257,4 +240,40 @@ protected function formatServer($server) return $server; } + + /** + * Retry the callback when a RedisException is catched. + * + * @param callable $callback The operation to execute. + * @param int $retryLimit The number of times the retry is performed. + * @param int $retryWait The time in milliseconds to wait before retrying again. + * @param callable $failureCallback The operation to execute when a failure happens. + * @return mixed The result of the first successful attempt. + * + * @throws RedisRetryException After exhausting the allowed number of + * attempts to connect. + */ + public static function retryOnFailure(callable $callback, int $retryLimit, int $retryWait, callable $failureCallback = null) + { + $attempts = 0; + $previousException = null; + + while ($attempts < $retryLimit) { + try { + return $callback(); + } catch (RedisException $exception) { + $previousException = $exception; + + if ($failureCallback) { + call_user_func($failureCallback); + } + + usleep($retryWait * 1000); + + $attempts++; + } + } + + throw new RedisRetryException(sprintf('Reached the (re)connect limit of %d attempts', $attempts), 0, $previousException); + } } From 0f69a2bd7f482a65ac6e9084661efdccfb980701 Mon Sep 17 00:00:00 2001 From: Jeffrey Zant Date: Fri, 25 Feb 2022 10:34:38 +0100 Subject: [PATCH 18/18] Add Laravel 9.x Compatibility --- composer.json | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/composer.json b/composer.json index 4cedab8..d5fc785 100644 --- a/composer.json +++ b/composer.json @@ -1,7 +1,12 @@ { "name": "monospice/laravel-redis-sentinel-drivers", "description": "Redis Sentinel integration for Laravel and Lumen.", - "keywords": ["redis", "sentinel", "laravel", "lumen"], + "keywords": [ + "redis", + "sentinel", + "laravel", + "lumen" + ], "type": "library", "license": "MIT", "authors": [ @@ -12,21 +17,21 @@ ], "require": { "php": ">=5.6.4", - "illuminate/cache": "^5.4 || ^6.0 || ^7.0 || ^8.0", - "illuminate/contracts": "^5.4 || ^6.0 || ^7.0 || ^8.0", - "illuminate/queue": "^5.4 || ^6.0 || ^7.0 || ^8.0", - "illuminate/redis": "^5.4 || ^6.0 || ^7.0 || ^8.0", - "illuminate/session": "^5.4 || ^6.0 || ^7.0 || ^8.0", - "illuminate/support": "^5.4 || ^6.0 || ^7.0 || ^8.0", + "illuminate/cache": "^5.4 || ^6.0 || ^7.0 || ^8.0 || ^9.0", + "illuminate/contracts": "^5.4 || ^6.0 || ^7.0 || ^8.0 || ^9.0", + "illuminate/queue": "^5.4 || ^6.0 || ^7.0 || ^8.0 || ^9.0", + "illuminate/redis": "^5.4 || ^6.0 || ^7.0 || ^8.0 || ^9.0", + "illuminate/session": "^5.4 || ^6.0 || ^7.0 || ^8.0 || ^9.0", + "illuminate/support": "^5.4 || ^6.0 || ^7.0 || ^8.0 || ^9.0", "monospice/spicy-identifiers": "^0.1", "predis/predis": "^1.1" }, "require-dev": { - "laravel/framework": "^5.4 || ^6.0 || ^7.0 || ^8.0", - "laravel/horizon": "^1.0 || ^2.0 || ^3.0 || ^4.0", - "laravel/lumen-framework": "^5.4 || ^6.0 || ^7.0 || ^8.0", + "laravel/framework": "^5.4 || ^6.0 || ^7.0 || ^8.0 || ^9.0", + "laravel/horizon": "^1.0 || ^2.0 || ^3.0 || ^4.0 || ^5.8", + "laravel/lumen-framework": "^5.4 || ^6.0 || ^7.0 || ^8.0 || ^9.0", "mockery/mockery": "^1.3", - "phpunit/phpunit": "^5.0" + "phpunit/phpunit": "^5.0 || ^9.5.10" }, "autoload": { "psr-4": {