mediocregopher / radix.v2

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

an error like "use of closed network connection" occured when using the sentienl of radix.v2 #57

Closed panjf2000 closed 7 years ago

panjf2000 commented 7 years ago

i have a program which would run ten goroutines to pop up messages from redis,

and this program run well at the beginning, but after running for some time, it would get a error EOF

and all goroutines would get an error "write tcp xx.xx.xx.xx:25252->xx.xx.xx.xx:7239: use of closed network connection" when reading a message from redis in these codes: redisClient.Cmd("RPOP", p.queue).Bytes()

how can i fix this issue?

panjf2000 commented 7 years ago

and why cannot i pass a list of sentinel nodes to function “sentinel.NewClient()” when init a sentienl and automatically switch to new sentiel address if the current node is unavailable?

mediocregopher commented 7 years ago

Hey there! I'm not sure why you're getting the closed network error, could it be that your master is failing over to a slave? If so you'll need to retrieve a new connection from the sentinel client.

As for NewClient, that's my fault, I designed the client poorly and now we're kind of stuck with it =/

panjf2000 commented 7 years ago

@mediocregopher hey man! first of all,thanks for your response. and i've got a solution from your previous reply to another man of the first issue, which claimed that i can add the following code to keep a redis connection alive:

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

but what i am using is the sentienl package not the pool package, so the redis connection got from sentinel would got a "wrong type" error when it try pop up a message from redis after i start a new goroutine to execute "ping" action, and it seems to unsafe-thread. any advice to this problem?

by the way, i've read your source code and i noticed there are two function spin and subspin in the NewClientCustom function to keep sentinel Ping periodically, so why don't you do the same thing to every single redis connection?what's the thinking behind the current decision?

mediocregopher commented 7 years ago

why don't you do the same thing to every single redis connection?

The pool package doesn't currently do this because I wanted the implementation to be the absolute simplest and smallest it could be, and doing this would add a whole extra go-routine to it. It also made the close semantics a bit weird, since currently the method is called Empty which doesn't imply that the go-routine would be stopped.

On the other hand, so many people have asked for it at this point I'm really thinking about just doing it.

In the meantime, you should be able to do the following:

go func() {
    for {
        time.Sleep(1 * time.Second)
        conn, err := client.GetMaster("master")
        if err != nil {
            // log if you want
            continue
        }
        conn.Cmd("PING")
        client.PutMaster("master", conn)
    }
}

and it would do roughly the same thing that the snippet you posted does, but with a sentinel client.

panjf2000 commented 7 years ago

@mediocregopher thanks, man!

mediocregopher commented 7 years ago

gonna go ahead and close this, let me know if there's any other questions you have :)