Skip to content

Commit

Permalink
Add phpredis Sentinel support
Browse files Browse the repository at this point in the history
  • Loading branch information
ostrolucky committed Dec 7, 2022
1 parent af457cd commit 6f654e4
Show file tree
Hide file tree
Showing 9 changed files with 154 additions and 12 deletions.
8 changes: 8 additions & 0 deletions .github/workflows/continuous-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ jobs:
SYMFONY_REQUIRE: "${{matrix.symfony-require}}"
SYMFONY_DEPRECATIONS_HELPER: "max[self]=0"

services:
redis-sentinel:
image: bitnami/redis-sentinel
env:
REDIS_MASTER_HOST: localhost
ports:
- 26379:26379

strategy:
fail-fast: false
matrix:
Expand Down
2 changes: 1 addition & 1 deletion docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ snc_redis:
Please note that the master dsn connection needs to be tagged with the ```master``` alias.
If not, `predis` will complain.

A setup using `predis` sentinel replication could look like this:
A setup using `predis` or `phpredis` sentinel replication could look like this:

``` yaml
snc_redis:
Expand Down
2 changes: 1 addition & 1 deletion src/DependencyInjection/Configuration/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ private function addClientsSection(ArrayNodeDefinition $rootNode): void
->scalarNode('profile')->defaultValue('default')->end()
->scalarNode('cluster')->defaultNull()->end()
->scalarNode('prefix')->defaultNull()->end()
->enumNode('replication')->values([true, false, 'sentinel'])->end()
->enumNode('replication')->values([true, false, 'sentinel'])->defaultValue(false)->end()
->scalarNode('service')->defaultNull()->end()
->enumNode('slave_failover')->values(['none', 'error', 'distribute', 'distribute_slaves'])->end()
->arrayNode('parameters')
Expand Down
19 changes: 13 additions & 6 deletions src/DependencyInjection/SncRedisExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use LogicException;
use Predis\Command\Processor\KeyPrefixProcessor;
use Predis\Profile\Factory;
use RedisSentinel;
use Snc\RedisBundle\DependencyInjection\Configuration\Configuration;
use Snc\RedisBundle\DependencyInjection\Configuration\RedisDsn;
use Snc\RedisBundle\DependencyInjection\Configuration\RedisEnvDsn;
Expand Down Expand Up @@ -225,16 +226,22 @@ private function loadPredisConnectionParameters(string $clientAlias, array $opti
/** @param mixed[] $options A client configuration */
private function loadPhpredisClient(array $options, ContainerBuilder $container): void
{
$connectionCount = count($options['dsns']);
$hasClusterOption = $options['options']['cluster'] !== null;
$connectionCount = count($options['dsns']);
$hasClusterOption = $options['options']['cluster'] !== null;
$hasSentinelOption = (bool) $options['options']['replication'];

if ($connectionCount > 1 && !$hasClusterOption) {
throw new LogicException(sprintf('\RedisArray is not supported yet but \RedisCluster is: set option "cluster" to true to enable it.'));
if ($connectionCount > 1 && !$hasClusterOption && !$hasSentinelOption) {
throw new LogicException('Use options "cluster" or "sentinel" to enable support for multi DSN instances.');
}

if ($hasClusterOption && $hasSentinelOption) {
throw new LogicException('You cannot have both cluster and sentinel enabled for same redis connection');
}

$phpredisClientClass = (string) $container->getParameter('snc_redis.phpredis_' . ($hasClusterOption ? 'cluster' : '') . 'client.class');
$phpredisDef = new Definition($phpredisClientClass, [
$phpredisClientClass,

$phpredisDef = new Definition($phpredisClientClass, [
$hasSentinelOption ? RedisSentinel::class : $phpredisClientClass,
array_map('strval', $options['dsns']),
$options['options'],
$options['alias'],
Expand Down
55 changes: 52 additions & 3 deletions src/Factory/PhpredisClientFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@

namespace Snc\RedisBundle\Factory;

use InvalidArgumentException;
use LogicException;
use ProxyManager\Configuration;
use ProxyManager\Factory\AccessInterceptorValueHolderFactory;
use ProxyManager\Proxy\AccessInterceptorInterface;
use Redis;
use RedisCluster;
use RedisSentinel;
use ReflectionClass;
use ReflectionMethod;
use Snc\RedisBundle\DependencyInjection\Configuration\RedisDsn;
Expand All @@ -26,6 +28,7 @@
use function is_array;
use function spl_autoload_register;
use function sprintf;
use function var_export;

/** @internal */
class PhpredisClientFactory
Expand Down Expand Up @@ -67,26 +70,72 @@ public function __construct(callable $interceptor, ?Configuration $proxyConfigur
*/
public function create(string $class, array $dsns, array $options, string $alias, bool $loggingEnabled)
{
if (!is_a($class, Redis::class, true) && !is_a($class, RedisCluster::class, true)) {
throw new LogicException(sprintf('The factory can only instantiate \Redis|\RedisCluster classes: "%s" asked', $class));
$isRedis = is_a($class, Redis::class, true);
$isSentinel = is_a($class, RedisSentinel::class, true);
$isCluster = is_a($class, RedisCluster::class, true);

if (!$isRedis && !$isSentinel && !$isCluster) {
throw new LogicException(sprintf('The factory can only instantiate Redis|RedisCluster|RedisSentinel classes: "%s" asked', $class));
}

// Normalize the DSNs, because using processed environment variables could lead to nested values.
$dsns = count($dsns) === 1 && is_array($dsns[0]) ? $dsns[0] : $dsns;

$parsedDsns = array_map(static fn (string $dsn) => new RedisDsn($dsn), $dsns);

if (is_a($class, Redis::class, true)) {
if ($isRedis) {
if (count($parsedDsns) > 1) {
throw new LogicException('Cannot have more than 1 dsn with \Redis and \RedisArray is not supported yet.');
}

return $this->createClient($parsedDsns[0], $class, $alias, $options, $loggingEnabled);
}

if ($isSentinel) {
return $this->createClientFromSentinel($parsedDsns, $alias, $options, $loggingEnabled);
}

return $this->createClusterClient($parsedDsns, $class, $alias, $options, $loggingEnabled);
}

/**
* @param list<RedisDsn> $dsns
* @param array{service: ?string} $options
*/
private function createClientFromSentinel(array $dsns, string $alias, array $options, bool $loggingEnabled): Redis
{
foreach ($dsns as $dsn) {
$address = (new RedisSentinel($dsn->getHost(), (int) $dsn->getPort()))->getMasterAddrByName($options['service']);

if (!$address) {
continue;
}

return $this->createClient(
new class ($dsn->__toString(), $address[0], (int) $address[1]) extends RedisDsn {
public function __construct(string $dsn, string $host, int $port)
{
parent::__construct($dsn);
$this->host = $host;
$this->port = $port;
}
},
Redis::class,
$alias,
$options,
$loggingEnabled,
);
}

throw new InvalidArgumentException(
sprintf(
'Failed to retrieve master information from sentinel %s and dsn %s.',
var_export($options['service'], true),
var_export($dsns, true),
),
);
}

/**
* @param RedisDsn[] $dsns
* @param mixed[] $options
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
parameters:
env(REDIS_URL_1): redis://localhost:7000

snc_redis:
clients:
phpredissentinel:
type: phpredis
alias: phpredissentinel
dsn: "%env(REDIS_URL_1)%"
options:
replication: sentinel
service: mymaster
38 changes: 37 additions & 1 deletion tests/DependencyInjection/SncRedisExtensionEnvTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Predis\Profile\RedisVersion260;
use Redis;
use RedisCluster;
use RedisSentinel;
use Snc\RedisBundle\DependencyInjection\SncRedisExtension;
use Snc\RedisBundle\Factory\PredisParametersFactory;
use Symfony\Component\Config\FileLocator;
Expand Down Expand Up @@ -63,6 +64,7 @@ public function testPhpredisDefaultParameterConfig(): void
'profile' => 'default',
'cluster' => null,
'prefix' => null,
'replication' => false,
'service' => null,
],
$clientDefinition->getArgument(2),
Expand Down Expand Up @@ -102,6 +104,7 @@ public function testPhpredisFullConfig(): void
'throw_errors' => true,
'profile' => 'default',
'cluster' => null,
'replication' => false,
'service' => null,
],
$clientDefinition->getArgument(2),
Expand Down Expand Up @@ -140,6 +143,7 @@ public function testPhpredisWithAclConfig(): void
'serialization' => 'php',
'service' => null,
'throw_errors' => true,
'replication' => null,
],
$clientDefinition->getArgument(2),
);
Expand Down Expand Up @@ -192,12 +196,43 @@ public function testPhpRedisClusterOption(): void
'serialization' => 'default',
'profile' => 'default',
'prefix' => null,
'replication' => false,
'service' => null,
],
$clientDefinition->getArgument(2),
);
}

public function testPhpRedisSentinelOption(): void
{
$container = $this->getConfiguredContainer('env_phpredis_sentinel');
$clientDefinition = $container->findDefinition('snc_redis.phpredissentinel');

$this->assertSame(Redis::class, $clientDefinition->getClass());
$this->assertSame(RedisSentinel::class, $clientDefinition->getArgument(0));
$this->assertStringContainsString('REDIS_URL_1', $clientDefinition->getArgument(1)[0]);
$this->assertSame('phpredissentinel', $clientDefinition->getArgument(3));
$this->assertFalse($clientDefinition->getArgument(4));

$this->assertSame(
[
'replication' => 'sentinel',
'service' => 'mymaster',
'connection_async' => false,
'connection_persistent' => false,
'connection_timeout' => 5,
'read_write_timeout' => null,
'iterable_multibulk' => false,
'throw_errors' => true,
'serialization' => 'default',
'profile' => 'default',
'cluster' => null,
'prefix' => null,
],
$clientDefinition->getArgument(2),
);
}

public function testPhpRedisClusterOptionMultipleDsn(): void
{
$container = $this->getConfiguredContainer('env_phpredis_cluster_multiple_dsn');
Expand All @@ -222,6 +257,7 @@ public function testPhpRedisClusterOptionMultipleDsn(): void
'serialization' => 'default',
'profile' => 'default',
'prefix' => null,
'replication' => false,
'service' => null,
],
$clientDefinition->getArgument(2),
Expand All @@ -231,7 +267,7 @@ public function testPhpRedisClusterOptionMultipleDsn(): void
public function testPhpRedisArrayIsNotSupported(): void
{
$this->expectException(LogicException::class);
$this->expectExceptionMessage('RedisArray is not supported yet');
$this->expectExceptionMessage('Use options "cluster" or "sentinel" to enable support for multi DSN instances.');

$this->getConfiguredContainer('env_phpredis_array_not_supported');
}
Expand Down
23 changes: 23 additions & 0 deletions tests/Factory/PhpredisClientFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Psr\Log\LoggerInterface;
use Redis;
use RedisCluster;
use RedisSentinel;
use Snc\RedisBundle\Factory\PhpredisClientFactory;
use Snc\RedisBundle\Logger\RedisCallInterceptor;
use Snc\RedisBundle\Logger\RedisLogger;
Expand Down Expand Up @@ -86,6 +87,28 @@ public function testCreateMinimalClusterConfig(): void
$this->assertSame(0, $client->getOption(RedisCluster::OPT_SLAVE_FAILOVER));
}

public function testCreatSentinelConfig(): void
{
$this->logger->method('debug')->withConsecutive(
[$this->stringContains('Executing command "CONNECT 127.0.0.1 6379 5 <null>')],
['Executing command "AUTH sncredis"'],
);
$factory = new PhpredisClientFactory(new RedisCallInterceptor($this->redisLogger));

$client = $factory->create(
RedisSentinel::class,
['redis://sncredis@localhost:26379'],
['connection_timeout' => 5, 'connection_persistent' => false, 'service' => 'mymaster'],
'phpredissentinel',
true,
);

$this->assertInstanceOf(Redis::class, $client);
$this->assertNull($client->getOption(Redis::OPT_PREFIX));
$this->assertSame(0, $client->getOption(Redis::OPT_SERIALIZER));
$this->assertSame('sncredis', $client->getAuth());
}

public function testCreateFullConfig(): void
{
$factory = new PhpredisClientFactory(new RedisCallInterceptor($this->redisLogger));
Expand Down
7 changes: 7 additions & 0 deletions tests/Functional/App/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ snc_redis:
read_write_timeout: 30
iterable_multibulk: false
throw_errors: true
sentinel:
type: phpredis
alias: sentinel
dsn: redis://sncredis@localhost:26379
options:
replication: true
service: mymaster
with_acl:
type: predis
alias: with_acl
Expand Down

0 comments on commit 6f654e4

Please sign in to comment.