From afc803549194e0a7a45cecdca76cb4f959f9b4a8 Mon Sep 17 00:00:00 2001 From: Vladyslav Vildanov <117659936+vladvildanov@users.noreply.github.com> Date: Tue, 19 May 2026 11:36:42 +0300 Subject: [PATCH] Revert GCRA command support (#1673) * Revert GCRA command support * Updated test image * Fixed flacky test --- CHANGELOG.md | 1 - src/ClientContextInterface.php | 1 - src/ClientInterface.php | 1 - src/Command/Redis/GCRA.php | 83 ------ tests/Predis/Command/Redis/GCRA_Test.php | 288 -------------------- tests/Predis/Command/Redis/HOTKEYS_Test.php | 14 +- 6 files changed, 12 insertions(+), 376 deletions(-) delete mode 100644 src/Command/Redis/GCRA.php delete mode 100644 tests/Predis/Command/Redis/GCRA_Test.php diff --git a/CHANGELOG.md b/CHANGELOG.md index ba4a6e55..86d0655e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,6 @@ ## Unreleased ### Added -- Added support for `GCRA` command (#1657) - Handle Redis Cluster `-READONLY` responses failover events (#1656) - Added FPHA argument for JSON.SET command (#1661) - Added new COUNT aggregator for Sorted Set commands (#1668) diff --git a/src/ClientContextInterface.php b/src/ClientContextInterface.php index cd1c2f2f..491fdacb 100644 --- a/src/ClientContextInterface.php +++ b/src/ClientContextInterface.php @@ -143,7 +143,6 @@ use Predis\Command\Redis\VADD; * @method $this ftsyndump(string $index) * @method $this ftsynupdate(string $index, string $synonymGroupId, ?SynUpdateArguments $arguments = null, string ...$terms) * @method $this fttagvals(string $index, string $fieldName) - * @method $this gcra(string $key, int $maxBurst, int $tokensPerPeriod, float $period, ?int $tokens = null) * @method $this get($key) * @method $this getbit($key, $offset) * @method $this getex(string $key, $modifier = '', $value = false) diff --git a/src/ClientInterface.php b/src/ClientInterface.php index 28b76dbd..0a1a7fd2 100644 --- a/src/ClientInterface.php +++ b/src/ClientInterface.php @@ -153,7 +153,6 @@ use Predis\Response\Status; * @method array ftsyndump(string $index) * @method Status ftsynupdate(string $index, string $synonymGroupId, ?SynUpdateArguments $arguments = null, string ...$terms) * @method array fttagvals(string $index, string $fieldName) - * @method array gcra(string $key, int $maxBurst, int $tokensPerPeriod, float $period, ?int $tokens = null) * @method string|null get(string $key) * @method int getbit(string $key, $offset) * @method int|null getex(string $key, $modifier = '', $value = false) diff --git a/src/Command/Redis/GCRA.php b/src/Command/Redis/GCRA.php deleted file mode 100644 index 82700b6d..00000000 --- a/src/Command/Redis/GCRA.php +++ /dev/null @@ -1,83 +0,0 @@ -responseSchema, $data); - } - - /** - * {@inheritdoc} - */ - public function parseResp3Response($data) - { - return $this->parseResponse($data); - } - - /** - * {@inheritdoc} - */ - public function prefixKeys($prefix) - { - $this->applyPrefixForFirstArgument($prefix); - } -} diff --git a/tests/Predis/Command/Redis/GCRA_Test.php b/tests/Predis/Command/Redis/GCRA_Test.php deleted file mode 100644 index 9a885589..00000000 --- a/tests/Predis/Command/Redis/GCRA_Test.php +++ /dev/null @@ -1,288 +0,0 @@ -getCommand(); - $command->setArguments($actualArguments); - - $this->assertSame($expectedArguments, $command->getArguments()); - } - - /** - * @group disconnected - */ - public function testParseResponse(): void - { - $raw = [0, 5, 4, -1, 10]; - $expected = [ - 'limited' => 0, - 'maxRequests' => 5, - 'availableRequests' => 4, - 'retryAfter' => -1, - 'fullBurstAfter' => 10, - ]; - $command = $this->getCommand(); - - $this->assertSame($expected, $command->parseResponse($raw)); - } - - /** - * @group disconnected - */ - public function testPrefixKeys(): void - { - /** @var PrefixableCommand $command */ - $command = $this->getCommand(); - $actualArguments = ['mykey', 4, 10, 1.0]; - $prefix = 'prefix:'; - $expectedArguments = ['prefix:mykey', 4, 10, 1.0]; - - $command->setArguments($actualArguments); - $command->prefixKeys($prefix); - - $this->assertSame($expectedArguments, $command->getArguments()); - } - - /** - * @group connected - * @requiresRedisVersion >= 8.7.2 - */ - public function testRequiredArgumentsDrainAllowanceByOne(): void - { - $redis = $this->getClient(); - - // max_burst=4, tokens_per_period=10, period=10 => 10 tokens/10s, emission_interval=1s - // With max_burst=4, max-req-num = 4+1 = 5 - $result = $redis->gcra('ratelimit:user1', 4, 10, 10); - $expectedResult = [ - 'limited' => 0, - 'maxRequests' => 5, - 'availableRequests' => 4, - 'retryAfter' => -1, - 'fullBurstAfter' => 1, - ]; - - // First request: not limited - $this->assertSame($expectedResult, $result); - - $result = $redis->gcra('ratelimit:user1', 4, 10, 10); - $expectedResult = [ - 'limited' => 0, - 'maxRequests' => 5, - 'availableRequests' => 3, - 'retryAfter' => -1, - 'fullBurstAfter' => 2, - ]; - - // Second request: allowance drained by 1 - $this->assertSame($expectedResult, $result); - } - - /** - * @group connected - * @requiresRedisVersion >= 8.7.2 - */ - public function testRequiredArgumentsDrainAllowanceByOneResp3(): void - { - $redis = $this->getResp3Client(); - - // max_burst=4, tokens_per_period=10, period=10 => 10 tokens/10s, emission_interval=1s - // With max_burst=4, max-req-num = 4+1 = 5 - $result = $redis->gcra('ratelimit:user1', 4, 10, 10); - $expectedResult = [ - 'limited' => 0, - 'maxRequests' => 5, - 'availableRequests' => 4, - 'retryAfter' => -1, - 'fullBurstAfter' => 1, - ]; - - // First request: not limited - $this->assertSame($expectedResult, $result); - - $result = $redis->gcra('ratelimit:user1', 4, 10, 10); - $expectedResult = [ - 'limited' => 0, - 'maxRequests' => 5, - 'availableRequests' => 3, - 'retryAfter' => -1, - 'fullBurstAfter' => 2, - ]; - - // Second request: allowance drained by 1 - $this->assertSame($expectedResult, $result); - } - - /** - * @group connected - * @requiresRedisVersion >= 8.7.2 - */ - public function testTokensPerPeriodAndPeriodCombination(): void - { - $redis = $this->getClient(); - - // max_burst=0, tokens_per_period=1, period=60 - // Only 1 token allowed per 60 seconds, no burst - // max-req-num = 0+1 = 1 - $result1 = $redis->gcra('ratelimit:strict', 0, 1, 60); - $expectedResult1 = [ - 'limited' => 0, - 'maxRequests' => 1, - 'availableRequests' => 0, - 'retryAfter' => -1, - 'fullBurstAfter' => 60, - ]; - - // First request: not limited - $this->assertSame($expectedResult1, $result1); - - // Second request is limited (no burst, only 1/60s allowed) - $result2 = $redis->gcra('ratelimit:strict', 0, 1, 60); - $this->assertSame(1, $result2['limited']); - $this->assertSame(1, $result2['maxRequests']); - $this->assertSame(0, $result2['availableRequests']); - $this->assertGreaterThan(0, $result2['retryAfter']); // retry-after > 0 (must wait) - } - - /** - * @group connected - * @requiresRedisVersion >= 8.7.2 - */ - public function testTokensDrainsAllowanceByCount(): void - { - $redis = $this->getClient(); - - // max_burst=9, tokens_per_period=10, period=10 => max-req-num = 10 - // Use TOKENS=3 to drain 3 at once - $result1 = $redis->gcra('ratelimit:bulk', 9, 10, 10, 3); - $expectedResult1 = [ - 'limited' => 0, - 'maxRequests' => 10, - 'availableRequests' => 7, - 'retryAfter' => -1, - 'fullBurstAfter' => 3, - ]; - - // First request: not limited, 7 available (10 - 3) - $this->assertSame($expectedResult1, $result1); - - // Another request with TOKENS=3 - $result2 = $redis->gcra('ratelimit:bulk', 9, 10, 10, 3); - $expectedResult2 = [ - 'limited' => 0, - 'maxRequests' => 10, - 'availableRequests' => 4, - 'retryAfter' => -1, - 'fullBurstAfter' => 6, - ]; - - // Second request: not limited, 4 available (7 - 3) - $this->assertSame($expectedResult2, $result2); - - // Another request with TOKENS=3 - $result3 = $redis->gcra('ratelimit:bulk', 9, 10, 10, 3); - $expectedResult3 = [ - 'limited' => 0, - 'maxRequests' => 10, - 'availableRequests' => 1, - 'retryAfter' => -1, - 'fullBurstAfter' => 9, - ]; - - // Third request: not limited, 1 available (4 - 3) - $this->assertSame($expectedResult3, $result3); - - // Next request with TOKENS=3 should be limited (only 1 left) - $result4 = $redis->gcra('ratelimit:bulk', 9, 10, 10, 3); - $this->assertSame(1, $result4['limited']); - $this->assertGreaterThan(0, $result4['retryAfter']); // must wait - } - - /** - * @group connected - * @requiresRedisVersion >= 8.7.2 - */ - public function testRetryAfterBehaviorAllowsRequestsAfterWaiting(): void - { - $redis = $this->getClient(); - - // max_burst=2, tokens_per_period=3, period=3 => 3 tokens/3s, emission_interval=1s - // max-req-num = 2+1 = 3 - // Drain all allowance with 3 requests - $redis->gcra('ratelimit:retry', 2, 3, 3); - $redis->gcra('ratelimit:retry', 2, 3, 3); - $redis->gcra('ratelimit:retry', 2, 3, 3); - - // Next request should be limited - $limitedResult = $redis->gcra('ratelimit:retry', 2, 3, 3); - $this->assertSame(1, $limitedResult['limited']); - $this->assertGreaterThan(0, $limitedResult['retryAfter']); - - // Sleep for retryAfter seconds - $retryAfterSeconds = $limitedResult['retryAfter']; - sleep($retryAfterSeconds); - - // After waiting, request should no longer be limited - $afterWaitResult = $redis->gcra('ratelimit:retry', 2, 3, 3); - $this->assertSame(0, $afterWaitResult['limited']); - $this->assertSame(-1, $afterWaitResult['retryAfter']); - } - - public function argumentsProvider(): array - { - return [ - 'with required arguments only' => [ - ['mykey', 4, 10, 1.0], - ['mykey', 4, 10, 1.0], - ], - 'with TOKENS' => [ - ['mykey', 4, 10, 1.0, 5], - ['mykey', 4, 10, 1.0, 'TOKENS', 5], - ], - 'with null TOKENS (omitted)' => [ - ['mykey', 4, 10, 1.0, null], - ['mykey', 4, 10, 1.0], - ], - ]; - } -} diff --git a/tests/Predis/Command/Redis/HOTKEYS_Test.php b/tests/Predis/Command/Redis/HOTKEYS_Test.php index ae9216d8..ae49a1d6 100644 --- a/tests/Predis/Command/Redis/HOTKEYS_Test.php +++ b/tests/Predis/Command/Redis/HOTKEYS_Test.php @@ -98,7 +98,12 @@ class HOTKEYS_Test extends PredisCommandTestCase $this->assertEquals('OK', $redis->hotkeys->stop()); $hotkeysInfo = $redis->hotkeys->get()[0]; - $this->assertCount(24, $hotkeysInfo['by-cpu-time-us']); + $tracked = count($hotkeysInfo['by-cpu-time-us']); + // COUNT 12 is an upper bound on tracked keys (24 = 12 key/score pairs); + // sampling may return fewer entries, but always in even-sized key/score pairs. + $this->assertNotEmpty($hotkeysInfo['by-cpu-time-us']); + $this->assertLessThanOrEqual(24, $tracked); + $this->assertSame(0, $tracked % 2); // Starts hotkeys tracking (with DURATION, SAMPLE) $this->assertEquals( @@ -160,7 +165,12 @@ class HOTKEYS_Test extends PredisCommandTestCase $this->assertEquals('OK', $redis->hotkeys->stop()); $hotkeysInfo = $redis->hotkeys->get()[0]; - $this->assertCount(24, $hotkeysInfo['by-cpu-time-us']); + $tracked = count($hotkeysInfo['by-cpu-time-us']); + // COUNT 12 is an upper bound on tracked keys (24 = 12 key/score pairs); + // sampling may return fewer entries, but always in even-sized key/score pairs. + $this->assertNotEmpty($hotkeysInfo['by-cpu-time-us']); + $this->assertLessThanOrEqual(24, $tracked); + $this->assertSame(0, $tracked % 2); // Starts hotkeys tracking (with DURATION, SAMPLE) $this->assertEquals(