Close idle conns when closing consul topo #15975
Closed
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.
Description
The
*api.Config
struct for the Consul client (ingo/vt/topo/consultopo
) holds a*http.Transport
in the config, which is long-lived 😕This object being in the "config" can cause idle connections to be left open when
.Close()
is called on the Consul topo implementation, which currently does nothing with the*http.Transport
. This usually won't matter - the idle conn will eventually get closed, but the fact all connections are not closed on.Close()
can cause unit test failures Slack sees on a yet-to-upstream patchThis PR:
.CloseIdleConnections()
on the*http.Transport
in the.Close()
of the consul topo. This ensures that the idle connections the long-lived*http.Transport
holds are closeddefer topoServer.Close()
to the.Setup()
method ingo/vt/vttest/topoctl.go
- thistopoServer
is only used in.Setup()
and has no.Close()
call todayRelated Issue(s)
Checklist
Deployment Notes