Skip to content

Commit

Permalink
Refactor IdleTracker to handle StateIdle transitions
Browse files Browse the repository at this point in the history
* Remove stutter naming for package and types
* Stop treating StateIdle the same as StateClosed, rather transitions to
  StateIdle will keep API timeout window open
* Remove redundate code

Fixes containers#7826

Signed-off-by: Jhon Honce <jhonce@redhat.com>
  • Loading branch information
jwhonce committed Sep 29, 2020
1 parent 2ee415b commit f03d470
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 91 deletions.
6 changes: 3 additions & 3 deletions pkg/api/handlers/compat/containers_attach.go
Expand Up @@ -6,7 +6,7 @@ import (
"github.com/containers/podman/v2/libpod"
"github.com/containers/podman/v2/libpod/define"
"github.com/containers/podman/v2/pkg/api/handlers/utils"
"github.com/containers/podman/v2/pkg/api/server/idletracker"
"github.com/containers/podman/v2/pkg/api/server/idle"
"github.com/gorilla/schema"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -92,7 +92,7 @@ func AttachContainer(w http.ResponseWriter, r *http.Request) {
return
}

idleTracker := r.Context().Value("idletracker").(*idletracker.IdleTracker)
idleTracker := r.Context().Value("idletracker").(*idle.Tracker)
hijackChan := make(chan bool, 1)

// Perform HTTP attach.
Expand All @@ -109,7 +109,7 @@ func AttachContainer(w http.ResponseWriter, r *http.Request) {
// We do need to tell the idle tracker that the
// connection has been closed, though. We can guarantee
// that is true after HTTPAttach exits.
idleTracker.TrackHijackedClosed()
idleTracker.Close()
} else {
// A hijack was not successfully completed. We need to
// report the error normally.
Expand Down
6 changes: 3 additions & 3 deletions pkg/api/handlers/compat/exec.go
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/containers/podman/v2/libpod/define"
"github.com/containers/podman/v2/pkg/api/handlers"
"github.com/containers/podman/v2/pkg/api/handlers/utils"
"github.com/containers/podman/v2/pkg/api/server/idletracker"
"github.com/containers/podman/v2/pkg/api/server/idle"
"github.com/containers/podman/v2/pkg/specgen/generate"
"github.com/gorilla/mux"
"github.com/pkg/errors"
Expand Down Expand Up @@ -174,7 +174,7 @@ func ExecStartHandler(w http.ResponseWriter, r *http.Request) {
return
}

idleTracker := r.Context().Value("idletracker").(*idletracker.IdleTracker)
idleTracker := r.Context().Value("idletracker").(*idle.Tracker)
hijackChan := make(chan bool, 1)

if err := sessionCtr.ExecHTTPStartAndAttach(sessionID, r, w, nil, nil, nil, hijackChan); err != nil {
Expand All @@ -186,7 +186,7 @@ func ExecStartHandler(w http.ResponseWriter, r *http.Request) {
// We do need to tell the idle tracker that the
// connection has been closed, though. We can guarantee
// that is true after HTTPAttach exits.
idleTracker.TrackHijackedClosed()
idleTracker.Close()
} else {
// A hijack was not successfully completed. We need to
// report the error normally.
Expand Down
96 changes: 96 additions & 0 deletions pkg/api/server/idle/tracker.go
@@ -0,0 +1,96 @@
package idle

import (
"net"
"net/http"
"sync"
"time"

"github.com/sirupsen/logrus"
)

// Tracker holds the state for the server's idle tracking
type Tracker struct {
// Duration is the API idle window
Duration time.Duration
hijacked int // count of active connections managed by handlers
managed map[net.Conn]struct{} // set of active connections managed by http package
mux sync.Mutex // protect managed map
timer *time.Timer
total int // total number of connections made to this server instance
}

// NewTracker creates and initializes a new Tracker object
// For best behavior, duration should be 2x http idle connection timeout
func NewTracker(idle time.Duration) *Tracker {
return &Tracker{
managed: make(map[net.Conn]struct{}),
Duration: idle,
timer: time.NewTimer(idle),
}
}

// ConnState is called on HTTP connection state changes.
// - Once StateHijacked, StateClose is _NOT_ called on that connection
// - There are two "idle" timeouts, the http idle connection (not to be confused with the TCP/IP idle socket timeout)
// and the API idle window. The caller should set the http idle timeout to 2x the time provided to NewTacker() which
// is the API idle window.
func (t *Tracker) ConnState(conn net.Conn, state http.ConnState) {
t.mux.Lock()
defer t.mux.Unlock()

logrus.Debugf("IdleTracker %p:%v %dm+%dh/%dt connection(s)", conn, state, len(t.managed), t.hijacked, t.TotalConnections())
switch state {
case http.StateNew, http.StateActive:
// stop the API timer when the server transitions any connection to an "active" state
t.managed[conn] = struct{}{}
t.timer.Stop()
t.total++
case http.StateHijacked:
// hijacked connections should call Close() when finished.
// Note: If a handler hijack's a connection and then doesn't Close() it,
// the API timer will not fire and the server will _NOT_ timeout.
delete(t.managed, conn)
t.hijacked++
case http.StateIdle:
// When any connection goes into the http idle state, we know:
// - we have an active connection
// - the API timer should not be counting down (See case StateNew/StateActive)
break
case http.StateClosed:
oldActive := t.ActiveConnections()

// Either the server or a hijacking handler has closed the http connection to a client
if _, found := t.managed[conn]; found {
delete(t.managed, conn)
} else {
t.hijacked-- // guarded by t.mux above
}

// Transitioned from any "active" connection to no connections
if oldActive > 0 && t.ActiveConnections() == 0 {
t.timer.Stop() // See library source for Reset() issues and why they are not fixed
t.timer.Reset(t.Duration) // Restart the API window timer
}
}
}

// Close is used to update Tracker that a StateHijacked connection has been closed by handler (StateClosed)
func (t *Tracker) Close() {
t.ConnState(nil, http.StateClosed)
}

// ActiveConnections returns the number of current managed or StateHijacked connections
func (t *Tracker) ActiveConnections() int {
return len(t.managed) + t.hijacked
}

// TotalConnections returns total number of connections made to this instance of the service
func (t *Tracker) TotalConnections() int {
return t.total
}

// Done is called when idle timer has expired
func (t *Tracker) Done() <-chan time.Time {
return t.timer.C
}
74 changes: 0 additions & 74 deletions pkg/api/server/idletracker/idletracker.go

This file was deleted.

22 changes: 11 additions & 11 deletions pkg/api/server/server.go
Expand Up @@ -16,7 +16,7 @@ import (

"github.com/containers/podman/v2/libpod"
"github.com/containers/podman/v2/pkg/api/handlers"
"github.com/containers/podman/v2/pkg/api/server/idletracker"
"github.com/containers/podman/v2/pkg/api/server/idle"
"github.com/coreos/go-systemd/v22/activation"
"github.com/coreos/go-systemd/v22/daemon"
"github.com/gorilla/mux"
Expand All @@ -26,14 +26,14 @@ import (
)

type APIServer struct {
http.Server // The HTTP work happens here
*schema.Decoder // Decoder for Query parameters to structs
context.Context // Context to carry objects to handlers
*libpod.Runtime // Where the real work happens
net.Listener // mux for routing HTTP API calls to libpod routines
context.CancelFunc // Stop APIServer
idleTracker *idletracker.IdleTracker // Track connections to support idle shutdown
pprof *http.Server // Sidecar http server for providing performance data
http.Server // The HTTP work happens here
*schema.Decoder // Decoder for Query parameters to structs
context.Context // Context to carry objects to handlers
*libpod.Runtime // Where the real work happens
net.Listener // mux for routing HTTP API calls to libpod routines
context.CancelFunc // Stop APIServer
idleTracker *idle.Tracker // Track connections to support idle shutdown
pprof *http.Server // Sidecar http server for providing performance data
}

// Number of seconds to wait for next request, if exceeded shutdown server
Expand Down Expand Up @@ -70,13 +70,13 @@ func newServer(runtime *libpod.Runtime, duration time.Duration, listener *net.Li
}

router := mux.NewRouter().UseEncodedPath()
idle := idletracker.NewIdleTracker(duration)
idle := idle.NewTracker(duration)

server := APIServer{
Server: http.Server{
Handler: router,
ReadHeaderTimeout: 20 * time.Second,
IdleTimeout: duration,
IdleTimeout: duration * 2,
ConnState: idle.ConnState,
ErrorLog: log.New(logrus.StandardLogger().Out, "", 0),
},
Expand Down

0 comments on commit f03d470

Please sign in to comment.