search

Grokzen / redis-py-cluster

Python cluster client for the official redis cluster. Redis 3.0+.
https://redis-py-cluster.readthedocs.io/
MIT License
1.1k stars 316 forks source link

allow read from replicas in pipeline #402

Open duke-cliff opened 3 years ago

duke-cliff commented 3 years ago

Can you merge this and make a 2.1.1 version first? Using mget is not acceptable as it's adding too much cost to us(3x as the expected cluster size).

Grokzen commented 3 years ago

@duke-cliff Yes i can make a 2.1.1 releaes after i merge your two PR:s

Grokzen commented 3 years ago

Also even if all the current tests actually works, do you know know that the current pipeline algorithm actually works out in basically all pipeline cases even with read-only mode? I have no tests for this and this PR do not include any extension to the tests or at least any manual steps or a validation script to validate that it actually works out as we expect and that it handles a lot of the error scenarios and failover scenarios that actually can happen that the algorithm have to take care of.

duke-cliff commented 3 years ago

@Grokzen Yup, it works. The redirect logic is working the same as before, the only difference is when it could get a connection from the slaves for a slot. The rest logic is the same to how you deal with exceptions/redirects(moved/ask) on masters:

image You can see our CPU was dropped from 16% to 5-6% because of this change.

duke-cliff commented 3 years ago

I will have more optimization for this PR coming. testing locally first.

jerroydmoore commented 3 years ago

When I merged master into this branch, the diff went away. I think this PR might be safe to close.