reverseproxy: Adjust defaults, document defaults (#4436)

* reverseproxy: Adjust defaults, document defaults

Related to some of the issues in https://github.com/caddyserver/caddy/issues/4245, a complaint about the proxy transport defaults not being properly documented in https://caddy.community/t/default-values-for-directives/14254/6.

- Dug into the stdlib to find the actual defaults for some of the timeouts and buffer limits, documenting them in godoc so the JSON docs get them next release.

- Moved the keep-alive and dial-timeout defaults from `reverseproxy.go` to `httptransport.go`. It doesn't make sense to set defaults in the proxy, because then any time the transport is configured with non-defaults, the keep-alive and dial-timeout defaults are lost!

- Sped up the dial timeout from 10s to 3s, in practice it rarely makes sense to wait a whole 10s for dialing. A shorter timeout helps a lot with the load balancer retries, so using something lower helps with user experience.

* reverseproxy: Make keepalive interval configurable via Caddyfile

* fastcgi: DialTimeout default for fastcgi transport too

5 files changed, 54 insertions, 20 deletions

diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_options.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_options.txt
index 544bb9f..70e7af6 100644
--- a/caddytest/integration/caddyfile_adapt/reverse_proxy_options.txt
+++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_options.txt
@@ -22,6 +22,7 @@ https://example.com {
 			compression off
 			max_conns_per_host 5
 			keepalive_idle_conns_per_host 2
+			keepalive_interval 30s
 		}
 	}
 }
@@ -80,7 +81,8 @@ https://example.com {
 														"dial_timeout": 3000000000,
 														"expect_continue_timeout": 9000000000,
 														"keep_alive": {
-															"max_idle_conns_per_host": 2
+															"max_idle_conns_per_host": 2,
+															"probe_interval": 30000000000
 														},
 														"max_conns_per_host": 5,
 														"max_response_header_size": 30000000,
diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go
index c7f555f..c37efd0 100644
--- a/modules/caddyhttp/reverseproxy/caddyfile.go
+++ b/modules/caddyhttp/reverseproxy/caddyfile.go
@@ -806,7 +806,9 @@ func (h *Handler) FinalizeUnmarshalCaddyfile(helper httpcaddyfile.Helper) error
 //         tls_trusted_ca_certs <cert_files...>
 //         tls_server_name <sni>
 //         keepalive [off|<duration>]
+//         keepalive_interval <interval>
 //         keepalive_idle_conns <max_count>
+//         keepalive_idle_conns_per_host <count>
 //         versions <versions...>
 //         compression off
 //         max_conns_per_host <count>
@@ -966,6 +968,19 @@ func (h *HTTPTransport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 				}
 				h.KeepAlive.IdleConnTimeout = caddy.Duration(dur)
 
