fix(http-client): reuse http.Transport to prevent ephemeral port exhaustion#167
Open
sk4mi wants to merge 2 commits into
Open
fix(http-client): reuse http.Transport to prevent ephemeral port exhaustion#167sk4mi wants to merge 2 commits into
sk4mi wants to merge 2 commits into
Conversation
a599209 to
563f376
Compare
…ustion Use a process-level cache of http.Client instances keyed by TLS config and timeout. This ensures connection pooling across reconcile cycles, preventing TCP connection leaks that exhaust the ephemeral port range. - Cache http.Client/Transport in a package-level sync.Map keyed by SHA-256 of TLS config + timeout - Fix response.Body.Close() ordering with defer to prevent leaks if io.ReadAll() fails - All TLS configurations (plain, InsecureSkipVerify, custom CA/certs) are cached uniformly Signed-off-by: Leo Garel <leo.garel@airnity.com>
… and client caching behavior Signed-off-by: Leo Garel <leo.garel@airnity.com>
d5d7377 to
7d354a6
Compare
This file contains hidden or 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
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.
Use a process-level cache of http.Client instances keyed by TLS config and timeout. This ensures connection pooling across reconcile cycles, preventing TCP connection leaks that exhaust the ephemeral port range.
Description of your changes
This PR fixes ephemeral TCP port exhaustion caused by creating a new
http.Clientandhttp.Transporton everySendRequest()call. SinceNewClient()is called per-reconcile viaConnect(), any per-instance state is destroyed each cycle, so transports were never reused — leaving idle connections in TCP TIME_WAIT until the entire ephemeral port range was exhausted.What changed:
sync.Mapcache (httpClientCache) that storeshttp.Clientinstances keyed by a SHA-256 hash of their TLS configuration + timeout. This ensures transport reuse across reconcile cycles for all connection types (plain HTTP, InsecureSkipVerify, custom CA bundles, client certificates).response.Body.Close()to usedeferimmediately after the response is received, preventing body/connection leaks ifio.ReadAll()fails.Reviewers should note:
LoadOrStoreis used instead ofLoad/Storeto handle the race where two goroutines create clients for the same key simultaneously — only one wins, the other is discarded.Clientinterface and all external behavior are unchanged. This is a purely internal optimization.Production context: This was observed on a production cluster where a single provider-http pod accumulated 28,280 open sockets, exhausting the Linux ephemeral port range (32768-60999) and causing
dial tcp: connect: cannot assign requested addresserrors at ~16/min.I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested
The fix was validated at multiple levels:
Unit tests (new):
TestHTTPClientCacheReuse: Two separateclientinstances (simulating consecutive reconcile cycles) with identical TLS config receive the same cachedhttp.Clientfrom the process-levelsync.Map, confirming cross-reconcile transport reuse.TestTransportReuse: Sends 50 sequential HTTP requests through a single client instance and verifies all succeed — proving connection reuse and no port leak under sustained load.Existing tests (unchanged, all pass):
TestSendRequestcovers GET, POST, custom headers, auth token, InsecureSkipVerify, and invalid TLS — all go through the new cache path.TestSendRequestIntegrationperforms end-to-end requests againsthttptest.NewTLSServerwith real TLS handshakes.TestHTTPClientConfigurationconfirms timeout propagation and proxy settings are preserved on cached clients.TestBuildTLSConfig(10 sub-tests) validates all TLS config permutations (nil, empty, CA bundle, client certs, InsecureSkipVerify).CI gate:
make reviewable testpasses (golangci-lint, go vet, all project tests).