Update default HTTP client transport settings

[welteki]

Description

Remove deprecated DualStack dialer field and unnecessary KeepAlive override. Increase IdleConnTimeout from 120ms to 5s for more efficient connection reuse.

Motivation and context

• The DualStack field on net.Dialer has been deprecated since Go 1.16 and is a no-op.
• The short KeepAlive of 1s was unnecessarily aggressive; the Go default (15s) is more appropriate.
• The IdleConnTimeout of 120ms was too short, causing idle connections to be closed prematurely and increasing connection churn under load.

How has this been tested?

The changes were vendored into faas-netes Pro and tested with a load test.

[reviewfn]

AI Pull Request Overview

Summary

• Removes deprecated DualStack field from net.Dialer (no-op since Go 1.16)
• Eliminates unnecessary KeepAlive override of 1s, allowing default 15s
• Increases IdleConnTimeout from 120ms to 5s for improved connection reuse
• Changes aim to reduce connection churn under load and align with Go best practices
• Tested via load testing in faas-netes integration
• No breaking changes or API modifications
• Maintains backward compatibility

Approval rating (1-10)

9/10 - Solid performance optimization with proper testing, removes deprecated code, minimal risk.

Summary per file

Summary per file

File path	Summary
proxy/proxy.go	Updates HTTP transport settings: removes deprecated DualStack, unnecessary KeepAlive override, increases IdleConnTimeout from 120ms to 5s

Overall Assessment

The changes represent a well-motivated performance improvement by removing deprecated fields and tuning connection timeouts for better efficiency under load. The 120ms IdleConnTimeout was indeed too aggressive, potentially causing unnecessary connection overhead. Increasing to 5s aligns with common Go HTTP client recommendations and should improve throughput in high-traffic scenarios. Removal of deprecated DualStack field is appropriate as it's been a no-op since Go 1.16. The KeepAlive override removal allows the default 15s interval, which is more reasonable than 1s. Load testing provides confidence in the changes.

Detailed Review

Detailed Review

proxy/proxy.go

• IdleConnTimeout increase: The change from 120ms to 5s is justified for reducing connection churn under load. However, consider if this timeout should be configurable via FaaSConfig to allow tuning for different deployment scenarios. The hardcoded 5s may not be optimal for all environments.

• DualStack removal: Correctly removes deprecated field that has been no-op since Go 1.16. No functional impact.

• KeepAlive removal: Removing the 1s override allows Go's default 15s KeepAlive, which is more appropriate for server-side connections. The 1s interval was unnecessarily aggressive and could increase CPU overhead from frequent keepalive packets.

• Potential regression risk: Longer idle timeout may hold connections open longer in low-traffic scenarios, potentially consuming more server resources. However, the benefit of reduced connection establishment overhead likely outweighs this in typical FaaS workloads.

• Testing consideration: While load testing was performed, consider adding unit tests for the NewProxyClient function to verify transport configuration. The current code lacks explicit tests for these timeout values.

• Consistency check: The timeout constants (TLSHandshakeTimeout: 10s, ExpectContinueTimeout: 1.5s) remain unchanged. Ensure these align with the increased IdleConnTimeout - 5s idle vs 10s handshake seems reasonable.

• Security impact: No apparent security implications. Longer idle connections don't introduce new vulnerabilities.

• Migration impact: No migration needed as this is internal client configuration. Existing functions won't be affected.

• Performance assumption: The commit message cites Prometheus and MinIO as similar projects that increased this timeout. Validate that the 5s value is indeed optimal - some sources recommend 90s for high-throughput services.

AI agent details.

Agent processing time: 26.792s
Environment preparation time: 5.803s
Total time from webhook: 36.706s