Skip to content

Conversation

jml
Copy link
Contributor

@jml jml commented Oct 12, 2016

As discussed, this has a minor memory leak, and the potential performance gains should probably be justified with evidence before we keep all this extra accounting code.


This change is Reviewable

@tomwilkie
Copy link
Contributor

Making a new http.Client for every RPC will prevent reuse of http connections.

@tomwilkie
Copy link
Contributor

And will leak existing connections.

@jml
Copy link
Contributor Author

jml commented Oct 12, 2016

Ah, I see. There's internal state that caches connections:

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.

From https://godoc.org/net/http#Client

How does Go clean up these cached connections?

@jml jml unassigned juliusv Oct 12, 2016
@jml jml closed this Oct 17, 2016
@tomwilkie tomwilkie deleted the drop-client-cache branch November 5, 2016 22:44
tomwilkie added a commit that referenced this pull request Nov 18, 2016
fd875e2 Fix test wrt shellcheck
54ec2d9 Don't capitalise error messages
19d3b6e Merge pull request #49 from weaveworks/pin-shfmt
fea98f6 Go get from the vendor dir
1d867b0 Try and vendor a specific version of shfmt
76619c2 Merge pull request #48 from weaveworks/revert-41-user-tokens
4f96c51 Revert "Add experimental support for user tokens"
d00033f Merge pull request #41 from weaveworks/user-tokens
245ed26 Merge pull request #47 from weaveworks/46-shfmt
c1d7815 Fix shfmt error
cb39746 Don't overright lint_result with 0 when shellcheck succeeds
8ab80e8 Merge pull request #45 from weaveworks/lint
83d5bd1 getting integration/config and test shellcheck-compliant
cff9ec3 Fix some shellcheck errors
7a843d6 run shellcheck as part of lint if it is installed
31552a0 removing spurious space from test
6ca7c5f Merge pull request #44 from weaveworks/shfmt
952356d Allow lint to lint itself
b7ac59c Run shfmt on all shell files in this repo
5570b0e Add shfmt formatting of shell files in lint
0a67594 fix circle build by splatting gopath permissions
b990f48 Merge pull request #42 from kinvolk/lorenzo/fix-git-diff
224a145 Check if SHA1 exists before calling `git diff`
1c3000d Add auto_apply config for wcloud
0ebf5c0 Fix wcloud -serivice
354e083 Fixing lint
586060b Add experimental support for user tokens

git-subtree-dir: tools
git-subtree-split: fd875e27c5379d443574bcf20f24a52a594871ca
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants