From 3052b479b7ebeed39c3d2ae706dea7da985c5255 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Thu, 7 May 2020 14:10:40 +0300 Subject: [PATCH] lib/httpserver: reduce typical duration for http server graceful shutdown Previously the duration for graceful shutdown for http server could take more than a minute because of imporperly set timeouts in setNetworkTimeout. Now typical duration for graceful shutdown should be reduced to less than 5 seconds. --- app/vmstorage/transport/server.go | 8 ------- lib/httpserver/httpserver.go | 22 +++++-------------- lib/ingestserver/opentsdbhttp/server.go | 8 ++++--- lib/netutil/conn.go | 29 ------------------------- lib/netutil/tcplistener.go | 17 --------------- 5 files changed, 10 insertions(+), 74 deletions(-) diff --git a/app/vmstorage/transport/server.go b/app/vmstorage/transport/server.go index 9589220d4..28be0fecb 100644 --- a/app/vmstorage/transport/server.go +++ b/app/vmstorage/transport/server.go @@ -86,14 +86,6 @@ func NewServer(vminsertAddr, vmselectAddr string, storage *storage.Storage) (*Se if err := encoding.CheckPrecisionBits(uint8(*precisionBits)); err != nil { return nil, fmt.Errorf("invalid -precisionBits: %s", err) } - - // Set network-level write timeouts to reasonable values in order to protect - // from broken networks. - // Do not set read timeouts, since they are managed separately - - // search for SetReadDeadline in this file. - vminsertLN.WriteTimeout = time.Minute - vmselectLN.WriteTimeout = time.Minute - s := &Server{ storage: storage, diff --git a/lib/httpserver/httpserver.go b/lib/httpserver/httpserver.go index 6b0add091..7b9c4c4e5 100644 --- a/lib/httpserver/httpserver.go +++ b/lib/httpserver/httpserver.go @@ -56,33 +56,21 @@ func Serve(addr string, rh RequestHandler) { if err != nil { logger.Panicf("FATAL: cannot start http server at %s: %s", addr, err) } - setNetworkTimeouts(ln) serveWithListener(addr, ln, rh) } -func setNetworkTimeouts(ln *netutil.TCPListener) { - // Set network-level read and write timeouts to reasonable values - // in order to protect from DoS or broken networks. - // Application-level timeouts must be set by the authors of request handlers. - // - // The read timeout limits the life of idle connection - ln.ReadTimeout = time.Minute - ln.WriteTimeout = time.Minute -} - func serveWithListener(addr string, ln net.Listener, rh RequestHandler) { s := &http.Server{ Handler: gzipHandler(rh), - // Disable http/2 + // Disable http/2, since it doesn't give any advantages for VictoriaMetrics services. TLSNextProto: make(map[string]func(*http.Server, *tls.Conn, http.Handler)), - // Do not set ReadTimeout and WriteTimeout here. - // Network-level timeouts are set in setNetworkTimeouts. - // Application-level timeouts must be set in the app. + ReadHeaderTimeout: 5 * time.Second, + IdleTimeout: time.Minute, - // Do not set IdleTimeout, since it is equivalent to read timeout - // set in setNetworkTimeouts. + // Do not set ReadTimeout and WriteTimeout here, + // since these timeouts must be controlled by request handlers. ErrorLog: logger.StdErrorLogger(), } diff --git a/lib/ingestserver/opentsdbhttp/server.go b/lib/ingestserver/opentsdbhttp/server.go index eeb83d9c3..d3d9384db 100644 --- a/lib/ingestserver/opentsdbhttp/server.go +++ b/lib/ingestserver/opentsdbhttp/server.go @@ -43,9 +43,11 @@ func MustStart(addr string, insertHandler func(req *http.Request) error) *Server func MustServe(ln net.Listener, insertHandler func(req *http.Request) error) *Server { h := newRequestHandler(insertHandler) hs := &http.Server{ - Handler: h, - ReadTimeout: 30 * time.Second, - WriteTimeout: 10 * time.Second, + Handler: h, + ReadHeaderTimeout: 5 * time.Second, + IdleTimeout: time.Minute, + // Do not set ReadTimeout and WriteTimeout here, + // since these timeouts must be controlled by request handler. } s := &Server{ s: hs, diff --git a/lib/netutil/conn.go b/lib/netutil/conn.go index 2bbb69d66..feb81452c 100644 --- a/lib/netutil/conn.go +++ b/lib/netutil/conn.go @@ -5,7 +5,6 @@ import ( "io" "net" "sync/atomic" - "time" "github.com/VictoriaMetrics/metrics" ) @@ -48,29 +47,12 @@ type statConn struct { closeCalls uint64 - readTimeout time.Duration - lastReadTime time.Time - - writeTimeout time.Duration - lastWriteTime time.Time - net.Conn cm *connMetrics } func (sc *statConn) Read(p []byte) (int, error) { - if sc.readTimeout > 0 { - t := time.Now() - if t.Sub(sc.lastReadTime) > sc.readTimeout>>4 { - d := t.Add(sc.readTimeout) - if err := sc.Conn.SetReadDeadline(d); err != nil { - // This error may occur when the client closes the connection before setting the deadline - return 0, err - } - } - } - n, err := sc.Conn.Read(p) sc.cm.readCalls.Inc() sc.cm.readBytes.Add(n) @@ -85,17 +67,6 @@ func (sc *statConn) Read(p []byte) (int, error) { } func (sc *statConn) Write(p []byte) (int, error) { - if sc.writeTimeout > 0 { - t := time.Now() - if t.Sub(sc.lastWriteTime) > sc.writeTimeout>>4 { - d := t.Add(sc.writeTimeout) - if err := sc.Conn.SetWriteDeadline(d); err != nil { - // This error may accour when the client closes the connection before setting the deadline - return 0, err - } - } - } - n, err := sc.Conn.Write(p) sc.cm.writeCalls.Inc() sc.cm.writtenBytes.Add(n) diff --git a/lib/netutil/tcplistener.go b/lib/netutil/tcplistener.go index 5ce16d606..267802c77 100644 --- a/lib/netutil/tcplistener.go +++ b/lib/netutil/tcplistener.go @@ -49,20 +49,6 @@ func getNetwork() string { // // It also gathers various stats for the accepted connections. type TCPListener struct { - // ReadTimeout is timeout for each Read call on accepted conns. - // - // By default it isn't set. - // - // Set ReadTimeout before calling Accept the first time. - ReadTimeout time.Duration - - // WriteTimeout is timeout for each Write call on accepted conns. - // - // By default it isn't set. - // - // Set WriteTimeout before calling Accept the first time. - WriteTimeout time.Duration - net.Listener accepts *metrics.Counter @@ -87,9 +73,6 @@ func (ln *TCPListener) Accept() (net.Conn, error) { } ln.conns.Inc() sc := &statConn{ - readTimeout: ln.ReadTimeout, - writeTimeout: ln.WriteTimeout, - Conn: conn, cm: &ln.connMetrics, }