Add a timeout mechanism?
alexcesaro opened this issue · 6 comments
This looks like a great package. I am considering using it but I think it lacks a key feature.
Have you considered adding a timeout mechanism to automatically close connections from the pool after a certain amount of time?
For example, a pool with a timeout of 30 seconds would automatically close connections that have been in the pool for more than 30 seconds.
Having such a mechanism would reduce the risk of getting stale connections from the pool and help decreasing the number of unused open connections with a server when the traffic slows down.
What do you think?
I think this can be done fairly with setting the various fields in net.Dialer and then creating connection according to those. Did you tried it ? There is no need to carry that logic into the pool mechanism itself.
I don't think you can do it with net.Dialer.
And even if it would be possible I think it would be nice for the stale connections to be automatically removed from the pool to avoid returning them to the user with Pool.Get.
The problem is that there is no single way of knowing what a stale connections is. The only way of doing it to try to read from it (peek) and then unread it. I really don't want to put this logic into it. What you need is to wrap it around another net.Conn interface implementation that supports it. Checkout the code, we already wrap the net.Conn.
I am not sure I made myself clear: I don't want a mechanism that check if a connection is stale.
I think what I want was better explained in my first post.
Basically I was thinking of something like a MaxAge parameter for the Pool. When you use PoolConn.Close() a deadline parameter of PoolConn is set to now + Pool.MaxAge.
And when you use Pool.Get, if it is past PoolConn.deadline. The PoolConn is automatically closed and you get the first PoolConn in the Pool which deadline has not passed. (This is a rough description, a background goroutine closing the PoolConn with a past deadline would be more appropriate).
Is it more clear?
If you don't think this is in the scope of this package, no problem!
I got what you mean but to implement this. We have to implement a timer that always runs in background and closes all connection inside the pool. That means first of all that we have to lock the channel (stop anyone to retrieve from it) and then check whether each connection has hit the 30 second timeout or not. We close all connections that timed out and then release the lock.
I think this is not suitable and against the idea of having a pool. What's the idea of a pool if I close connections every 30 seconds ? This package just gives you a primitive, and that's it. Introducing logic into the primitive will make it just worse. You can always fork this and implement it yourself btw, it's pretty stable and I'm not interested to improve it anymore (Because there is not much anymore to improve).
Ok I understand.