-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
*: drain http.Response.Body before closing #4880
Conversation
Won't do much speed-up since this is not in the critical path, I think. (Reference google/go-github#317 (comment)) |
@@ -231,6 +232,7 @@ func getVersion(m *Member, rt http.RoundTripper) (*version.Versions, error) { | |||
} | |||
// etcd 2.0 does not have version endpoint on peer url. | |||
if resp.StatusCode == http.StatusNotFound { | |||
io.Copy(ioutil.Discard, resp.Body) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grecefulClose(resp http.Response) {
io.Copy()
resp.Body.Close()
}
@xiang90 PTAL. |
|
||
// gracefulClose drains http.Response.Body to prevent TCP/TLS connections | ||
// from closing, then making it available for reuse. | ||
func gracefulClose(resp *http.Response) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this to pkg/httputil?
@xiang90 PTAL. Thanks! |
@@ -19,6 +19,7 @@ import ( | |||
"encoding/binary" | |||
"fmt" | |||
"io" | |||
"io/ioutil" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this line?
LGTM after tests pass. |
My goimports didn't work. Fixed. Will merge after green lights. Thanks! |
Draining the body prevents the TCP/TLS connection from closing
and makes the connection available for reuse.