Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#66 distribute the read operation on Redis Cache #132

Merged
merged 9 commits into from
Jan 4, 2018
83 changes: 81 additions & 2 deletions Cache.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,27 @@
* ]
* ~~~
*
* If you ever need to make the cache read from Replica.
* e.g. AWS ElasticCache Redis, default all read go to master node
*
* ~~~
* [
* 'components' => [
* 'cache' => [
* 'class' => 'yii\redis\Cache',
* 'enableReplicas' => true,
* 'replicas' => [
* //replica redis connection, (default class will be yii\redis\Connection if not provided)
* //you can optionally put in master as hostname as well, as all GET operation will use replicas
* ['hostname' => 'redis-master.xyz.ng.0001.apse1.cache.amazonaws.com'],
* ['hostname' => 'redis-slave-002.xyz.0001.apse1.cache.amazonaws.com'],
* ['hostname' => 'redis-slave-003.xyz.0001.apse1.cache.amazonaws.com'],
* ],
* ],
* ],
* ]
* ~~~
*
* @author Carsten Brandt <mail@cebe.cc>
* @since 2.0
*/
Expand All @@ -67,6 +88,21 @@ class Cache extends \yii\caching\Cache
*/
public $redis = 'redis';

/**
* @var bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing @since tag and description.

*/
public $enableReplicas = false;

/**
* @var array
*/
public $replicas = [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing @since tag and description.


/**
* @var Connection
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing description.

*/
private $_replica;


/**
* Initializes the redis Cache component.
Expand Down Expand Up @@ -99,15 +135,15 @@ public function exists($key)
*/
protected function getValue($key)
{
return $this->redis->executeCommand('GET', [$key]);
return $this->getReplica()->executeCommand('GET', [$key]);
}

/**
* @inheritdoc
*/
protected function getValues($keys)
{
$response = $this->redis->executeCommand('MGET', $keys);
$response = $this->getReplica()->executeCommand('MGET', $keys);
$result = [];
$i = 0;
foreach ($keys as $key) {
Expand Down Expand Up @@ -195,4 +231,47 @@ protected function flushValues()
{
return $this->redis->executeCommand('FLUSHDB');
}

/**
* @inheritdoc
* @return Connection
*/
protected function getReplica()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a new method. Thus, @inheritdoc doesn't make sense. Missing @since tag and description.

{
if ($this->enableReplicas === false) {
return $this->redis;
}

if (isset($this->_replica)) {
return $this->_replica;
}

if (empty($this->replicas)) {
return $this->_replica = $this->redis;
}

if (count($this->replicas) >= 2) {
shuffle($this->replicas);
}
$config = array_shift($this->replicas);
if (is_array($config)) {
if (!isset($config['class'])) {
$config['class'] = 'yii\redis\Connection';
}

//--- if hostname same, no need re-open connection
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// If hostname is the same, there is no need re-open connection

if (isset($config['hostname']) && $config['hostname'] === $this->redis->hostname) {
return $this->_replica = $this->redis;
}

/** @var Connection $redis */
$redis = Yii::createObject($config);
if ($redis instanceof Connection) {
return $this->_replica = $redis;
}
}

Yii::trace('Fall back to redis master');
return $this->_replica = $this->redis;
}
}
75 changes: 75 additions & 0 deletions tests/RedisCacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ protected function getCacheInstance()
return $this->_cacheInstance;
}

protected function resetCacheInstance()
{
$this->getCacheInstance()->flush();
$this->_cacheInstance = null;
}

public function testExpireMilliseconds()
{
$cache = $this->getCacheInstance();
Expand Down Expand Up @@ -120,4 +126,73 @@ public function testMultiByteGetAndSet()
$cache->set($key, $data);
$this->assertSame($cache->get($key), $data);
}

public function testReplica()
{
$this->resetCacheInstance();

$cache = $this->getCacheInstance();
$cache->enableReplicas = true;

$key = 'replica-1';
$value = 'replica';

//No Replica listed
$this->assertFalse($cache->get($key));
$cache->set($key, $value);
$this->assertSame($cache->get($key), $value);

$cache->replicas = [
['hostname' => 'localhost'],
];
$this->assertSame($cache->get($key), $value);

//One Replica listed
$this->resetCacheInstance();
$cache = $this->getCacheInstance();
$cache->enableReplicas = true;
$cache->replicas = [
['hostname' => 'localhost'],
];
$this->assertFalse($cache->get($key));
$cache->set($key, $value);
$this->assertSame($cache->get($key), $value);

//Multiple Replicas listed
$this->resetCacheInstance();
$cache = $this->getCacheInstance();
$cache->enableReplicas = true;

$cache->replicas = [
['hostname' => '127.0.0.1'],
['hostname' => '127.0.0.1'],
];
$this->assertFalse($cache->get($key));
$cache->set($key, $value);
$this->assertSame($cache->get($key), $value);

//invalid config
$this->resetCacheInstance();
$cache = $this->getCacheInstance();
$cache->enableReplicas = true;

$cache->replicas = [
['class' => 'yii\db\Connection'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should expect an exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial intention of not using Instance::ensure() is not to break the application if replica is not properly configured, will update the test to expect exception

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exception expect to be handled by UnitTest in Instance Class on Instance::ensure() function, therefore I removed the test for invalid config.

];
$this->assertFalse($cache->get($key));
$cache->set($key, $value);
$this->assertSame($cache->get($key), $value);

//invalid config
$this->resetCacheInstance();
$cache = $this->getCacheInstance();
$cache->enableReplicas = true;

$cache->replicas = ['redis'];
$this->assertFalse($cache->get($key));
$cache->set($key, $value);
$this->assertSame($cache->get($key), $value);

$this->resetCacheInstance();
}
}