Skip to content

Commit

Permalink
Timing fixes
Browse files Browse the repository at this point in the history 
- Improved timing code to be much cleaner
- Add IdleTimeout
- Remove obsolete optimization (was fixed in Go 2 years ago: golang/go#15133 (comment))
  • Loading branch information
@erikdubbelboer
erikdubbelboer committed Apr 21, 2019
1 parent ed16dc0 commit 3bfa86f
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 161 deletions.
187 changes: 71 additions & 116 deletions server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,19 +199,26 @@ type Server struct {
// Default buffer size is used if not set.
WriteBufferSize int

// Maximum duration for reading the full request (including body).
//
// This also limits the maximum duration for idle keep-alive
// connections.
// ReadTimeout is the amount of time allowed to read
// the full request including body. The connection's read
// deadline is reset when the connection opens, or for
// keep-alive connections after the first byte has been read.
//
// By default request read timeout is unlimited.
ReadTimeout time.Duration

// Maximum duration for writing the full response (including body).
// WriteTimeout is the maximum duration before timing out
// writes of the response. It is reset after the request handler
// has returned.
//
// By default response write timeout is unlimited.
WriteTimeout time.Duration

// IdleTimeout is the maximum amount of time to wait for the
// next request when keep-alives are enabled. If IdleTimeout
// is zero, the value of ReadTimeout is used.
IdleTimeout time.Duration

// Maximum number of concurrent client connections allowed per IP.
//
// By default unlimited number of concurrent connections
Expand All @@ -226,15 +233,8 @@ type Server struct {
// By default unlimited number of requests may be served per connection.
MaxRequestsPerConn int

// Maximum keep-alive connection lifetime.
//
// The server closes keep-alive connection after its' lifetime
// expiration.
//
// See also ReadTimeout for limiting the duration of idle keep-alive
// connections.
//
// By default keep-alive connection lifetime is unlimited.
// MaxKeepaliveDuration is a no-op and only left here for backwards compatibility.
// Deprecated: Use IdleTimeout instead.
MaxKeepaliveDuration time.Duration

// Whether to enable tcp keep-alive connections.
Expand Down Expand Up @@ -353,7 +353,6 @@ type Server struct {
readerPool sync.Pool
writerPool sync.Pool
hijackConnPool sync.Pool
bytePool sync.Pool

// We need to know our listener so we can close it in Shutdown().
ln net.Listener
Expand Down Expand Up @@ -463,8 +462,6 @@ type RequestCtx struct {

userValues userData

lastReadDuration time.Duration

connID uint64
connRequestNum uint64
connTime time.Time
Expand Down Expand Up @@ -657,10 +654,9 @@ type ctxLogger struct {
}

func (cl *ctxLogger) Printf(format string, args ...interface{}) {
ctxLoggerLock.Lock()
msg := fmt.Sprintf(format, args...)
ctx := cl.ctx
cl.logger.Printf("%.3f %s - %s", time.Since(ctx.Time()).Seconds(), ctx.String(), msg)
ctxLoggerLock.Lock()
cl.logger.Printf("%.3f %s - %s", time.Since(cl.ctx.ConnTime()).Seconds(), cl.ctx.String(), msg)
ctxLoggerLock.Unlock()
}

Expand Down Expand Up @@ -1711,10 +1707,6 @@ var (
// ErrConcurrencyLimit may be returned from ServeConn if the number
// of concurrently served connections exceeds Server.Concurrency.
ErrConcurrencyLimit = errors.New("cannot serve the connection because Server.Concurrency concurrent connections are served")

// ErrKeepaliveTimeout is returned from ServeConn
// if the connection lifetime exceeds MaxKeepaliveDuration.
ErrKeepaliveTimeout = errors.New("exceeded MaxKeepaliveDuration")
)

// ServeConn serves HTTP requests from the given connection.
Expand Down Expand Up @@ -1799,6 +1791,13 @@ func nextConnID() uint64 {
// See Server.MaxRequestBodySize for details.
const DefaultMaxRequestBodySize = 4 * 1024 * 1024

func (s *Server) idleTimeout() time.Duration {
if s.IdleTimeout != 0 {
return s.IdleTimeout
}
return s.ReadTimeout
}

func (s *Server) serveConn(c net.Conn) error {
defer atomic.AddInt32(&s.open, -1)

Expand All @@ -1817,8 +1816,7 @@ func (s *Server) serveConn(c net.Conn) error {
}
connRequestNum := uint64(0)
connID := nextConnID()
currentTime := time.Now()
connTime := currentTime
connTime := time.Now()
maxRequestBodySize := s.MaxRequestBodySize
if maxRequestBodySize <= 0 {
maxRequestBodySize = DefaultMaxRequestBodySize
Expand All @@ -1835,37 +1833,55 @@ func (s *Server) serveConn(c net.Conn) error {
timeoutResponse *Response
hijackHandler HijackHandler

lastReadDeadlineTime time.Time
lastWriteDeadlineTime time.Time

connectionClose bool
isHTTP11 bool

reqReset bool
)
for {
connRequestNum++
ctx.time = currentTime

if s.ReadTimeout > 0 || s.MaxKeepaliveDuration > 0 {
lastReadDeadlineTime = s.updateReadDeadline(c, ctx, lastReadDeadlineTime)
if lastReadDeadlineTime.IsZero() {
err = ErrKeepaliveTimeout
break
// If this is a keep-alive connection set the idle timeout.
if connRequestNum > 1 {
if d := s.idleTimeout(); d > 0 {
if err := c.SetReadDeadline(time.Now().Add(d)); err != nil {
panic(fmt.Sprintf("BUG: error in SetReadDeadline(%s): %s", d, err))
}
}
}

if !(s.ReduceMemoryUsage || ctx.lastReadDuration > time.Second) || br != nil {
if !s.ReduceMemoryUsage || br != nil {
if br == nil {
br = acquireReader(ctx)
}

// If this is a keep-alive connection we want to try and read the first bytes
// within the idle time.
if connRequestNum > 1 {
var b []byte
b, err = br.Peek(4)
if len(b) == 0 {
// If reading from a keep-alive connection returns nothing it means
// the connection was closed (either timeout or from the other side).
err = errNothingRead
}
}
} else {
// If this is a keep-alive connection acquireByteReader will try to peek
// a couple of bytes already so the idle timeout will already be used.
br, err = acquireByteReader(&ctx)
}

ctx.Request.isTLS = isTLS
ctx.Response.Header.noDefaultContentType = s.NoDefaultContentType

if err == nil {
if s.ReadTimeout > 0 {
if err := c.SetReadDeadline(time.Now().Add(s.ReadTimeout)); err != nil {
panic(fmt.Sprintf("BUG: error in SetReadDeadline(%s): %s", s.ReadTimeout, err))
}
}

if s.DisableHeaderNamesNormalizing {
ctx.Request.Header.DisableNormalizing()
ctx.Response.Header.DisableNormalizing()
Expand All @@ -1882,9 +1898,6 @@ func (s *Server) serveConn(c net.Conn) error {
}
}

currentTime = time.Now()
ctx.lastReadDuration = currentTime.Sub(ctx.time)

if err != nil {
if err == io.EOF {
err = nil
Expand Down Expand Up @@ -1941,7 +1954,7 @@ func (s *Server) serveConn(c net.Conn) error {
}
ctx.connID = connID
ctx.connRequestNum = connRequestNum
ctx.time = currentTime
ctx.time = time.Now()
s.Handler(ctx)

timeoutResponse = ctx.timeoutResponse
Expand Down Expand Up @@ -1969,8 +1982,10 @@ func (s *Server) serveConn(c net.Conn) error {
ctx.SetConnectionClose()
}

if s.WriteTimeout > 0 || s.MaxKeepaliveDuration > 0 {
lastWriteDeadlineTime = s.updateWriteDeadline(c, ctx, lastWriteDeadlineTime)
if s.WriteTimeout > 0 {
if err := c.SetWriteDeadline(time.Now().Add(s.WriteTimeout)); err != nil {
panic(fmt.Sprintf("BUG: error in SetWriteDeadline(%s): %s", s.WriteTimeout, err))
}
}

connectionClose = connectionClose || ctx.Response.ConnectionClose()
Expand Down Expand Up @@ -2019,8 +2034,8 @@ func (s *Server) serveConn(c net.Conn) error {
hjr = br
br = nil

// br may point to ctx.fbr, so do not return ctx into pool.
ctx = s.acquireCtx(c)
// br may point to ctx.fbr, so do not return ctx into pool below.
ctx = nil
}
if bw != nil {
err = bw.Flush()
Expand All @@ -2038,7 +2053,6 @@ func (s *Server) serveConn(c net.Conn) error {
break
}

currentTime = time.Now()
s.setState(c, StateIdle)

if atomic.LoadInt32(&s.stop) == 1 {
Expand All @@ -2053,13 +2067,15 @@ func (s *Server) serveConn(c net.Conn) error {
if bw != nil {
releaseWriter(s, bw)
}
// in unexpected cases the for loop will break
// before request reset call. in such cases, call it before
// release to fix #548
if !reqReset {
ctx.Request.Reset()
if ctx != nil {
// in unexpected cases the for loop will break
// before request reset call. in such cases, call it before
// release to fix #548
if !reqReset {
ctx.Request.Reset()
}
s.releaseCtx(ctx)
}
s.releaseCtx(ctx)
return err
}

Expand All @@ -2069,59 +2085,6 @@ func (s *Server) setState(nc net.Conn, state ConnState) {
}
}

func (s *Server) updateReadDeadline(c net.Conn, ctx *RequestCtx, lastDeadlineTime time.Time) time.Time {
readTimeout := s.ReadTimeout
currentTime := ctx.time
if s.MaxKeepaliveDuration > 0 {
connTimeout := s.MaxKeepaliveDuration - currentTime.Sub(ctx.connTime)
if connTimeout <= 0 {
return zeroTime
}
if connTimeout < readTimeout {
readTimeout = connTimeout
}
}

// Optimization: update read deadline only if more than 25%
// of the last read deadline exceeded.
// See https://github.com/golang/go/issues/15133 for details.
if currentTime.Sub(lastDeadlineTime) > (readTimeout >> 2) {
if err := c.SetReadDeadline(currentTime.Add(readTimeout)); err != nil {
panic(fmt.Sprintf("BUG: error in SetReadDeadline(%s): %s", readTimeout, err))
}
lastDeadlineTime = currentTime
}
return lastDeadlineTime
}

func (s *Server) updateWriteDeadline(c net.Conn, ctx *RequestCtx, lastDeadlineTime time.Time) time.Time {
writeTimeout := s.WriteTimeout
if s.MaxKeepaliveDuration > 0 {
connTimeout := s.MaxKeepaliveDuration - time.Since(ctx.connTime)
if connTimeout <= 0 {
// MaxKeepAliveDuration exceeded, but let's try sending response anyway
// in 100ms with 'Connection: close' header.
ctx.SetConnectionClose()
connTimeout = 100 * time.Millisecond
}
if connTimeout < writeTimeout {
writeTimeout = connTimeout
}
}

// Optimization: update write deadline only if more than 25%
// of the last write deadline exceeded.
// See https://github.com/golang/go/issues/15133 for details.
currentTime := time.Now()
if currentTime.Sub(lastDeadlineTime) > (writeTimeout >> 2) {
if err := c.SetWriteDeadline(currentTime.Add(writeTimeout)); err != nil {
panic(fmt.Sprintf("BUG: error in SetWriteDeadline(%s): %s", writeTimeout, err))
}
lastDeadlineTime = currentTime
}
return lastDeadlineTime
}

func hijackConnHandler(r io.Reader, c net.Conn, s *Server, h HijackHandler) {
hjc := s.acquireHijackConn(r, c)
h(hjc)
Expand Down Expand Up @@ -2209,24 +2172,17 @@ func acquireByteReader(ctxP **RequestCtx) (*bufio.Reader, error) {
ctx := *ctxP
s := ctx.s
c := ctx.c
t := ctx.time
s.releaseCtx(ctx)

// Make GC happy, so it could garbage collect ctx
// while we waiting for the next request.
ctx = nil
*ctxP = nil

v := s.bytePool.Get()
if v == nil {
v = make([]byte, 1)
}
b := v.([]byte)
n, err := c.Read(b)
ch := b[0]
s.bytePool.Put(v)
var b [1]byte
n, err := c.Read(b[:])

ctx = s.acquireCtx(c)
ctx.time = t
*ctxP = ctx
if err != nil {
// Treat all errors as EOF on unsuccessful read
Expand All @@ -2238,7 +2194,7 @@ func acquireByteReader(ctxP **RequestCtx) (*bufio.Reader, error) {
}

ctx.fbr.c = c
ctx.fbr.ch = ch
ctx.fbr.ch = b[0]
ctx.fbr.byteRead = false
r := acquireReader(ctx)
r.Reset(&ctx.fbr)
Expand Down Expand Up @@ -2310,7 +2266,6 @@ func (ctx *RequestCtx) Init2(conn net.Conn, logger Logger, reduceMemoryUsage boo
ctx.s = fakeServer
ctx.connRequestNum = 0
ctx.connTime = time.Now()
ctx.time = ctx.connTime

keepBodyBuffer := !reduceMemoryUsage
ctx.Request.keepBodyBuffer = keepBodyBuffer
Expand Down
Loading

0 comments on commit 3bfa86f

Please sign in to comment.