yiisoft / yii2-redis

Yii 2 Redis extension.
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
452 stars 183 forks source link

Redis cluster refactoring #178

Closed hofrob closed 4 years ago

hofrob commented 5 years ago
Q A
Is bugfix? yes
New feature? yes
Breaks BC? no
Tests pass? yes
Fixed issues #147 #143 #142 #136 #135 #134 #66
hofrob commented 5 years ago

Connections to redis are now pooled, to enable redirects without opening and closing connections during a request.

If redis is running in cluster mode, it is possible to supply a hashtag for MGET and MSET operations. If there are now hashtags, it falls back to GET and SET.

To test this locally I used a ready-to-use docker cluster container:

docker run --network=some_local_network --network-alias=redis-cluster grokzen/redis-cluster:redis_version

Put a PHP container with Yii in the same network (some_local_network), select a redis version (redis_version), and try the example.

hofrob commented 5 years ago

I should implement some tests for this change. Probably by using the aforementioned docker container. But to be honest, I have no idea where to start. If someone could point me in the right direction, I'd be happy to get started.

hofrob commented 5 years ago

@cebe

This PR would fix the problem you mentioned here: https://github.com/yiisoft/yii2-redis/pull/147#discussion_r188220246

cebe commented 5 years ago

Thanks for your work on this. I am planning to use a redis cluster in the near future and will review this PR when I get to that.

hofrob commented 5 years ago

I can give you an update from what we're using now. Turns out this solution works, but somehow it was the slowest of all the solutions we tried (our production env runs on AWS EB with extensive use of redis ElastiCache). Our requests were just about 10-15% slower than before. But it was noticeable.

And yes, weirdly enough opening and closing the connections after redirects was faster. But I did not do any more research on that because....

We then tried the redis pecl extension. It supports cluster mode completely and we're now at about the same request duration we were before.

To be able to use the pecl extension with \yii\caching\Cache I just had to write a very simple wrapper (about 80 lines). The only thing left for us to completely switch to this extension is rewriting the yii2-queue. It's unfortunate that the method names are written differently.

So I've not yet tackled this. I think it would be great if you could switch from yii2-redis to the pecl extension without having to change something in yii2-queue. If you or the yii devs in general are interested in implementing this, maybe I can help.

samdark commented 5 years ago

That is interesting, especially for 3.0.

samdark commented 5 years ago

@cebe did you have a chance to use Redis cluster?

pacifier17 commented 4 years ago

@samdark do you think we can get this merged soon? We are looking to use cluster Redis for our use case. We did test out the fork and it works with cluster redis config.

samdark commented 4 years ago

Overall it looks OK. Some minor adjustments and changelog line are needed.

samdark commented 4 years ago

Unit tests would be awesome as well.

samdark commented 4 years ago

@hofrob would you please apply suggestions?

hofrob commented 4 years ago

changelog line are needed.

Since this is a really old branch, is there a preferred way how to update it? I'd just rebase it (and then add the changelog).

samdark commented 4 years ago

Either merging master into it or rebasing.

hofrob commented 4 years ago

Unit tests would be awesome as well.

After a quick look at the sources I'm not really sure how to configure/start/use a redis cluster for tests.

samdark commented 4 years ago

Merged. Thank you very much!

pacifier17 commented 4 years ago

Thanks a lot @samdark and @hofrob for merging these changes in so quickly. I really appreciate it.

samdark commented 4 years ago

Not so quickly. Look how much time it took me to get to it and review/merge it :) @hofrob sorry for that btw.