New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
net/http: leaking connections in Client #4049
Labels
Milestone
Comments
thanks for the response mike. We are not reusing the client because we need to specify a dynamic timeout. Within a day's time around 2000 open connections are piled up in "ESTABLISHED" state. Please find below the relevant section of the code. func PostForm(url string, data url.Values, tracker string, timeout time.Duration) (body []byte, err error) { out.Printf("[%s] Firing a POST request to [%s] with data [%v]\n", tracker, url, data) var resp *http.Response client := http.Client{ Transport: &http.Transport{ Dial: timeoutDialler(timeout, tracker), DisableKeepAlives: true, }, } resp, err = client.PostForm(url, data) if err != nil { out.Printf("[%s] Error in POST request. Reason = [%s]", tracker, err) return } defer resp.Body.Close() body, err = ioutil.ReadAll(resp.Body) if err != nil { out.Printf("[%s] Error in fetching request body. Reason = [%s]", tracker, err) return } return } CALL (within a goroutine): response, err = PostForm(url, postParms, tracker, time.Duration(210*time.Second)) Our app spawns several goroutines and each of them talks to multiple servers running on different machines using this POST request. I have set the limit for max parallel goroutines to 15. Please let me know if you need any more information or if there is a better way of doing it. |
This is easy to fix, if you make your client a global var, this should solve your leaky connection. also your notice an improvement in speed, since it will reuse connections, even though you have set DisableKeepAlives guess to try and fix the leak? from net/http client docs. The Client's Transport typically has internal state (cached TCP connections), so Clients should be reused instead of created as needed. Clients are safe for concurrent use by multiple goroutines. |
Thanks Mike. I will try reusing the client and let you know if it worked. Is there a way to add a timeout if i declare a global var? Because in my case i need different requests to timeout after different intervals of time. Currently i am doing it this way whenever i create a new client object, func timeoutDialler(timeout time.Duration, tracker string) func(net, addr string) (client net.Conn, err error) { return func(netw, addr string) (net.Conn, error) { client, err := net.DialTimeout(netw, addr, time.Duration(30*time.Second)) if err != nil { out.Printf("[%s] Failed to connect to [%s]. Timed out after 30 seconds\n", tracker, addr) return nil, err } client.SetDeadline(time.Now().Add(timeout)) return client, nil } } |
One thing I should have mentioned, is you do not have to make client a global var, you can pass it to PostForm as a pointer. As for timing out, I've not had to do this my self. But one method might be to create your own client struct and embed http.Client. then say attach a Timeout method that changes the Dial function. however you would have to provide some synchronization with this method. |
This code may help to run more tests. http://play.golang.org/p/w5JMXiGLCT If client does 23: tcp_conn.SetLinger(0) then connections are closed right away. If that line is commented, connections hang in TIME_WAIT. |
It's not clear to me what this bug is about, or if there's actually a problem here or people are just talking about TCP's normal behavior. There have been a number of DialTimeout and HTTP changes in the past few months. I'm setting this bug to WaitingForReply for now. Please reply with a summary of the problem, example code showing the problem, and which version of Go and operating system you're using. If I don't get a reply, I'll close this assuming it's an old dup. Status changed to WaitingForReply. |
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
by thejuskrishna:
The text was updated successfully, but these errors were encountered: