From e3ae04f8ddb31d38b059f4792118df50ca5e04f9 Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Wed, 15 Apr 2020 11:30:24 +0200 Subject: [PATCH] Add timeout knobs (#25) * Add timeout knobs * Add missing godoc and unexport unnecessarily exported constants --- README.md | 6 +++- internal/proxy/option.go | 60 +++++++++++++++++++++++++++++++++++---- internal/proxy/proxy.go | 45 +++++++++++++++++++++++++---- internal/server/option.go | 42 +++++++++++++++++++-------- internal/server/server.go | 29 +++++++++++++------ main.go | 31 +++++++++++++------- 6 files changed, 169 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 4274025f1..25305ca52 100644 --- a/README.md +++ b/README.md @@ -48,5 +48,9 @@ Usage of ./observatorium: -web.listen string The address on which internal server runs. (default ":8080") -web.timeout duration - The maximum duration before timing out the request, and closing idle connections. (default 5m0s) + The maximum duration before timing out the request, and closing idle connections. (default 2m0s) + -web.timeout.read duration + The maximum duration before reading the entire request, including the body. (default 2m0s) + -web.timeout.write duration + The maximum duration before timing out writes of the response. (default 2m0s) ``` diff --git a/internal/proxy/option.go b/internal/proxy/option.go index 3320cf350..56e8014c7 100644 --- a/internal/proxy/option.go +++ b/internal/proxy/option.go @@ -3,9 +3,15 @@ package proxy import "time" type options struct { - bufferCount int - bufferSizeBytes int - flushInterval time.Duration + bufferCount int + bufferSizeBytes int + maxIdleConns int + flushInterval time.Duration + timeout time.Duration + keepAlive time.Duration + idleConnTimeout time.Duration + tlsHandshakeTimeout time.Duration + expectContinueTimeout time.Duration } // Option overrides behavior of Proxy. @@ -19,23 +25,65 @@ func (f optionFunc) apply(o *options) { f(o) } -// WithBufferCount TODO +// WithBufferCount sets the buffer count option for the reverse proxy. func WithBufferCount(i int) Option { return optionFunc(func(o *options) { o.bufferCount = i }) } -// WithBufferSizeBytes TODO +// WithBufferSizeBytes sets the buffer size bytes option for the reverse proxy. func WithBufferSizeBytes(i int) Option { return optionFunc(func(o *options) { o.bufferSizeBytes = i }) } -// WithFlushInterval TODO +// WithFlushInterval sets the flush interval option for the reverse proxy. func WithFlushInterval(t time.Duration) Option { return optionFunc(func(o *options) { o.flushInterval = t }) } + +// WithMaxIdsConns sets the max idle conns for the underlying reverse proxy transport. +func WithMaxIdsConns(i int) Option { + return optionFunc(func(o *options) { + o.maxIdleConns = i + }) +} + +// WithIdleConnTimeout sets the idle timeout duration for the underlying reverse proxy transport. +func WithIdleConnTimeout(t time.Duration) Option { + return optionFunc(func(o *options) { + o.idleConnTimeout = t + }) +} + +// WithTimeout sets the timeout duration for the underlying reverse proxy connection. +func WithTimeout(t time.Duration) Option { + return optionFunc(func(o *options) { + o.timeout = t + }) +} + +// WithKeepAlive sets the keep alive duration for the underlying reverse proxy connection. +func WithKeepAlive(t time.Duration) Option { + return optionFunc(func(o *options) { + o.keepAlive = t + }) +} + +// WithTLSHandshakeTimeout sets the max TLS handshake timeout duration for the underlying reverse proxy transport. +func WithTLSHandshakeTimeout(t time.Duration) Option { + return optionFunc(func(o *options) { + o.tlsHandshakeTimeout = t + }) +} + +// WithExpectContinueTimeout sets the max expected continue timeout duration for the underlying reverse proxy transport. +func WithExpectContinueTimeout(t time.Duration) Option { + return optionFunc(func(o *options) { + o.expectContinueTimeout = t + }) +} diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 66f415012..f25f11b18 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -4,6 +4,7 @@ import ( "bytes" "io/ioutil" stdlog "log" + "net" "net/http" "net/http/httputil" "net/url" @@ -17,12 +18,26 @@ import ( ) const ( - // DefaultBufferCount TODO + // DefaultBufferCount is the default value for the maximum size of the buffer pool for the reverse proxy. DefaultBufferCount = 2 * 1024 - // DefaultBufferSizeBytes TODO + // DefaultBufferSizeBytes is the default value for the length of the buffers in the buffer pool for the reverse proxy. DefaultBufferSizeBytes = 32 * 1024 - // DefaultFlushInterval TODO + // DefaultFlushInterval is the default value for the flush interval of reverse proxy to flush to the client while copying the response body. DefaultFlushInterval = time.Duration(-1) + + // defaultTimeout is the default value for the maximum amount of time a dial will wait for a connect to complete. + defaultTimeout = 30 * time.Second + // defaultKeepAlive is the default value for the interval between keep-alive probes for an active network connection. + defaultKeepAlive = 30 * time.Second + // defaultMaxIdleConns is the default value for the maximum idle (keep-alive) connections to keep per-host. + defaultMaxIdleConns = 100 + // defaultIdleConnTimeout is the default value for the maximum amount of time an idle (keep-alive) connection will remain idle before closing itself. + defaultIdleConnTimeout = 90 * time.Second + // defaultTLSHandshakeTimeout is the default value for the maximum amount of time waiting to wait for a TLS handshake. + defaultTLSHandshakeTimeout = 10 * time.Second + // defaultExpectContinueTimeout is the default value for the amount of time to wait for a server's first response headers after fully writing the request headers, + // if the request has an "Expect: 100-continue" header. + defaultExpectContinueTimeout = 1 * time.Second ) type Proxy struct { @@ -32,9 +47,15 @@ type Proxy struct { func New(logger log.Logger, prefix string, endpoint *url.URL, opts ...Option) *Proxy { options := options{ - bufferCount: DefaultBufferCount, - bufferSizeBytes: DefaultBufferSizeBytes, - flushInterval: DefaultFlushInterval, + bufferCount: DefaultBufferCount, + bufferSizeBytes: DefaultBufferSizeBytes, + flushInterval: DefaultFlushInterval, + maxIdleConns: defaultMaxIdleConns, + timeout: defaultTimeout, + keepAlive: defaultKeepAlive, + idleConnTimeout: defaultIdleConnTimeout, + tlsHandshakeTimeout: defaultTLSHandshakeTimeout, + expectContinueTimeout: defaultExpectContinueTimeout, } for _, o := range opts { @@ -64,6 +85,18 @@ func New(logger log.Logger, prefix string, endpoint *url.URL, opts ...Option) *P Director: director, ErrorLog: stdErrLogger, FlushInterval: options.flushInterval, + Transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + Dial: (&net.Dialer{ + Timeout: options.timeout, + KeepAlive: options.keepAlive, + DualStack: true, + }).Dial, + MaxIdleConns: options.maxIdleConns, + IdleConnTimeout: options.idleConnTimeout, + TLSHandshakeTimeout: options.tlsHandshakeTimeout, + ExpectContinueTimeout: options.expectContinueTimeout, + }, } return &Proxy{logger: logger, reverseProxy: &rev} diff --git a/internal/server/option.go b/internal/server/option.go index c315eb340..fe1d5c551 100644 --- a/internal/server/option.go +++ b/internal/server/option.go @@ -9,9 +9,13 @@ import ( ) type options struct { - gracePeriod time.Duration - timeout time.Duration - tlsConfig *tls.Config + gracePeriod time.Duration + timeout time.Duration + requestTimeout time.Duration + readTimeout time.Duration + writeTimeout time.Duration + + tlsConfig *tls.Config metricsUIEndpoint *url.URL metricsReadEndpoint *url.URL @@ -34,24 +38,38 @@ func (f optionFunc) apply(o *options) { f(o) } -// WithGracePeriod TODO +// WithGracePeriod sets graceful shutdown period for the server. func WithGracePeriod(t time.Duration) Option { return optionFunc(func(o *options) { o.gracePeriod = t }) } -// WithListen TODO +// WithListen sets the port to listen for the server. func WithListen(s string) Option { return optionFunc(func(o *options) { o.listen = s }) } -// WithTimeout TODO -func WithTimeout(t time.Duration) Option { +// WithRequestTimeout sets the timeout duration for an individual request. +func WithRequestTimeout(t time.Duration) Option { + return optionFunc(func(o *options) { + o.requestTimeout = t + }) +} + +// WithReadTimeout sets the read timeout duration for the underlying HTTP server. +func WithReadTimeout(t time.Duration) Option { + return optionFunc(func(o *options) { + o.readTimeout = t + }) +} + +// WithWriteTimeout sets the write timeout duration for the underlying HTTP server. +func WithWriteTimeout(t time.Duration) Option { return optionFunc(func(o *options) { - o.timeout = t + o.writeTimeout = t }) } @@ -69,28 +87,28 @@ func WithMetricUIEndpoint(u *url.URL) Option { }) } -// WithMetricReadEndpoint TODO +// WithMetricReadEndpoint sets the URL to proxy metrics read request to. func WithMetricReadEndpoint(u *url.URL) Option { return optionFunc(func(o *options) { o.metricsReadEndpoint = u }) } -// WithMetricWriteEndpoint TODO +// WithMetricWriteEndpoint sets the URL to proxy metrics write request to. func WithMetricWriteEndpoint(u *url.URL) Option { return optionFunc(func(o *options) { o.metricsWriteEndpoint = u }) } -// WithProfile TODO +// WithProfile sets the option to enable/disable profiler endpoint. func WithProfile(p bool) Option { return optionFunc(func(o *options) { o.profile = p }) } -// WithProxyOptions TODO +// WithProxyOptions sets the proxy options fot the underlying reverse proxy. func WithProxyOptions(opts ...proxy.Option) Option { return optionFunc(func(o *options) { o.proxyOptions = opts diff --git a/internal/server/server.go b/internal/server/server.go index 1ed76a9a0..be711ff1b 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -17,10 +17,19 @@ import ( "github.com/prometheus/client_golang/prometheus/promhttp" ) +// DefaultGracePeriod is the default value of the duration gracefully shuts down the server without interrupting any active connections. const DefaultGracePeriod = 5 * time.Second -const DefaultTimeout = 5 * time.Minute -// Server TODO +// DefaultRequestTimeout is the default value of the timeout duration per request. +const DefaultRequestTimeout = 2 * time.Minute + +// DefaultReadTimeout is the default value of the maximum duration for reading the entire request, including the body. +const DefaultReadTimeout = 2 * time.Minute + +// DefaultWriteTimeout is the default value of the maximum duration before timing out writes of the response. +const DefaultWriteTimeout = 2 * time.Minute + +// Server defines parameters for running an HTTP server. type Server struct { logger log.Logger prober *prober.Prober @@ -29,7 +38,7 @@ type Server struct { opts options } -// New creates a new Server +// New creates a new Server. func New(logger log.Logger, reg *prometheus.Registry, opts ...Option) Server { options := options{ gracePeriod: DefaultGracePeriod, @@ -45,7 +54,7 @@ func New(logger log.Logger, reg *prometheus.Registry, opts ...Option) Server { r.Use(middleware.RealIP) r.Use(middleware.Recoverer) r.Use(middleware.StripSlashes) - r.Use(middleware.Timeout(options.timeout)) + r.Use(middleware.Timeout(options.requestTimeout)) if options.profile { r.Mount("/debug", middleware.Profiler()) @@ -109,15 +118,17 @@ func New(logger log.Logger, reg *prometheus.Registry, opts ...Option) Server { logger: logger, prober: p, srv: &http.Server{ - Addr: options.listen, - Handler: r, - TLSConfig: options.tlsConfig, + Addr: options.listen, + Handler: r, + TLSConfig: options.tlsConfig, + ReadTimeout: options.readTimeout, + WriteTimeout: options.writeTimeout, }, opts: options, } } -// ListenAndServe TODO +// ListenAndServe listens on the TCP network address and handles connections with given server configuration. func (s *Server) ListenAndServe() error { level.Info(s.logger).Log("msg", "starting the HTTP server", "address", s.opts.listen) s.prober.Ready() @@ -130,7 +141,7 @@ func (s *Server) ListenAndServe() error { return s.srv.ListenAndServe() } -// Shutdown TODO +// Shutdown gracefully shuts down the server. func (s *Server) Shutdown(err error) { s.prober.NotReady(err) diff --git a/main.go b/main.go index e0a6d6da6..b5b2a0f1e 100644 --- a/main.go +++ b/main.go @@ -30,11 +30,8 @@ type config struct { logLevel string logFormat string - listen string - gracePeriod time.Duration - timeout time.Duration - debug debugConfig + server serverConfig tls tlsConfig proxy proxyConfig metrics metricsConfig @@ -46,6 +43,14 @@ type debugConfig struct { name string } +type serverConfig struct { + listen string + gracePeriod time.Duration + requestTimeout time.Duration + readTimeout time.Duration + writeTimeout time.Duration +} + type tlsConfig struct { certFile string keyFile string @@ -163,9 +168,11 @@ func exec(logger log.Logger, reg *prometheus.Registry, cfg config) error { srv := server.New( logger, reg, - server.WithListen(cfg.listen), - server.WithGracePeriod(cfg.gracePeriod), - server.WithTimeout(cfg.timeout), + server.WithListen(cfg.server.listen), + server.WithGracePeriod(cfg.server.gracePeriod), + server.WithRequestTimeout(cfg.server.requestTimeout), + server.WithReadTimeout(cfg.server.readTimeout), + server.WithWriteTimeout(cfg.server.writeTimeout), server.WithTLSConfig(tlsConfig), server.WithProfile(os.Getenv("PROFILE") != ""), server.WithMetricUIEndpoint(cfg.metrics.uiEndpoint), @@ -203,12 +210,16 @@ func parseFlags(logger log.Logger) (config, error) { "The log filtering level. Options: 'error', 'warn', 'info', 'debug'.") flag.StringVar(&cfg.logFormat, "log.format", internal.LogFormatLogfmt, "The log format to use. Options: 'logfmt', 'json'.") - flag.StringVar(&cfg.listen, "web.listen", ":8080", + flag.StringVar(&cfg.server.listen, "web.listen", ":8080", "The address on which internal server runs.") - flag.DurationVar(&cfg.gracePeriod, "web.grace-period", server.DefaultGracePeriod, + flag.DurationVar(&cfg.server.gracePeriod, "web.grace-period", server.DefaultGracePeriod, "The time to wait after an OS interrupt received.") - flag.DurationVar(&cfg.timeout, "web.timeout", server.DefaultTimeout, + flag.DurationVar(&cfg.server.requestTimeout, "web.timeout", server.DefaultRequestTimeout, "The maximum duration before timing out the request, and closing idle connections.") + flag.DurationVar(&cfg.server.readTimeout, "web.timeout.read", server.DefaultReadTimeout, + "The maximum duration before reading the entire request, including the body.") + flag.DurationVar(&cfg.server.writeTimeout, "web.timeout.write", server.DefaultWriteTimeout, + "The maximum duration before timing out writes of the response.") flag.StringVar(&rawMetricsReadEndpoint, "metrics.read.endpoint", "", "The endpoint against which to send read requests for metrics. It used as a fallback to 'query.endpoint' and 'query-range.endpoint'.") flag.StringVar(&rawMetricsUIEndpoint, "metrics.ui.endpoint", "",