From 9ee68c1bd57d72e8a969f1da492bd51bfa5ed9a0 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 24 Nov 2021 01:32:25 -0500 Subject: [PATCH] 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 --- .../caddyfile_adapt/reverse_proxy_options.txt | 4 ++- modules/caddyhttp/reverseproxy/caddyfile.go | 15 ++++++++ .../caddyhttp/reverseproxy/fastcgi/fastcgi.go | 11 +++++- .../caddyhttp/reverseproxy/httptransport.go | 35 +++++++++++++------ .../caddyhttp/reverseproxy/reverseproxy.go | 9 +---- 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 544bb9ff..70e7af60 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 c7f555f8..c37efd07 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 // tls_server_name // keepalive [off|] +// keepalive_interval // keepalive_idle_conns +// keepalive_idle_conns_per_host // versions // compression off // max_conns_per_host @@ -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 40e0207f..05b776da 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 35bc9478..f23504c7 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 36dfbfe6..c629ef6e 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)