diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 762c8e1..7dc0368 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -9,52 +9,40 @@ on: jobs: test-all: - name: Test PHP ${{ matrix.php-version }} + name: Test PHP ${{ matrix.php-version }}, Redis ${{ matrix.redis-version }} runs-on: ${{ matrix.operating-system }} strategy: matrix: operating-system: [ubuntu-latest] - php-version: ['8.1', '8.2', '8.3', '8.4'] + php-version: ['8.2', '8.3', '8.4'] + redis-version: ['7.4.5', '8.0.3'] include: - operating-system: 'ubuntu-latest' php-version: '8.4' + redis-version: '8.0.3' run-sonarqube-analysis: true - services: - redis: - image: bitnami/redis:7.4-debian-12 - ports: - - 6379:6379 - env: - ALLOW_EMPTY_PASSWORD: 'yes' - options: >- - --health-cmd "redis-cli -p 6379 ping" - --health-start-period 5s - --health-interval 10s - --health-timeout 5s - --health-retries 5 - redis-sentinel: - image: bitnami/redis-sentinel:7.4-debian-12 - ports: - - 26379:26379 - env: - REDIS_MASTER_HOST: redis - REDIS_MASTER_SET: mymaster - REDIS_SENTINEL_QUORUM: 1 - options: >- - --health-cmd "redis-cli -p 26379 ping" - --health-start-period 5s - --health-interval 10s - --health-timeout 5s - --health-retries 5 - steps: - uses: actions/checkout@v4 with: fetch-depth: 0 + - name: Install Redis + run: | + wget http://download.redis.io/releases/redis-${{ matrix.redis-version }}.tar.gz + tar xzf redis-${{ matrix.redis-version }}.tar.gz + cd redis-${{ matrix.redis-version }} + make + sudo make install + + - name: Make Redis cluster script executable + run: chmod +x ./start-redis-cluster.sh + + - name: Start Redis cluster + run: ./start-redis-cluster.sh & + - name: Setup PHPUnit uses: shivammathur/setup-php@v2 with: @@ -87,7 +75,7 @@ jobs: env: REDIS_SENTINEL_HOST: 127.0.0.1 REDIS_SENTINEL_PORT: 26379 - REDIS_SENTINEL_SERVICE: mymaster + REDIS_SENTINEL_SERVICE: service1 - name: Prepare paths for SonarQube analysis if: ${{ matrix.run-sonarqube-analysis && !github.event.pull_request.head.repo.fork }} diff --git a/.php-cs-fixer.php b/.php-cs-fixer.php deleted file mode 100644 index 0ee8b53..0000000 --- a/.php-cs-fixer.php +++ /dev/null @@ -1,177 +0,0 @@ - ['syntax' => 'short'], - 'binary_operator_spaces' => [ - 'default' => 'single_space', - 'operators' => ['=>' => null, '|' => null] - ], - 'blank_line_after_namespace' => true, - 'blank_line_after_opening_tag' => true, - 'blank_line_before_statement' => [ - 'statements' => ['break', 'continue', 'declare', 'return', 'throw'] - ], - 'braces' => true, - 'cast_spaces' => true, - 'class_attributes_separation' => [ - 'elements' => ['method' => 'one'] - ], - 'class_definition' => true, - 'concat_space' => [ - 'spacing' => 'none' - ], - 'constant_case' => ['case' => 'lower'], - 'declare_equal_normalize' => true, - 'declare_strict_types' => true, - 'elseif' => true, - 'encoding' => true, - 'full_opening_tag' => true, - 'fully_qualified_strict_types' => true, - 'function_declaration' => true, - 'function_typehint_space' => true, - 'general_phpdoc_tag_rename' => [ - 'case_sensitive' => true, - 'fix_annotation' => true, - 'fix_inline' => true, - 'replacements' => [ - 'inheritDocs' => 'inheritdoc', - ], - ], - 'heredoc_to_nowdoc' => true, - 'include' => true, - 'increment_style' => ['style' => 'post'], - 'indentation_type' => true, - 'linebreak_after_opening_tag' => true, - 'line_ending' => true, - 'lowercase_cast' => true, - 'lowercase_keywords' => true, - 'lowercase_static_reference' => true, - 'magic_method_casing' => true, - 'magic_constant_casing' => true, - 'method_argument_space' => true, - 'native_function_casing' => true, - 'no_alias_functions' => true, - 'no_extra_blank_lines' => [ - 'tokens' => [ - 'extra', - 'throw', - 'use', - 'use_trait', - ] - ], - 'no_blank_lines_after_class_opening' => true, - 'no_blank_lines_after_phpdoc' => true, - 'no_closing_tag' => true, - 'no_empty_phpdoc' => true, - 'no_empty_statement' => true, - 'no_leading_import_slash' => true, - 'no_leading_namespace_whitespace' => true, - 'no_mixed_echo_print' => [ - 'use' => 'echo' - ], - 'no_multiline_whitespace_around_double_arrow' => true, - 'multiline_whitespace_before_semicolons' => [ - 'strategy' => 'no_multi_line' - ], - 'no_short_bool_cast' => true, - 'no_singleline_whitespace_before_semicolons' => true, - 'no_spaces_after_function_name' => true, - 'no_spaces_around_offset' => true, - 'no_spaces_inside_parenthesis' => true, - 'no_trailing_comma_in_list_call' => true, - 'no_trailing_comma_in_singleline_array' => true, - 'no_trailing_whitespace' => true, - 'no_trailing_whitespace_in_comment' => true, - 'no_unneeded_control_parentheses' => true, - 'no_unreachable_default_argument_value' => true, - 'no_useless_return' => true, - 'no_whitespace_before_comma_in_array' => true, - 'no_whitespace_in_blank_line' => true, - 'normalize_index_brace' => true, - 'not_operator_with_successor_space' => true, - 'object_operator_without_whitespace' => true, - 'ordered_imports' => ['sort_algorithm' => 'alpha'], - 'phpdoc_indent' => true, - 'phpdoc_inline_tag_normalizer' => [ - 'tags' => ['example', 'id', 'internal', 'inheritdoc', 'inheritdocs', 'link', 'source', 'toc', 'tutorial'], - ], - 'phpdoc_no_access' => true, - 'phpdoc_no_package' => true, - 'phpdoc_no_useless_inheritdoc' => true, - 'phpdoc_scalar' => true, - 'phpdoc_single_line_var_spacing' => true, - 'phpdoc_summary' => true, - 'phpdoc_tag_type' => [ - 'tags' => [ - 'api' => 'annotation', - 'author' => 'annotation', - 'copyright' => 'annotation', - 'deprecated' => 'annotation', - 'example' => 'annotation', - 'global' => 'annotation', - 'inheritDoc' => 'inline', - 'internal' => 'annotation', - 'license' => 'annotation', - 'method' => 'annotation', - 'package' => 'annotation', - 'param' => 'annotation', - 'property' => 'annotation', - 'return' => 'annotation', - 'see' => 'annotation', - 'since' => 'annotation', - 'throws' => 'annotation', - 'todo' => 'annotation', - 'uses' => 'annotation', - 'var' => 'annotation', - 'version' => 'annotation', - ], - ], - 'phpdoc_to_comment' => false, - 'phpdoc_trim' => true, - 'phpdoc_types' => true, - 'phpdoc_var_without_name' => true, - 'psr_autoloading' => true, - 'self_accessor' => true, - 'short_scalar_cast' => true, - 'simplified_null_return' => false, - 'single_blank_line_at_eof' => true, - 'single_blank_line_before_namespace' => true, - 'single_class_element_per_statement' => true, - 'single_import_per_statement' => true, - 'single_line_after_imports' => true, - 'single_line_comment_style' => [ - 'comment_types' => ['hash'] - ], - 'single_quote' => true, - 'space_after_semicolon' => true, - 'standardize_not_equals' => true, - 'switch_case_semicolon_to_colon' => true, - 'switch_case_space' => true, - 'ternary_operator_spaces' => true, - 'trailing_comma_in_multiline' => true, - 'trim_array_spaces' => true, - 'unary_operator_spaces' => true, - 'visibility_required' => true, - 'whitespace_after_comma_in_array' => true, -]; - -$project_path = getcwd(); -$finder = Finder::create() - ->in([ - $project_path . '/src', - $project_path . '/tests', - ]) - ->name('*.php') - ->ignoreDotFiles(true) - ->ignoreVCS(true); - -return (new Config) - ->setFinder($finder) - ->setRules($rules) - ->setRiskyAllowed(true) - ->setUsingCache(true); diff --git a/README.md b/README.md index 01acca3..cc9f1cf 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,10 @@ To use the Redis Sentinel driver, the `redis` section in `config/database.php` n 'sentinel_read_timeout' => (float) env('REDIS_SENTINEL_READ_TIMEOUT', 0), 'sentinel_username' => env('REDIS_SENTINEL_USERNAME'), 'sentinel_password' => env('REDIS_SENTINEL_PASSWORD'), + + 'connector_retry_attempts' => env('REDIS_CONNECTOR_RETRY_ATTEMPTS'), + 'connector_retry_delay' => env('REDIS_CONNECTOR_RETRY_DELAY'), + 'password' => env('REDIS_PASSWORD'), 'database' => (int) env('REDIS_DB', 0), ] diff --git a/composer.json b/composer.json index b408bf0..62a1997 100644 --- a/composer.json +++ b/composer.json @@ -15,14 +15,14 @@ } ], "require": { - "php": "^8.1", + "php": "^8.2", "ext-redis": "*", "illuminate/contracts": "^8.0|^9.0|^10.0|^11.0|^12.0", "illuminate/redis": "^8.0|^9.0|^10.0|^11.0|^12.0", "illuminate/support": "^8.0|^9.0|^10.0|^11.0|^12.0" }, "require-dev": { - "friendsofphp/php-cs-fixer": "^3.0", + "laravel/pint": "^1.22", "orchestra/testbench": "^6.0|^7.0|^8.0|^9.0|^10.0" }, "scripts": { @@ -30,9 +30,9 @@ "@test:cs", "@test:unit" ], - "test:cs": "PHP_CS_FIXER_IGNORE_ENV=true vendor/bin/php-cs-fixer fix --dry-run --diff --ansi", + "test:cs": "vendor/bin/pint -v", "test:unit": "vendor/bin/phpunit --testdox --log-junit=phpunit.report-junit.xml --coverage-clover=phpunit.coverage-clover.xml --coverage-text", - "fix:cs": "PHP_CS_FIXER_IGNORE_ENV=true vendor/bin/php-cs-fixer fix --diff --ansi" + "fix:cs": "vendor/bin/pint -v" }, "autoload": { "psr-4": { diff --git a/pint.json b/pint.json new file mode 100644 index 0000000..93061b6 --- /dev/null +++ b/pint.json @@ -0,0 +1,3 @@ +{ + "preset": "laravel" +} diff --git a/src/Connections/PhpRedisSentinelConnection.php b/src/Connections/PhpRedisSentinelConnection.php index 61cb4c5..bfe07f7 100644 --- a/src/Connections/PhpRedisSentinelConnection.php +++ b/src/Connections/PhpRedisSentinelConnection.php @@ -8,242 +8,212 @@ use Closure; use Illuminate\Redis\Connections\PhpRedisConnection; -use Illuminate\Support\Str; +use Namoshek\Redis\Sentinel\Exceptions\RetryRedisException; +use Namoshek\Redis\Sentinel\Services\RetryContext; use Redis; use RedisException; +use Throwable; /** * The connection to Redis after connecting through a Sentinel using the PhpRedis extension. */ class PhpRedisSentinelConnection extends PhpRedisConnection { - // The following array contains all exception message parts which are interpreted as a connection loss or - // another unavailability of Redis. - private const ERROR_MESSAGES_INDICATING_UNAVAILABILITY = [ - 'connection closed', - 'connection refused', - 'connection lost', - 'failed while reconnecting', - 'is loading the dataset in memory', - 'php_network_getaddresses', - 'read error on connection', - 'socket', - 'went away', - 'loading', - 'readonly', - "can't write against a read only replica", - ]; + /** + * Create a new PhpRedis connection. + * + * @param \Redis $client + */ + public function __construct( + $client, + ?callable $connector, + array $config, + protected RetryContext $retryContext, + ) { + parent::__construct($client, $connector, $config); + } /** * {@inheritdoc} * - * @throws RedisException + * @throws RetryRedisException */ public function scan($cursor, $options = []): mixed { - try { - return parent::scan($cursor, $options); - } catch (RedisException $e) { - $this->reconnectIfRedisIsUnavailableOrReadonly($e); - - throw $e; - } + return $this->retryOnFailure(fn () => parent::scan($cursor, $options)); } /** * {@inheritdoc} * - * @throws RedisException + * @throws RetryRedisException */ public function zscan($key, $cursor, $options = []): mixed { - try { - return parent::zscan($key, $cursor, $options); - } catch (RedisException $e) { - $this->reconnectIfRedisIsUnavailableOrReadonly($e); - - throw $e; - } + return $this->retryOnFailure(fn () => parent::zscan($key, $cursor, $options)); } /** * {@inheritdoc} * - * @throws RedisException + * @throws RetryRedisException */ public function hscan($key, $cursor, $options = []): mixed { - try { - return parent::hscan($key, $cursor, $options); - } catch (RedisException $e) { - $this->reconnectIfRedisIsUnavailableOrReadonly($e); - - throw $e; - } + return $this->retryOnFailure(fn () => parent::hscan($key, $cursor, $options)); } /** * {@inheritdoc} * - * @throws RedisException + * @throws RetryRedisException */ public function sscan($key, $cursor, $options = []): mixed { - try { - return parent::sscan($key, $cursor, $options); - } catch (RedisException $e) { - $this->reconnectIfRedisIsUnavailableOrReadonly($e); - - throw $e; - } + return $this->retryOnFailure(fn () => parent::sscan($key, $cursor, $options)); } /** * {@inheritdoc} * - * @throws RedisException + * @throws RetryRedisException */ - public function pipeline(?callable $callback = null): Redis|array - { - try { - return parent::pipeline($callback); - } catch (RedisException $e) { - $this->reconnectIfRedisIsUnavailableOrReadonly($e); - - throw $e; - } + public function pipeline( + ?callable $callback = null, + ?int $retryAttempts = null, + ): Redis|array { + return $this->retryOnFailure( + fn () => parent::pipeline($callback), + $retryAttempts, + ); } /** * {@inheritdoc} * - * @throws RedisException + * @throws RetryRedisException */ - public function transaction(?callable $callback = null): Redis|array - { - try { - return parent::transaction($callback); - } catch (RedisException $e) { - $this->reconnectIfRedisIsUnavailableOrReadonly($e); - - throw $e; - } + public function transaction( + ?callable $callback = null, + ?int $retryAttempts = null, + ): Redis|array { + return $this->retryOnFailure( + fn () => parent::transaction($callback), + $retryAttempts, + ); } /** * {@inheritdoc} * - * @throws RedisException + * @throws RetryRedisException */ public function evalsha($script, $numkeys, ...$arguments): mixed { - try { - return parent::evalsha($script, $numkeys, $arguments); - } catch (RedisException $e) { - $this->reconnectIfRedisIsUnavailableOrReadonly($e); - - throw $e; - } + return $this->retryOnFailure(fn () => parent::evalsha($script, $numkeys, ...$arguments)); } /** * {@inheritdoc} * - * @throws RedisException + * @throws RetryRedisException */ public function subscribe($channels, Closure $callback): void { - try { - parent::subscribe($channels, $callback); - } catch (RedisException $e) { - $this->reconnectIfRedisIsUnavailableOrReadonly($e); - - throw $e; - } + $this->retryOnFailure(fn () => parent::subscribe($channels, $callback)); } /** * {@inheritdoc} * - * @throws RedisException + * @throws RetryRedisException */ public function psubscribe($channels, Closure $callback): void { - try { - parent::psubscribe($channels, $callback); - } catch (RedisException $e) { - $this->reconnectIfRedisIsUnavailableOrReadonly($e); - - throw $e; - } + $this->retryOnFailure(fn () => parent::psubscribe($channels, $callback)); } /** * {@inheritdoc} * - * @throws RedisException + * @throws RetryRedisException */ public function flushdb(): void { - try { - parent::flushdb(); - } catch (RedisException $e) { - $this->reconnectIfRedisIsUnavailableOrReadonly($e); - - throw $e; - } + $this->retryOnFailure(fn () => parent::flushdb()); } /** * {@inheritdoc} * - * @throws RedisException + * @throws RetryRedisException */ public function command($method, array $parameters = []): mixed { - try { - return parent::command($method, $parameters); - } catch (RedisException $e) { - $this->reconnectIfRedisIsUnavailableOrReadonly($e); - - throw $e; - } + return $this->retryOnFailure(fn () => parent::command($method, $parameters)); } /** * {@inheritdoc} * - * @throws RedisException + * @throws RetryRedisException */ public function __call($method, $parameters): mixed { - try { - return parent::__call(strtolower($method), $parameters); - } catch (RedisException $e) { - $this->reconnectIfRedisIsUnavailableOrReadonly($e); + return $this->retryOnFailure(fn () => parent::__call(strtolower($method), $parameters)); + } - throw $e; - } + /** + * Attempt to retry the provided operation when the client fails to connect + * to a Redis server. + * + * @param callable $callback The operation to execute. + * @param ?int $retryAttempts The number of times the retry is performed. + * @param ?int $retryDelay The time in milliseconds to wait before retrying again. + * @return mixed The result of the first successful attempt. + * + * @throws RetryRedisException|RedisException + */ + protected function retryOnFailure( + callable $callback, + ?int $retryAttempts = null, + ?int $retryDelay = null, + ): mixed { + return $this->retryContext->retryOnFailure( + $callback, + $retryAttempts, + $retryDelay, + failureCallback: fn () => $this->forceReconnect(), + ); } /** - * Inspects the given exception and reconnects the client if the reported error indicates that the server - * went away or is in readonly mode, which may happen in case of a Redis Sentinel failover. + * Force a reconnect, we ignore naming resolution exceptions. */ - private function reconnectIfRedisIsUnavailableOrReadonly(RedisException $exception): void + protected function forceReconnect(): void { - // We convert the exception message to lower-case in order to perform case-insensitive comparison. - $exceptionMessage = strtolower($exception->getMessage()); - - // Because we also match only partial exception messages, we cannot use in_array() at this point. - foreach (self::ERROR_MESSAGES_INDICATING_UNAVAILABILITY as $errorMessage) { - if (str_contains($exceptionMessage, $errorMessage)) { - // Here we reconnect through Redis Sentinel if we lost connection to the server or if another unavailability occurred. - // We may actually reconnect to the same, broken server. But after a failover occured, we should be ok. - // It may take a moment until the Sentinel returns the new master, so this may be triggered multiple times. - $this->reconnect(); + try { + $this->disconnect(); + } catch (RedisException $e) { + // Ignore when the creation of a new client gets an exception. + // If this exception isn't caught the retry will stop. + } catch (Throwable $e) { + if (! $this->retryContext->manager()->isNameResolutionException($e)) { + throw $e; + } + } - return; + // Here we reconnect through Redis Sentinel if we lost connection to the server or if another unavailability occurred. + // We may actually reconnect to the same, broken server. But after a failover occured, we should be ok. + // It may take a moment until the Sentinel returns the new master, so this may be triggered multiple times. + try { + $this->reconnect(); + } catch (RedisException $e) { + // Ignore when the creation of a new client gets an exception. + // If this exception isn't caught the retry will stop. + } catch (Throwable $e) { + if (! $this->retryContext->manager()->isNameResolutionException($e)) { + throw $e; } } } diff --git a/src/Connectors/PhpRedisSentinelConnector.php b/src/Connectors/PhpRedisSentinelConnector.php index 61d81d4..ae280a6 100644 --- a/src/Connectors/PhpRedisSentinelConnector.php +++ b/src/Connectors/PhpRedisSentinelConnector.php @@ -8,6 +8,8 @@ use Illuminate\Support\Arr; use Namoshek\Redis\Sentinel\Connections\PhpRedisSentinelConnection; use Namoshek\Redis\Sentinel\Exceptions\ConfigurationException; +use Namoshek\Redis\Sentinel\Services\RetryContext; +use Namoshek\Redis\Sentinel\Services\RetryManager; use Redis; use RedisException; use RedisSentinel; @@ -17,6 +19,27 @@ */ class PhpRedisSentinelConnector extends PhpRedisConnector { + /** + * The default of times the client attempts to retry a command when it fails + * to connect to a Redis instance behind Sentinel. + */ + public const DEFAULT_CONNECTOR_RETRY_ATTEMPTS = 20; + + /** + * The default time in milliseconds to wait before the client retries a failed + * command. + */ + public const DEFAULT_CONNECTOR_RETRY_DELAY = 1000; + + /** + * Setup the connector. + */ + public function __construct( + protected RetryManager $retryManager, + ) { + // + } + /** * {@inheritdoc} * @@ -32,7 +55,19 @@ public function connect(array $config, array $options): PhpRedisSentinelConnecti )); }; - return new PhpRedisSentinelConnection($connector(), $connector, $config); + $retryAttempts = is_numeric($config['connector_retry_attempts'] ?? null) + ? (int) $config['connector_retry_attempts'] + : self::DEFAULT_CONNECTOR_RETRY_ATTEMPTS; + + $retryDelay = is_numeric($config['connector_retry_delay'] ?? null) + ? (int) $config['connector_retry_delay'] + : self::DEFAULT_CONNECTOR_RETRY_DELAY; + + $retryContext = new RetryContext($this->retryManager, $retryAttempts, $retryDelay); + + $connection = $retryContext->retryOnFailure(fn () => $connector()); + + return new PhpRedisSentinelConnection($connection, $connector, $config, $retryContext); } /** diff --git a/src/Exceptions/ConfigurationException.php b/src/Exceptions/ConfigurationException.php index d3e4f6c..2e475e9 100644 --- a/src/Exceptions/ConfigurationException.php +++ b/src/Exceptions/ConfigurationException.php @@ -9,4 +9,5 @@ */ class ConfigurationException extends \RuntimeException { + // } diff --git a/src/Exceptions/RetryRedisException.php b/src/Exceptions/RetryRedisException.php new file mode 100644 index 0000000..1de9dd8 --- /dev/null +++ b/src/Exceptions/RetryRedisException.php @@ -0,0 +1,10 @@ +app->extend('redis', function (RedisManager $service) { - return $service->extend('phpredis-sentinel', fn () => new PhpRedisSentinelConnector); + $this->app->singleton(RetryManager::class, fn () => new RetryManager); + } + + /** + * Bootstrap any application services. + */ + public function boot(): void + { + $this->app->extend('redis', function (RedisManager $service, $app) { + $retryManager = $app->make(RetryManager::class); + + return $service->extend('phpredis-sentinel', fn () => new PhpRedisSentinelConnector($retryManager)); }); } } diff --git a/src/Services/RetryContext.php b/src/Services/RetryContext.php new file mode 100644 index 0000000..0a37b50 --- /dev/null +++ b/src/Services/RetryContext.php @@ -0,0 +1,60 @@ +retryAttempts; + $retryDelay ??= $this->retryDelay; + + return $this->retryManager->retryOnFailure( + $callback, + $retryAttempts, + $retryDelay, + $failureCallback, + ); + } + + /** + * Retrieve the RetryManager. + */ + public function manager(): RetryManager + { + return $this->retryManager; + } +} diff --git a/src/Services/RetryManager.php b/src/Services/RetryManager.php new file mode 100644 index 0000000..1cc42a6 --- /dev/null +++ b/src/Services/RetryManager.php @@ -0,0 +1,159 @@ +retry($callback); + } catch (Throwable $exception) { + // Check if we should retry this exception. + if (! $this->shouldRetry($exception)) { + throw $exception; + } + + // Wait before retry. + if ($retryAttempts !== 0) { + usleep($retryDelay * 1000); + } + + // Execute optional failure callback. + if ($failureCallback && is_callable($failureCallback)) { + call_user_func($failureCallback); + } + + $lastException = $exception; + } + } + + throw new RetryRedisException( + sprintf('Reached the (re)connect limit of %d attempts.', $retryAttempts), + 0, + $lastException, + ); + } + + /** + * Perform as retry when it is a second attempt. This makes testing easier. + */ + public function retry(callable $callback): mixed + { + return $callback(); + } + + /** + * We check if the Exception should be retried. This means checking for: + * - retryable redis exceptions + * - name resolution exceptions + */ + public function shouldRetry(Throwable $exception): bool + { + if ($exception instanceof RedisException && $this->shouldRetryRedisException($exception)) { + return true; + } + + if ($this->isNameResolutionException($exception)) { + return true; + } + + return false; + } + + /** + * Inspects the given exception and reconnects the client if the reported error indicates that the server + * went away or is in readonly mode, which may happen in case of a Redis Sentinel failover. + */ + public function shouldRetryRedisException(RedisException $exception): bool + { + // We convert the exception message to lower-case in order to perform case-insensitive comparison. + $exceptionMessage = strtolower($exception->getMessage()); + + // Because we also match only partial exception messages, we cannot use in_array() at this point. + foreach (self::ERROR_MESSAGES_INDICATING_UNAVAILABILITY as $errorMessage) { + if (str_contains($exceptionMessage, $errorMessage)) { + return true; + } + } + + return false; + } + + /** + * Check if the given exception is a name resolution exception. + */ + public function isNameResolutionException(Throwable $exception): bool + { + // We convert the exception message to lower-case in order to perform case-insensitive comparison. + $exceptionMessage = strtolower($exception->getMessage()); + + // Because we also match only partial exception messages, we cannot use in_array() at this point. + foreach (self::MESSAGES_INDICATING_NAME_RESOLUTION_ERRORS as $errorMessage) { + if (str_contains($exceptionMessage, $errorMessage)) { + return true; + } + } + + return false; + } +} diff --git a/start-redis-cluster.sh b/start-redis-cluster.sh index 68e6542..e116fbe 100644 --- a/start-redis-cluster.sh +++ b/start-redis-cluster.sh @@ -42,6 +42,7 @@ TRUNCATE_LOGS="${TRUNCATE_LOGS:-yes}" CLEANUP="${CLEANUP:-yes}" SUPERVISE="${SUPERVISE:-yes}" LOGGING="${LOGGING:-no}" +RESTART="${RESTART:-yes}" if [ -z "$REDIS_GROUP_1" ] && [ -z "$REDIS_GROUP_2" ] \ && [ -z "$REDIS_GROUP_3" ] && [ -z "$REDIS_GROUP_4" ] \ @@ -104,6 +105,7 @@ Options (from environment variables): CLEANUP Remove server-created files except logs (default: "yes"). SUPERVISE Stay in foreground (default: "yes"). LOGGING Output server logs in supervised mode (default: "no"). + RESTART Restart server instances when getting stopped (default: "yes"). With the default options, this tool creates the working directory in ./cluster and starts three Sentinels and two groups of three Redis servers. The script @@ -160,6 +162,7 @@ start_redis() { printf '' > "$WORKDIR/redis-$2.log" fi + # Optional master_port (only set on initial cluster start) master_port="$3" set -- --port "$2" \ @@ -169,9 +172,10 @@ start_redis() { --pidfile "redis-$2.pid" \ --logfile "redis-$2.log" \ --dbfilename "dump-$2.rdb" \ - --appendfilename "appendonly-$2.aof" + --appendfilename "appendonly-$2.aof" \ + --enable-debug-command yes - if [ "$2" -ne "$master_port" ]; then + if [ -n "$master_port" ] && [ "$2" -ne "$master_port" ]; then set -- "$@" --slaveof 127.0.0.1 "$master_port" fi @@ -335,6 +339,46 @@ wait_for_servers() { printf 'ERROR: No servers running.\n' >&2 } +monitor_and_restart() { + while true; do + for port in $Redis_Ports; do + # Check if Redis is alive using redis-cli + if ! redis-cli -p "$port" PING > /dev/null 2>&1; then + echo "Redis on port $port is DOWN. Attempting restart after the Sentinel failover has kicked in..." + + # Wait till the failover kicks in + sleep $(echo "scale=2; $DOWN_AFTER / 1000 + 2" | bc) + + restarted=0 + for i in 1 2 3 4 5 6 7 8 9; do + eval group=\$REDIS_GROUP_$i + [ -n "$group" ] || continue + + group_name=$(echo "$group" | cut -d' ' -f1) + port_spec=$(echo "$group" | cut -d' ' -f2) + ports=$(parse_ports "$port_spec") + + for p in $ports; do + if [ "$p" = "$port" ]; then + echo "Restarting Redis on port $port (group: $group_name)..." + rm -f "$WORKDIR/redis-$port.pid" + start_redis "$group_name" "$port" + restarted=1 + break 2 + fi + done + done + + if [ "$restarted" -ne 1 ]; then + echo "ERROR: Could not determine how to restart Redis on port $port" + fi + fi + done + + sleep 2 + done +} + verify_replication() { master_port="${1%% *}" replica_count="$(count_items "${1#* }")" @@ -489,7 +533,6 @@ unset Verify_Pids verify_quorum || terminate $? if is_true "$SUPERVISE"; then - printf 'Press Ctrl-C to stop...\n' trap 'printf "Shutting down...\n"; terminate 0' INT TERM server_pids="$(cat "$WORKDIR/"*.pid)" || exit $? @@ -498,5 +541,14 @@ if is_true "$SUPERVISE"; then start_logger fi - wait_for_servers $server_pids + if is_true "$RESTART"; then + echo 'Monitoring Redis servers for unexpected termination...' + printf 'Press Ctrl-C to stop...\n' + + monitor_and_restart + else + printf 'Press Ctrl-C to stop...\n' + + wait_for_servers $server_pids + fi fi diff --git a/tests/Connectors/PhpRedisSentinelConnectorTest.php b/tests/Connectors/PhpRedisSentinelConnectorTest.php index 0500e85..23bac80 100644 --- a/tests/Connectors/PhpRedisSentinelConnectorTest.php +++ b/tests/Connectors/PhpRedisSentinelConnectorTest.php @@ -5,42 +5,141 @@ namespace Namoshek\Redis\Sentinel\Tests\Connectors; use Illuminate\Redis\RedisManager; +use Mockery; +use Mockery\MockInterface; use Namoshek\Redis\Sentinel\Connections\PhpRedisSentinelConnection; use Namoshek\Redis\Sentinel\Connectors\PhpRedisSentinelConnector; +use Namoshek\Redis\Sentinel\Exceptions\RetryRedisException; +use Namoshek\Redis\Sentinel\Services\RetryManager; use Namoshek\Redis\Sentinel\Tests\TestCase; use Redis; use RedisException; +use ReflectionFunction; /** * Ensures that the {@see PhpRedisSentinelConnector} functions properly. */ class PhpRedisSentinelConnectorTest extends TestCase { - /** - * @throws RedisException - */ - public function test_connecting_to_redis_through_sentinel_without_password_works() + public function test_connecting_to_redis_through_sentinel_without_password_works(): void { - /** @var RedisManager $redisManager */ - $redisManager = $this->app->make('redis'); - - /** @var PhpRedisSentinelConnection $connection */ - $connection = $redisManager->connection('default'); + $connection = $this->getRedisConnection(); + $serverId = $this->extractServerId($connection->executeRaw(['INFO', 'server'])); self::assertTrue($connection->ping()); + self::assertSame($serverId, $this->extractServerId($connection->executeRaw(['INFO', 'server']))); } - /** - * @throws RedisException - */ - public function test_when_connection_goes_away_it_is_reestablished() + public function test_initial_connect_to_redis_with_a_failing_node(): void { - /** @var RedisManager $redisManager */ - $redisManager = $this->app->make('redis'); + /** @var \Mockery\MockInterface $spy */ + $spy = $this->spy(RetryManager::class, fn (MockInterface $mock) => $mock->makePartial()); + + // Retrieve the connection. + $connection = $this->getRedisConnection(); + + // Force the shutdown of a node, but avoid aborting the test case. + try { + $connection->client()->rawCommand('DEBUG', 'SEGFAULT'); + } catch (RedisException) { + // Ignored on purpose. + } + + // Recreate the connection. + $this->app->forgetInstance('redis'); + $connection = $this->getRedisConnection(); + + /** @var \Mockery\VerificationDirector $expectation */ + $expectation = $spy->shouldHaveReceived('retry'); + + // Make we are calling the retry from the connector and not the connection. + $expectation->with(Mockery::on(function ($arg) { + if (! is_callable($arg)) { + return false; + } + + $callableClass = (new ReflectionFunction($arg)) + ->getClosureScopeClass() + ?->getName(); + + return $callableClass === PhpRedisSentinelConnector::class; + })); + + // It should be retried at least 3 times as there is some time before + // sentinel marks the node as down and kicks in the failover. + $expectation->between(3, PhpRedisSentinelConnector::DEFAULT_CONNECTOR_RETRY_ATTEMPTS); + } + + public function test_retries_when_master_goes_away(): void + { + /** @var \Mockery\MockInterface $spy */ + $spy = $this->spy(RetryManager::class, fn (MockInterface $mock) => $mock->makePartial()); // Connect for the first time and remember the object hash of the connection. - /** @var PhpRedisSentinelConnection $connection */ - $connection = $redisManager->connection('default'); + $connection = $this->getRedisConnection(); + $serverId = $this->extractServerId($connection->executeRaw(['INFO', 'server'])); + + // Perform some random actions. + $connection->set('foo', 'bar'); + $connection->get('foo'); + $connection->del('foo'); + + $spy->shouldNotHaveReceived('retry'); + + // Force the shutdown of a node, but avoid aborting the test case. + try { + $connection->client()->rawCommand('DEBUG', 'SEGFAULT'); + } catch (RedisException) { + // Ignored on purpose. + } + + // Set and check if a new Redis instance is connected. + $connection->set('foo', 'bar'); + $connection->get('foo'); + $connection->del('foo'); + + /** @var \Mockery\VerificationDirector $expectation */ + $expectation = $spy->shouldHaveReceived('retry'); + + // It should be retried at least 3 times as there is some time before + // sentinel marks the node as down and kicks in the failover. + $expectation->between(3, PhpRedisSentinelConnector::DEFAULT_CONNECTOR_RETRY_ATTEMPTS); + + // Check the server id is updated. + self::assertNotSame($serverId, $this->extractServerId($connection->executeRaw(['INFO', 'server']))); + } + + public function test_no_retries_on_normal_exception(): void + { + $connection = $this->getRedisConnection(); + $clientId = spl_object_hash($connection->client()); + + // Perform some random actions. + $connection->ping(); + $connection->set('foo', 'bar'); + $connection->get('foo'); + $connection->del('foo'); + + // Force an exception, but avoid aborting the test case. + try { + $connection->transaction(fn (Redis $redis) => throw new RedisException('this exception should not be retried.')); + } catch (RedisException $e) { + self::assertNotInstanceOf(RetryRedisException::class, $e); + + // We need to discard the ->multi() in the transaction, otherwise other tests may fail. + $connection->client()->discard(); + } + + $connection->set('foo', 'bar'); + + // Connect a second time and compare the object hash of this and the old connection. + self::assertSame($clientId, spl_object_hash($connection->client())); + } + + public function test_when_connection_goes_away_it_is_reestablished(): void + { + // Connect for the first time and remember the object hash of the connection. + $connection = $this->getRedisConnection(); $clientId = spl_object_hash($connection->client()); // Perform some random actions. @@ -51,29 +150,22 @@ public function test_when_connection_goes_away_it_is_reestablished() // Force an exception, but avoid aborting the test case. try { - $connection->transaction(fn (Redis $redis) => throw new RedisException('went away')); + $connection->transaction( + fn (Redis $redis) => throw new RedisException('went away'), + retryAttempts: 0, + ); } catch (RedisException) { // Ignored on purpose. } // Connect a second time and compare the object hash of this and the old connection. - $connection = $redisManager->connection('default'); - $clientId2 = spl_object_hash($connection->client()); - - self::assertNotSame($clientId, $clientId2); + self::assertNotSame($clientId, spl_object_hash($connection->client())); } - /** - * @throws RedisException - */ - public function test_when_connection_becomes_readonly_it_is_reestablished() + public function test_when_connection_becomes_readonly_it_is_reestablished(): void { - /** @var RedisManager $redisManager */ - $redisManager = $this->app->make('redis'); - // Connect for the first time and remember the object hash of the connection. - /** @var PhpRedisSentinelConnection $connection */ - $connection = $redisManager->connection('default'); + $connection = $this->getRedisConnection(); $clientId = spl_object_hash($connection->client()); // Perform some random actions. @@ -84,29 +176,22 @@ public function test_when_connection_becomes_readonly_it_is_reestablished() // Force an exception, but avoid aborting the test case. try { - $connection->transaction(fn (Redis $redis) => throw new RedisException('READONLY')); + $connection->transaction( + fn (Redis $redis) => throw new RedisException('READONLY'), + retryAttempts: 0, + ); } catch (RedisException) { // Ignored on purpose. } // Connect a second time and compare the object hash of this and the old connection. - $connection = $redisManager->connection('default'); - $clientId2 = spl_object_hash($connection->client()); - - self::assertNotSame($clientId, $clientId2); + self::assertNotSame($clientId, spl_object_hash($connection->client())); } - /** - * @throws RedisException - */ - public function test_when_connection_becomes_readonly_it_is_reestablished2() + public function test_when_connection_becomes_readonly_it_is_reestablished2(): void { - /** @var RedisManager $redisManager */ - $redisManager = $this->app->make('redis'); - // Connect for the first time and remember the object hash of the connection. - /** @var PhpRedisSentinelConnection $connection */ - $connection = $redisManager->connection('default'); + $connection = $this->getRedisConnection(); $clientId = spl_object_hash($connection->client()); // Perform some random actions. @@ -117,15 +202,39 @@ public function test_when_connection_becomes_readonly_it_is_reestablished2() // Force an exception, but avoid aborting the test case. try { - $connection->transaction(fn (Redis $redis) => throw new RedisException("You can't write against a read only replica")); + $connection->transaction( + fn (Redis $redis) => throw new RedisException("You can't write against a read only replica"), + retryAttempts: 0, + ); } catch (RedisException) { // Ignored on purpose. } // Connect a second time and compare the object hash of this and the old connection. - $connection = $redisManager->connection('default'); - $clientId2 = spl_object_hash($connection->client()); + self::assertNotSame($clientId, spl_object_hash($connection->client())); + } + + /** + * Extract the run_id from the server info block. + */ + private function extractServerId(string $serverInfo): ?string + { + if (preg_match('/^run_id:(.+)$/m', $serverInfo, $matches)) { + return trim($matches[1]); + } + + return null; + } + + /** + * Retrieve the Redis connection. + */ + private function getRedisConnection(string $name = 'default'): PhpRedisSentinelConnection + { + /** @var RedisManager $redisManager */ + $redisManager = $this->app->make('redis'); - self::assertNotSame($clientId, $clientId2); + /** @var PhpRedisSentinelConnection $connection */ + return $redisManager->connection($name); } } diff --git a/tests/TestCase.php b/tests/TestCase.php index 09dca5b..e5dcf0a 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -15,7 +15,7 @@ abstract class TestCase extends \Orchestra\Testbench\TestCase /** * Returns a list of service providers required for the tests. * - * @param \Illuminate\Foundation\Application $app + * @param \Illuminate\Foundation\Application $app * @return string[] */ protected function getPackageProviders($app): array @@ -29,7 +29,7 @@ protected function getPackageProviders($app): array /** * Define environment setup. * - * @param \Illuminate\Foundation\Application $app + * @param \Illuminate\Foundation\Application $app * @return void */ protected function getEnvironmentSetUp($app) @@ -44,6 +44,9 @@ protected function getEnvironmentSetUp($app) 'sentinel_username' => env('REDIS_SENTINEL_USERNAME'), 'sentinel_password' => env('REDIS_SENTINEL_PASSWORD'), 'sentinel_service' => env('REDIS_SENTINEL_SERVICE', 'mymaster'), + + 'connector_retry_attempts' => 20, + 'connector_retry_delay' => 1000, ]); } }