search

mediocregopher / radix.v2

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

Count number of open connections in Pool #18

Closed siebertm closed 8 years ago

siebertm commented 8 years ago

Due to a bug in our code, I accidentally opened 1000s of connections to redis. In order to avoid this in the future (and to be able to track the number of connections), I want to be able to know how many connections are in the pool right now.

As I see, this is just a matter of exposing len(pool), right? If you'd be willing to accept a MR (and my approach is correct, since I'm kind of new to golang), I'd gladly do it.

Just need a "go" from you ;)

mediocregopher commented 8 years ago

Hi! Thanks for submitting the issue, I guess there's some points to clarify first:

1) Yes, exposing len(pool) would allow someone to see the number of connections in the pool. kind of. In reality, that would give the number of connections that are available to be used at any given moment, it does not take into account connections which have been gotten but not yet put back. So a more accurate description of len(pool) would be "Number of available connections in the pool".

2) The number of available connections in the pool will always be less than the size parameter passed in when the pool is created. In the case you're describing, where you accidentally created thousands of connections, it wouldn't actually help you to know the number of available connections, since that number wouldn't grow with all the new connections you're making.

While being able to see the number of available connections might be useful, I don't think it would help in this case so I'd like to hold off until a better use-case presents itself. If I could ask, what was the exact bug which caused you to create all those connections?

siebertm commented 8 years ago

Ok, you're right, under these circumstances, len(pool) instrumentation wouldn't have seen the bug:

for {
  redis := pool.Get()
  defer pool.Put(redis)

  // do stuff and sleep
}

Rather dumb bug cause by developer dumbness ;-)

mediocregopher commented 8 years ago

lol, yeah that would be a really tough one to catch. Depending on what // do stuff and sleep entails, you might be able to use the Cmd method, which does the Get and Put for you. Unless you specifically need pipelines or transactions it's often the best way to go, cause it prevents small bugs like this :P

I'm gonna go ahead and close this out, feel free to re-open it if you have any more questions :)