Design on the FIFO connection pool

[jasonjoo2010]

Hi, folks

Very glad to see go-redis begins to consider a different type of connection pool(FIFO pool) in the latest version and has done it within small pieces of code. Good job!

We also have implemented a FIFO connection pool in our interval fork of go-redis. Compared to the recent feature, I have read and thought carefully on it and found the following points that worths more considerations:

1. The new FIFO pool doesn't support recycling the idle connections anymore if there was a continuous traffic. Because the traffic will be distributed well between the connections in the pool. That also means the pool won't shrink back again after any expansions.
2. The poping is a little expensive on memory operations with lots of memory shifting. A linked list would be better to improve. I have read the previous proposal on the FIFO pool and found the sample pieces of code were very similar to our internal implementation. (Though I don't know why we abandon the idle connections recycling feature on the final implementation.)

And let me explain a bit more why the FIFO pool is important in the proxy scenarios. As we all know the traffic distribution on the connection in the pool is now balanced. It's not a big deal in most cases. When a peak came, occasionally one of the remote proxies is a little slower than others but may not last long, the connections to it could be returned back a bit slower than others. Then after a long time running a reordering will happen to the connection pool, that is, some of the connections of that "lucky" endpoint sit on the top of the pool stack. Then more traffic will be applied to a single endpoint which will cause a high load on it.

Sure it's not critical because the traffic will have the chance to spread to other endpoints if the lucky one is "slow" enough under a higher load. But still, we will observe a significant imbalance in CPU consumption, QPS applied, a little higher latency on the lucky endpoint.

I have several figures that show the difference between the two types of pool.

Serial Test

Firstly we did a serial test. We used a simple loop to do the test. Only the last connection in the idle list has the chance to be used if using the LIFO connection pool.

We can see the difference in this use case:

• All the traffic is applied to a single connection when using the LIFO pool. The traffic is balanced among the 10 idled connections when using the FIFO pool.
• They have similar performance.
• The difference in the latency is in the epoll model the more traffic the lower latency under a specific QPS.

Concurrent Test

We created a bunch of routines to simultaneously send the requests. And use a limiter to keep the traffic at a certain load and change the limit periodically to simulate the real changing traffic.

From the results, we can see:

• The traffic is balanced as expected when using the FIFO pool compared to the LIFO pool.
• The IdleTimeout and MaxConnAge are working correctly with the FIFO pool.
• The CPU consumptions are balanced with the FIFO pool.

More with MaxConnAge

Sometimes we use MaxConnAge to make the connections have the chance to refresh. But in the current design, it will cause most of the connections to expire together since they are created together, too. So we also implement a tweak on it trying to distribute their life in the last 20% expected lifetime. By doing this we won't have a big chance to recreate many connections in a short time.

So how do you think of it, is it necessary to have it for everyone? Btw we have used it in production for several months so I believe it's ready.

[vmihailenco]

Thanks for the detailed message - this is definitely an interesting discussion 👍

The new FIFO pool doesn't support recycling the idle connections anymore if there was a continuous traffic. Because the traffic will be distributed well between the connections in the pool. That also means the pool won't shrink back again after any expansions.

This is expected and I don't see how to fix/improve it. That is why we have both FIFO and LIFO. Does your fork address this?

The poping is a little expensive on memory operations with lots of memory shifting. A linked list would be better to improve.

We decided to continue to use a slice to have minimal diff from LIFO. database/sql also uses this so we assumed it is fast enough.

I guess we could switch to using linked lists if you have some numbers (benchmark) proving it is worth it...

So we also implement a tweak on it trying to distribute their life in the last 20% expected lifetime.

Would be nice to have this :+1 (in a separate PR if possible).

[jasonjoo2010]

Hi @vmihailenco

Thanks for your reply.
Coming to our internal implementation, we implement a new pool following the pool.Pooler interface and also its own unit test. Though it's not an implementation with minor changes, it works better.

It supports the idle connection recycle by sampling the count of the idle connections 10 times each IdleTimeout period, and determine whether to recycle or not by comparing the MinIdle and the minimum count in the samples.

And more, we introduce a PoolType in the Options as an enum instead of a bool to make it extensible. Other key points are mentioned in the article above.

Coming to the time complexity, using a linked list will be definitely more efficient than doing memory copying when get each time. Surely there is still space to improve by reusing the linked list entry structure but it could be discussed. Currently we don't have a big stress in GC.

[vmihailenco]

Hey @jasonjoo2010,

Couild you send a PR so we can look at your changes and then decide how to proceed? I am a bit worried that we will have to duplicate some functionality in LIFO and FIFO pools, but perhaps it is not too bad...

[jasonjoo2010]

Hi @vmihailenco

I have submitted it here #1869

And also have marked some points. Let me know if you have any questions or different / better thoughts on it.