+			case "keepalive_interval":
+				if !d.NextArg() {
+					return d.ArgErr()
+				}
+				dur, err := caddy.ParseDuration(d.Val())
+				if err != nil {
+					return d.Errf("bad interval value '%s': %v", d.Val(), err)
+				}
+				if h.KeepAlive == nil {
+					h.KeepAlive = new(KeepAlive)
+				}
+				h.KeepAlive.ProbeInterval = caddy.Duration(dur)
+
 			case "keepalive_idle_conns":
 				if !d.NextArg() {
 					return d.ArgErr()
diff --git a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go
index 40e0207..05b776d 100644
--- a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go
+++ b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go
@@ -64,7 +64,7 @@ type Transport struct {
 	// Extra environment variables.
 	EnvVars map[string]string `json:"env,omitempty"`
 
-	// The duration used to set a deadline when connecting to an upstream.
+	// The duration used to set a deadline when connecting to an upstream. Default: `3s`.
 	DialTimeout caddy.Duration `json:"dial_timeout,omitempty"`
 
 	// The duration used to set a deadline when reading from the FastCGI server.
@@ -88,13 +88,22 @@ func (Transport) CaddyModule() caddy.ModuleInfo {
 // Provision sets up t.
 func (t *Transport) Provision(ctx caddy.Context) error {
 	t.logger = ctx.Logger(t)
+
 	if t.Root == "" {
 		t.Root = "{http.vars.root}"
 	}
+
 	t.serverSoftware = "Caddy"
 	if mod := caddy.GoModule(); mod.Version != "" {
 		t.serverSoftware += "/" + mod.Version
 	}
+
+	// Set a relatively short default dial timeout.
+	// This is helpful to make load-balancer retries more speedy.
+	if t.DialTimeout == 0 {
+		t.DialTimeout = caddy.Duration(3 * time.Second)
+	}
+
 	return nil
 }
 
diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go
index 35bc947..f23504c 100644
--- a/modules/caddyhttp/reverseproxy/httptransport.go
+++ b/modules/caddyhttp/reverseproxy/httptransport.go
@@ -63,28 +63,28 @@ type HTTPTransport struct {
 	MaxConnsPerHost int `json:"max_conns_per_host,omitempty"`
 
 	// How long to wait before timing out trying to connect to
-	// an upstream.
+	// an upstream. Default: `3s`.
 	DialTimeout caddy.Duration `json:"dial_timeout,omitempty"`
 
 	// How long to wait before spawning an RFC 6555 Fast Fallback
-	// connection. A negative value disables this.
+	// connection. A negative value disables this. Default: `300ms`.
 	FallbackDelay caddy.Duration `json:"dial_fallback_delay,omitempty"`
 
-	// How long to wait for reading response headers from server.
+	// How long to wait for reading response headers from server. Default: No timeout.
 	ResponseHeaderTimeout caddy.Duration `json:"response_header_timeout,omitempty"`
 
 	// The length of time to wait for a server's first response
 	// headers after fully writing the request headers if the
-	// request has a header "Expect: 100-continue".
+	// request has a header "Expect: 100-continue". Default: No timeout.
 	ExpectContinueTimeout caddy.Duration `json:"expect_continue_timeout,omitempty"`
 
-	// The maximum bytes to read from response headers.
+	// The maximum bytes to read from response headers. Default: `10MiB`.
 	MaxResponseHeaderSize int64 `json:"max_response_header_size,omitempty"`
 
-	// The size of the write buffer in bytes.
+	// The size of the write buffer in bytes. Default: `4KiB`.
 	WriteBufferSize int `json:"write_buffer_size,omitempty"`
 
-	// The size of the read buffer in bytes.
+	// The size of the read buffer in bytes. Default: `4KiB`.
 	ReadBufferSize int `json:"read_buffer_size,omitempty"`
 
 	// The versions of HTTP to support. As a special case, "h2c"
@@ -147,6 +147,21 @@ func (h *HTTPTransport) Provision(ctx caddy.Context) error {
 
 // NewTransport builds a standard-lib-compatible http.Transport value from h.
 func (h *HTTPTransport) NewTransport(ctx caddy.Context) (*http.Transport, error) {
+	// Set keep-alive defaults if it wasn't otherwise configured
+	if h.KeepAlive == nil {
+		h.KeepAlive = &KeepAlive{
+			ProbeInterval:       caddy.Duration(30 * time.Second),
+			IdleConnTimeout:     caddy.Duration(2 * time.Minute),
+			MaxIdleConnsPerHost: 32, // seems about optimal, see #2805
+		}
+	}
+
+	// Set a relatively short default dial timeout.
+	// This is helpful to make load-balancer retries more speedy.
+	if h.DialTimeout == 0 {
+		h.DialTimeout = caddy.Duration(3 * time.Second)
+	}
+
 	dialer := &net.Dialer{
 		Timeout:       time.Duration(h.DialTimeout),
 		FallbackDelay: time.Duration(h.FallbackDelay),
@@ -303,7 +318,7 @@ type TLSConfig struct {
 	// option except in testing or local development environments.
 	InsecureSkipVerify bool `json:"insecure_skip_verify,omitempty"`
 
-	// The duration to allow a TLS handshake to a server.
+	// The duration to allow a TLS handshake to a server. Default: No timeout.
 	HandshakeTimeout caddy.Duration `json:"handshake_timeout,omitempty"`
 
 	// The server name (SNI) to use in TLS handshakes.
@@ -405,7 +420,7 @@ type KeepAlive struct {
 	// Whether HTTP Keep-Alive is enabled. Default: true
 	Enabled *bool `json:"enabled,omitempty"`
 
-	// How often to probe for liveness.
+	// How often to probe for liveness. Default: `30s`.
 	ProbeInterval caddy.Duration `json:"probe_interval,omitempty"`
 
 	// Maximum number of idle connections. Default: 0, which means no limit.
@@ -414,7 +429,7 @@ type KeepAlive struct {
 	// Maximum number of idle connections per host. Default: 32.
 	MaxIdleConnsPerHost int `json:"max_idle_conns_per_host,omitempty"`
 
-	// How long connections should be kept alive when idle. Default: 0, which means no timeout.
+	// How long connections should be kept alive when idle. Default: `2m`.
 	IdleConnTimeout caddy.Duration `json:"idle_timeout,omitempty"`
 }
 
diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go
index 36dfbfe..c629ef6 100644
--- a/modules/caddyhttp/reverseproxy/reverseproxy.go
+++ b/modules/caddyhttp/reverseproxy/reverseproxy.go
@@ -204,14 +204,7 @@ func (h *Handler) Provision(ctx caddy.Context) error {
 
 	// set up transport
 	if h.Transport == nil {
-		t := &HTTPTransport{
-			KeepAlive: &KeepAlive{
-				ProbeInterval:       caddy.Duration(30 * time.Second),
-				IdleConnTimeout:     caddy.Duration(2 * time.Minute),
-				MaxIdleConnsPerHost: 32, // seems about optimal, see #2805
-			},
-			DialTimeout: caddy.Duration(10 * time.Second),
-		}
+		t := &HTTPTransport{}
 		err := t.Provision(ctx)
 		if err != nil {
 			return fmt.Errorf("provisioning default transport: %v", err)