misc: Drain body responses so that connection is eligible for reuse #2036
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alienth
reviewed
Mar 14, 2017
| @@ -111,6 +112,10 @@ func enableURL(url string, regexes ...string) func() bool { | |||
| return false | |||
| } | |||
| defer resp.Body.Close() | |||
| if resp.StatusCode != 200 || len(res) == 0 { | |||
There was a problem hiding this comment.
Could just batch this up in an anonymous defer func rather than having this awkward condition here that mirrors future conditions. Since we ignore errors intentionally from CopyN in this case, doesn't matter if a ReadAll has already occurred.
Just a nitpick, tho. Fine as is if you'd prefer.
alienth
reviewed
Mar 14, 2017
collect/queue.go
Outdated
| @@ -138,6 +139,8 @@ func sendBatch(batch []*opentsdb.DataPoint) { | |||
| time.Sleep(d) | |||
| return | |||
| } | |||
| // Drain up to 512 bytes and close the body to let the Transport reuse the connection | |||
There was a problem hiding this comment.
Comment here is semi-erroneous since we don't exactly close the body here.
|
@gbrayut A couple nitpick comments, otherwise lgtm. |
alienth
approved these changes
Mar 14, 2017
… reused I did an audit of the varous places we use Body.Close but don't read any bytes. This should allow connection reuse in those cases.
|
I changed interval.go to use an anon func as that is cleaner. I also updated the comment in queue.go to be clearer. I will do some more testing on scollector dev and then rollout tsdbrelay later tonight. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After Jason updated tsdbrelay I did an audit of the various places we use Body.Close but don't read any bytes. This should allow connection reuse in those cases. Testing on ny-gbraylx01 but would like someone to review @captncraig @alienth @kylebrandt