search

valyala / fasthttp

Fast HTTP package for Go. Tuned for high performance. Zero memory allocations in hot paths. Up to 10x faster than net/http
MIT License
21.95k stars 1.76k forks source link

Recommendation on cleaning of HostClient. #726

Open anshul-jain881 opened 4 years ago

anshul-jain881 commented 4 years ago

As of now the only way to override Host header is to use HostClient as per Issue 318. This means we have to manage reusing/cleanup of HostClients ourselves which fasthttp.Client provided out of the box.

We will be writing something very similar to fasthttp.Client to handle this. Is it ok to remove the HostClient if it is not being used from more than MaxIdleConnDuration*2 or something, as we are sure all goroutines for individual connection cleaning would have run? We don't have access to connsCount as it is an unexported field and was modified to use connsCount instead of time. 

if t.Sub(v.LastUseTime()) > (MaxIdleConnDuration*2) {
                delete(m, k)
            }

If not, what is the recommendation here?

erikdubbelboer commented 4 years ago

Waiting MaxIdleConnDuration*2 after PendingRequests() returned 0 should work yes.

But I would also accept a pull request that adds a CloseIdleConnections() function to HostClient. In which case calling CloseIdleConnections() when PendingRequests() == 0 will clean up the HostClient making sure no outgoing connections are left and no goroutines are running anymore.

anshul-jain881 commented 4 years ago

Gotcha!!, I think we also need to make sure that While waiting MaxIdleConnDuration*2(minimum MaxIdleConnDuration+1) after PendingRequests() returned 0, no new request came or LastUseTime has not changed since then, as we wont be holding the lock on the map while sleeping. If any request came in between just start over.

For now this satisfies my use-case. I will see if I can work out the CloseIdleConnections() solution.

Thanks for the help 👍