mediocregopher / radix.v2

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

Too hard to tune connection pool size #70

Closed fillest closed 6 years ago

fillest commented 7 years ago

We've got a somewhat latency-sensitive app and use e.g. sentinel.NewClientCustom to create a pool before any requests start to be served to avoid connecting on-the-fly. But how to know how many connections are needed? In theory we can 1) set an approximately huge number, 2) check how many connections are used in reality an adjust this number (having too many connections may waste some resources, also they are created sequentially which affects the start up time). But one can't at least just read the pool struct fields thanks to golang design ("cannot refer to unexported field").

Example solutions:

Also I've failed to grasp the logic of returning extra connections to the pool. Pool.pool is a chan with fixed size. Which means when extra-created connection is being returned... it blocks or closes it or?..

mediocregopher commented 6 years ago

Hi @fillest, sorry this took so long for me to respond to. It slipped through my email (inbox is doing something to my github emails and I haven't yet figured out what).

I'm a little confused by what you're referring to when you say "pool size". Do you mean the capacity of the pool (total number of connections it can hold at one time)? If so this number is the size which is passed in when creating the pool. If you mean the number of connections currently available to be used (aka capacity - connections currently in use) then that number can be retrieved using the Avail method on the pool.

But how to know how many connections are needed?

This is a bit of a tricky problem, and is going to very much depend on your application. If you're running some kind of webapp or service then you probably want to base your pool capacity on expected number of concurrent requests (maybe concurrent requests x2 or something like that). You're right that figuring out this number in general can be difficult though, that's something which could be improved.

pool could log when it creates an extra connection (above the size), ideally including the new total size (so one could be able to e.g. grep it from the logs)

This is probably the best solution, having something which can be registered on Pool, like a callback or a channel, which will be triggered when a connection is created due to the pool being empty. In fact you could implement something like this currently, using pool.NewCustom:

poolCreated := make(chan struct{})
df := func(network, addr string) (*redis.Client, error) {
    select {
    case <-poolCreated:
        log.Printf("creating new redis connection to %q, pool must be empty", addr)
    default:
    }
    return redis.Dial(network, addr)
}

pool, err := pool.NewCustom("tcp", "127.0.0.1:6379", df)
close(poolCreated)
if err != nil {
    panic(err)
}

Also I've failed to grasp the logic of returning extra connections to the pool. Pool.pool is a chan with fixed size. Which means when extra-created connection is being returned... it blocks or closes it or?..

When an "extra" connection is being returned it is closed and discarded. So they aren't really returned to the pool at all, since the pool is already full in that case.

As a final note, I'd encourage you (as I'm encouraging everyone) to check out radix.v3 if you can. It's currently in beta and probably has some bugs, but the design of it is much more flexible (and faster), and it allows for custom implementations of things like Pool which can still be used by the Cluster or Sentinel clients. I understand if your use-case makes using a beta driver not possible though.

Thanks for submitting the issue, sorry again it took me so long to respond >_<

fillest commented 6 years ago

Hi!

I'm a little confused by what you're referring to when you say "pool size".

I meant just the total number of connections established initially in advance.

In fact you could implement something like this currently, using pool.NewCustom

Yep, but the problem is we're using sentinel and there is no way to pass a custom pool there.

When an "extra" connection is being returned it is closed and discarded. So they aren't really returned to the pool at all, since the pool is already full in that case.

Ah, I got it. Looks like it's also already documented but a bit not clearly enough, IMHO.

radix.v3

Yeah, we are looking forward to happily switching to v3 but only after the core and the sentinel will be stable enough :) Because we are using it in a loaded crucial component.

As for now, I'm just checking netstat -natp | grep procname --line-buffered | grep 6380 | wc -l sometimes in production to increase poolSize. And as the pool indeed can be customized for logging in v3 (via custom ClientFunc I guess) then I'm closing this and waiting for v3 release.

Thanks.