mediocregopher / radix.v2

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

IOErr occured when client idle more than 300 seconds #46

Closed liuaifu closed 7 years ago

liuaifu commented 7 years ago

I use pool. I found if a client idle more than 300 seconds(redis server default timeout is 300s), the Cmd() will fail. Someone suggest use the following code to avoid:

go func() {
    for {
        p.Cmd("PING")
        time.Sleep(1 * time.Second)
    }
}()

But can you move it to library? Because not everyone is observed this problem.

mediocregopher commented 7 years ago

At the moment that little snippet is the best approach, unfortunately. My reasoning for this is that I'm trying to keep the pool package as simple as possible. Pools are a tricky thing where everyone wants something slightly different depending on their use-case, and it's very difficult to actually cover all use-cases with a single implementation. For example in this particular case, some users will have long running programs that need pinging, but others will have short-lived, performance critical ones where pinging would just slow things down. There's other concerns too. What if someone wants to write a log whenever the ping fails? What if they want to make a new connection vs just dropping the old? What if they want to do something crazy like re-make the entire pool when one connection fails?

The point is by keeping the implementation super simple I can accommodate all use-cases, although one some have to do more work than others. That said, I am working on a v3 of radix that will accommodate this particular use-case a bit better (since it does come up rather often). I'll link to it on the README of this project when it's ready for beta, should happen in the next month or so, so keep an eye out if you're interested in helping me test it.

Thanks for submitting the issue and PR, I'm sorry to shoot you down in both cases, but I appreciate your interest and you using the package. Let me know if there's anything else you have questions about! :)

zhaoyuxi commented 7 years ago

@mediocregopher I have also found this issue. Add some extra my case. 1 Dynamic connections pool according to the load. My service probably has more than 10 redis master-slave nodes. Each client will connect to all nodes and each pool will have 40~120 connections.So i want much more connections when there is a high peak and small connections with a low peak. 2 The PING is just sent for the idle connections. 3 I also expect the pool can do some performance improvement. It relies on channel. Channel is low performance compared to the mutex. My case has probably 1000 connections to all redis server. 4 Pool is FIFO. But probably i want the LIFO(return the connection to the header of pool and the getting is from the head). Currently, if it's LIFO, it has a higier possiblity that the connection is unavaiable. Because the firewall probably cut the connection off if the connection is idle for 10 minutes. Redis server and client can't feel it otherwise sending message.