10000 bug #51578 [Cache] always select database for persistent redis connec… · symfony/symfony@bdb4916 · GitHub
[go: up one dir, main page]
Skip to content

Commit bdb4916

Browse files
uncaughtuncaught
committed
bug #51578 [Cache] always select database for persistent redis connections
1 parent 184597d commit bdb4916

File tree

2 files changed

+55
-1
lines changed

2 files changed

+55
-1
lines changed

‎src/Symfony/Component/Cache/Tests/Traits/RedisTraitTest.php

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,55 @@ public static function provideCreateConnection(): array
7474
],
7575
];
7676
}
77+
78+
/**
79+
* Due to a bug in phpredis, the persistent connection will keep its last selected database. So when re-using
80+
* a persistent connection, the database has to be re-selected, too.
81+
* @see https://github.com/phpredis/phpredis/issues/1920
82+
* @see https://github.com/symfony/symfony/issues/51578
83+
*/
84+
public function testPconnectSelectsCorrectDatabase() {
85+
if (!class_exists('Redis')) {
86+
throw new SkippedTestSuiteError('The "Redis" class is required.');
87+
}
88+
if (!getenv('REDIS_HOST')) {
89+
throw new SkippedTestSuiteError('REDIS_HOST env var is not defined.');
90+
}
91+
if (!ini_get('redis.pconnect.pooling_enabled')) {
92+
throw new SkippedTestSuiteError('The bug only occurs when pooling is enabled.');
93+
}
94+
95+
//Limit the connection pool size to 1:
96+
$prevPoolSize = ini_set('redis.pconnect.connection_limit', 1);
97+
if (false === $prevPoolSize) {
98+
throw new SkippedTestSuiteError('Unable to set pool size');
99+
}
100+
101+
try {
102+
$mock = self::getObjectForTrait(RedisTrait::class);
103+
104+
$dsn = 'redis://' . getenv('REDIS_HOST');
105+
106+
$cacheKey = 'testPconnectSelectsCorrectDatabase';
107+
$cacheValueOnDb1 = 'I should only be on database 1';
108+
109+
//First connect to database 1 and set a value there so we can identify this database:
110+
$db1 = $mock::createConnection($dsn, ['dbindex' => 1, 'persistent' => 1]);
111+
self::assertInstanceOf('Redis', $db1);
112+
self::assertSame(1, $db1->getDbNum());
113+
$db1->set($cacheKey, $cacheValueOnDb1);
114+
self::assertSame($cacheValueOnDb1, $db1->get($cacheKey));
115+
116+
//Unset the connection - do not use `close()` or we will lose the persistent connection:
117+
unset($db1);
118+
119+
//Now connect to database 0 and see that we do not actually ended up on database 1 by checking the value:
120+
$db0 = $mock::createConnection($dsn, ['dbindex' => 0, 'persistent' => 1]);
121+
self::assertInstanceOf('Redis', $db0);
122+
self::assertSame(0, $db0->getDbNum()); //Redis is lying here! We could actually be on any database!
123+
self::assertNotSame($cacheValueOnDb1, $db0->get($cacheKey)); //This value should not exist if we are actually on db 0
124+
} finally {
125+
ini_set('redis.pconnect.connection_limit', $prevPoolSize); //Restore previous pool size
126+
}
127+
}
77128
}

‎src/Symfony/Component/Cache/Traits/RedisTrait.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,10 @@ public static function createConnection(string $dsn, array $options = [])
283283
}
284284

285285
if ((null !== $auth && !$redis->auth($auth))
286-
|| ($params['dbindex'] && !$redis->select($params['dbindex']))
286+
//Due to a bug in phpredis we must always select the dbindex if persistent pooling is enabled
287+
// @see https://github.com/phpredis/phpredis/issues/1920
288+
// @see https://github.com/symfony/symfony/issues/51578
289+
|| (($params['dbindex'] || ($connect === 'pconnect' && ini_get('redis.pconnect.pooling_enabled') !== '0')) && !$redis->select($params['dbindex']))
287290
) {
288291
$e = preg_replace('/^ERR /', '', $redis->getLastError());
289292
throw new InvalidArgumentException('Redis connection failed: '.$e.'.');

0 commit comments

Comments
 (0)
0