mediocregopher / radix.v2

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

Under Multiple goroutines, when using Empty and NewCustom method in same time,connection will enter the pool #94

Closed steedyu closed 5 years ago

steedyu commented 5 years ago

the condition is below: One app has multiple goroutines to deal with many requests. Every request operates the redis. After Issues happen in the network or environment, every request operate the redis falied and every time will re-init redis pool. Before re-init redis pool, one goroutine in the app firstly invoke Empty() and then NewCustom(). So Empty() can't close all connection in pool in one goroutine and the other goroutine will add connection into pool. because the code in pool.go below: mkConn := func() error { client, err := df(network, addr) if err == nil { p.pool <- client } return err }

if size > 0 {
    // make one connection to make sure the redis instance is actually there
    if err := mkConn(); err != nil {
        return &p, err
    }
}

// make the rest of the connections in the background, if any fail it's fine
go func() {
    for i := 0; i < size-1; i++ {
        mkConn()
    }
    close(p.initDoneCh)
}()

i think the bold block code should replace p.Put(client) with p.pool <- client. It ensure when one goroutine invoke empty() ,anothers can't invoke NewCustom() to add client into the pool.

mediocregopher commented 5 years ago

Hi @steedyu, thanks for submitting an issue here. I'm afraid I don't fully understand your problem. NewCustom makes a completely new Pool instance, which will not effect the first Pool in anyway, no matter what it's doing. I'm confused what you mean by:

So Empty() can't close all connection in pool in one goroutine and the other goroutine will add connection into pool.

Also, just a heads up, that radix.v2 is no longer really supported, so I won't be adding any new features to it. All future development is being done on radix/v3, which is faster, more feature-rich, and more flexible. Thanks!

steedyu commented 5 years ago

Ok. Thank you for your reply. I will try to use radix/v3.