Support for multiple Redis servers

[aw]

Hi,

I was trying to modify the code to make this work but there's some magic CodeIgniter 🌈 stuff going on which I don't fully understand.

TLDR; I got it working, but it's a total hack.

Essentially I'm trying to configure two separate Redis servers - one for read 📖 requests, one for write 📝 requests.

I pushed a new branch which adds this functionality but pretty much breaks previous functionality, as seen here: https://github.com/aw/codeigniter-redis/commit/6fc6dd1ef732ffa4671e39b3438327e351552993

I'm also unsure how this affects Sparks since I don't use them.

Perhaps you can have a look at the changes I made, and suggest some improvements to enable backwards compatibility with the config file / usage (ie: not requiring $this->redis->connect() if a scope isn't defined) ?

Let me know what's possible when you have a chance.

Thanks!

[joelcox]

CodeIgniter's singleton oriented approach definitely is the culprit here.

When looking at the way CI handles multiple database connections (http://ellislab.com/codeigniter/user-guide/database/connecting.html) you see they added an extra parameter to return the object. Sadly this is not an option when loading libraries.

Another possibility would be to use the second and third argument in the Load::library() method:

 $this->load->library('redis', array('connection_group' => 'slave'), 'redis_slave');
 // Instance of the class using this connection group would now be available like this
 $this->redis_slave->ping();

As for backwards compatibility, ensuring compatibility with the current config structure would be preferable. I'd also suggest that you always connect to the master, without the need of running connect(). If this is about performance, we can always decided to move it the the encode_request() method and only connect on demand.

[aw]

Joel your suggestions were spot-on! 🎉

I wasn't aware of the ability to assign a library to a different object name. That's so much better!!

I added this functionality in a new branch, you'll notice it doesn't require many changes and maintains backwards compatibility.

Tell me what you think.

[joelcox]

Looking good, but I'm not really a fan having multiple config files. What's the use case for this? Using a multidimensional array for the different connection groups would be better IMHO, just like CI does for databases.

$config['default']['redis_host'] = 'localhost';
$config['default']['redis_port'] = '6379';
$config['default']['redis_password'] = '';

$config['slave']['redis_host'] = 'otherhost';
$config['slave']['redis_port'] = '6379';
$config['slave']['redis_password'] = '';

If the config array isn't multidimensional (i.e. there is no 'default' key) it would assume you're using the old configuration style.

[aw]

hmmm.... yeah the problem is PHP doesn't allow using an array as a key inside an array. Once again I'm not sure what kind of magic CI is using for databases, but it's the only place i've seen that type of config in PHP.

Perhaps this: ??

$config['redis_host'] = 'localhost';
$config['redis_port'] = '6379';
$config['redis_password'] = '';

$config['redis_host_slave'] = 'otherhost';
$config['redis_port_slave'] = '6379';
$config['redis_password_slave'] = '';

[joelcox]

I pulled in your branch and made some changes for the solution I was thinking of. Let me know what you think. https://github.com/joelcox/codeigniter-redis/tree/multiple-servers. I moved 'redis' in front of the connection group to keep config items from colliding. This solutions allows for the following config styles:

$config['redis_default']['host'] = 'localhost';
$config['redis_default']['password'] = '1234';
$config['redis_default']['port'] = 'topsecret';

$config['redis_slave']['host'] = 'otherhost';
$config['redis_slave']['password'] = '1234';
$config['redis_slave']['port'] = 'topsecret2';

and the old style:

$config['redis_host'] = 'localhost';
$config['redis_password'] = '1234';
$config['redis_port'] = 'topsecret';

I've yet to thoroughly test the code and write tests for the branch mentioned above, but wanted to see what you think about this first.

[joelcox]

Hey Alex, care to pitch in on the changes I made to your code in the branch I mentioned above?

[aw]

Oh sorry for the delay! I'll have a look right now.

[aw]

Ah so far it appears AUTH is broken again ;)

From here: 7aac0bf

[aw]

OK I just tested the library with various Redis server configs, works perfectly after fixing the AUTH issue!

[joelcox]

Great. I'll probably add some test cases this weekend and merge it into develop. Thanks for your contribution.

On Mar 28, 2013, at 3:04 PM, Alex Williams notifications@github.com wrote:

OK I just tested the library with various Redis server configs, works perfectly after fixing the AUTH issue!

—
Reply to this email directly or view it on GitHub.

[aw]

👍 Thanks for this library! ✨

[joelcox]

Hey Alex, I finally got around to writing some proper tests and merged the changes into develop. I added your name to the thank you section.

297fda6