mediocregopher / radix.v2

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

How can I ensure the redis.Client in the pool.Pool connected? #21

Closed Akagi201 closed 8 years ago

Akagi201 commented 8 years ago

I wrote a redis http interface using the redis and pool sub-package.

In the main function

p, err := pool.New("tcp", u.Host, concurrency) // concurrency = 3

In the function called in the main function

func redisDo(p *pool.Pool, cmd string, args ...interface{}) (reply *redis.Resp, err error) {
    conn, err := p.Get()
    errHndlr(err)
    defer p.Put(conn)

    // do the request.
    reply = conn.Cmd(cmd, args...)
    if err = reply.Err; err != nil {
        if err != io.EOF {
            Fatal.Println("redis", cmd, args, "err is", err)
        }
        return
    }

    return
}

My redis-server has a timeout config timeout 300.

Here is my problem: When the program is idle for more than 300 seconds, I will get an EOF error returned from the redis. That is because the redis-server close the connection, but I can't get notified when I use conn, err := p.Get().

So, what is the right way to use radix's pool package?

For now, I just retry 3 times to establish the connection again when I get EOF error. I hope the redis pool can deal the redis-server timeout silently without my concerns.

Could you give me some suggestions?

mediocregopher commented 8 years ago

Hi there! Unfortunately radix's pool package does not spin up any go-routines of its own to do any work in the background, so there's no way for it to keep connections alive on its own. A very simply method you can use to ensure connections don't timeout would be to have something like this called after you create the pool:

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

There's no need to check for errors in there or anything, the Cmd method on pool will handle any connection errors it sees and make sure that dead connections don't get returned to the pool.

On that note, your code could be made a little simpler if you yourself used the Cmd method on pool, so you wouldn't have to worry about the Get and Put. That's not really related to your problem, but maybe it'll save you some lines :P

Akagi201 commented 8 years ago

@mediocregopher Thanks for your answers very much!

mediocregopher commented 8 years ago

No problem! Let me know if there's anything else I can help with

On Wed, Feb 3, 2016, 7:14 PM Bob Liu notifications@github.com wrote:

@mediocregopher https://github.com/mediocregopher Thanks for your answers very much!

— Reply to this email directly or view it on GitHub https://github.com/mediocregopher/radix.v2/issues/21#issuecomment-179578271 .

Akagi201 commented 8 years ago

@mediocregopher What I may be worried is whether the period "PING" request cost a lot for the redis server. And I tested that your code also works for redis-server restart.

mediocregopher commented 8 years ago

Its only getting one every second, if that sleep wasn't there I think it wouldn't be great, but with it I can't imagine it being a problem. I've hit a redis server on meh hardware with thousands of actual requests per second, a couple extra pings here and there won't hurt anything.

On Wed, Feb 3, 2016, 8:05 PM Bob Liu notifications@github.com wrote:

@mediocregopher https://github.com/mediocregopher What I may be worried is whether the period "PING" request cost a lot for the redis server. And I tested that your code also works for redis-server restart.

— Reply to this email directly or view it on GitHub https://github.com/mediocregopher/radix.v2/issues/21#issuecomment-179592216 .

Akagi201 commented 8 years ago

@mediocregopher Thanks~~

daemonfire300 commented 8 years ago

@mediocregopher I think, this should probably be handled like in redigo or other libraries, where the pool checks if the connection is alive, if it throws an error == not alive and spins up a new connection and returns it.

mediocregopher commented 8 years ago

@daemonfire300 that solution doesn't actually improve user experience in any way, although it feels like it would. The problem is that in between the time of the pool giving you the connection and you using it there's a chance of that connection dying, regardless of if the pool is doing its own checks or not. So your application code must expect that the connection might be dead when it goes to use it, and handle that appropriately, in order to be correct. So even if radix's pool did do its own checking your code would still look the same.

The one difference is that sometimes you end up needing to write ping loops like above. I have an idea about how to go about making them not be needed in most cases that also doesn't involve an extra go-routine, I just haven't had a chance to work on it yet.