search

mediocregopher / radix.v2

Redis client for Go
http://godoc.org/github.com/mediocregopher/radix.v2
MIT License
433 stars 92 forks source link

optimize the pool package by limiting connections to a fixed number #77

Closed panjf2000 closed 6 years ago

panjf2000 commented 6 years ago

Hi @mediocregopher ,i have found that there may be some problems in the pool package of radix.v2 during my use of this package.

Here is the thing,i once ran a program that would init a redis pool with limited size 100,and the program would generate more than 1000 goroutines to get redis connections and unexpectedly, the pool package didn't limit the number of connections to the fixed size which was assigned when initializing the pool instance,this case cause a shutdown of my redis server because it received too many connecting requests.

Then i checked out the source codes of radix.v2 and i found that the pool package would return an available redis client,if there are none available it will create a new one,this kind of implementation may get some potential problems,like i said above,it may create too many connections with redis server at the same time and a shutdown of redis may occured.Besides,creating all connections(pool size) when init a redis pool may not be a very good way to init a redis pool,it may appear wasteful.

In summary,i am trying to optimize this pool package,make it possible that pool instance always generate the fixed number of connections which (i think) will prevent redis server from crashing.

please check my pr out,and I'm looking forward to receiving your reply,thanks!

mediocregopher commented 6 years ago

Hi @panjf2000 I'm so sorry it's taken me so long to reply to your PR, I've been traveling and haven't had internet really.

I'm afraid that at this point pool's behavior is pretty set in stone. The behavior you describe is intentional, as with a properly tuned pool size it gracefully and simply handles request spikes. The documentation for pool also describes the behavior, so changing it would constitute a backwards incompatible change, which I can't do.

What you can do, though, is check out the radix.v3 package, which I designed with the goal that each component is swappable by a custom implementation. So you could use your custom Pool in it and it would correctly work with all other parts of the package.

I'm sorry again to shut you down like this, but I hope the v3 package is more helpful to you. Thanks!

panjf2000 commented 6 years ago

Hey @mediocregopher ,although you said it is intentional,but i still think there could be a potential problem in programs using radix.v2 pool package.It can create way more than the size of redis pool (say you create 10000 connections and the pool size you assigned is just 100) and it may shut the redis server down,so please think about this issue even if you solve it by another way instead of mine eventually,thanks.