]> Cypherpunks repositories - gostls13.git/commitdiff
net/http: add Transport.MaxIdleConns limit
authorBrad Fitzpatrick <bradfitz@golang.org>
Sat, 30 Apr 2016 22:17:26 +0000 (17:17 -0500)
committerBrad Fitzpatrick <bradfitz@golang.org>
Sun, 1 May 2016 00:12:20 +0000 (00:12 +0000)
The HTTP client had a limit for the maximum number of idle connections
per-host, but not a global limit.

This CLs adds a global idle connection limit too,
Transport.MaxIdleConns.

All idle conns are now also stored in a doubly-linked list. When there
are too many, the oldest one is closed.

Fixes #15461

Change-Id: I72abbc28d140c73cf50f278fa70088b45ae0deef
Reviewed-on: https://go-review.googlesource.com/22655
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/go/build/deps_test.go
src/net/http/export_test.go
src/net/http/transport.go
src/net/http/transport_test.go

index 67057a960bc1e4c79d37706d166c561c77c5ab82..8a8c4be2174af0dc7946cb3dd14e99b56a9de522 100644 (file)
@@ -360,7 +360,7 @@ var pkgDeps = map[string][]string{
        // HTTP, kingpin of dependencies.
        "net/http": {
                "L4", "NET", "OS",
-               "context", "compress/gzip", "crypto/tls",
+               "context", "compress/gzip", "container/list", "crypto/tls",
                "mime/multipart", "runtime/debug",
                "net/http/internal",
                "golang.org/x/net/http2/hpack",
index 4c6d9e361f4b97a749bd5ae2c76d74837aa595cb..d1baed896a4a10cd4faab67aaf127c15bbc1d81c 100644 (file)
@@ -9,6 +9,7 @@ package http
 
 import (
        "net"
+       "sort"
        "sync"
        "time"
 )
@@ -86,6 +87,7 @@ func (t *Transport) IdleConnKeysForTesting() (keys []string) {
        for key := range t.idleConn {
                keys = append(keys, key.String())
        }
+       sort.Strings(keys)
        return
 }
 
index f1e0560ab7ebeb8f8b3999cf19f4fd855aeed88e..b4d56ab69947dff98ca50217e6a8b1d4a3b26d7c 100644 (file)
@@ -12,6 +12,7 @@ package http
 import (
        "bufio"
        "compress/gzip"
+       "container/list"
        "context"
        "crypto/tls"
        "errors"
@@ -38,6 +39,7 @@ var DefaultTransport RoundTripper = &Transport{
                Timeout:   30 * time.Second,
                KeepAlive: 30 * time.Second,
        },
+       MaxIdleConns:          100,
        TLSHandshakeTimeout:   10 * time.Second,
        ExpectContinueTimeout: 1 * time.Second,
 }
@@ -69,6 +71,7 @@ type Transport struct {
        idleCount  int
        idleConn   map[connectMethodKey][]*persistConn
        idleConnCh map[connectMethodKey]chan *persistConn
+       idleLRU    connLRU
 
        reqMu       sync.Mutex
        reqCanceler map[*Request]func()
@@ -127,6 +130,10 @@ type Transport struct {
        // uncompressed.
        DisableCompression bool
 
+       // MaxIdleConns controls the maximum number of idle (keep-alive)
+       // connections across all hosts. Zero means no limit.
+       MaxIdleConns int
+
        // MaxIdleConnsPerHost, if non-zero, controls the maximum idle
        // (keep-alive) connections to keep per-host. If zero,
        // DefaultMaxIdleConnsPerHost is used.
@@ -455,6 +462,8 @@ func (t *Transport) CloseIdleConnections() {
        t.idleConn = nil
        t.idleConnCh = nil
        t.wantIdle = true
+       t.idleCount = 0
+       t.idleLRU = connLRU{}
        t.idleMu.Unlock()
        for _, conns := range m {
                for _, pconn := range conns {
@@ -555,6 +564,7 @@ var (
        errConnBroken         = errors.New("http: putIdleConn: connection is in bad state")
        errWantIdle           = errors.New("http: putIdleConn: CloseIdleConnections was called")
        errTooManyIdle        = errors.New("http: putIdleConn: too many idle connections")
+       errTooManyIdleHost    = errors.New("http: putIdleConn: too many idle connections for host")
        errCloseIdleConns     = errors.New("http: CloseIdleConnections called")
        errReadLoopExiting    = errors.New("http: persistConn.readLoop exiting")
        errServerClosedIdle   = errors.New("http: server closed idle conn")
@@ -566,6 +576,13 @@ func (t *Transport) putOrCloseIdleConn(pconn *persistConn) {
        }
 }
 
+func (t *Transport) maxIdleConnsPerHost() int {
+       if v := t.MaxIdleConnsPerHost; v != 0 {
+               return v
+       }
+       return DefaultMaxIdleConnsPerHost
+}
+
 // tryPutIdleConn adds pconn to the list of idle persistent connections awaiting
 // a new request.
 // If pconn is no longer needed or not in a good state, tryPutIdleConn returns
@@ -578,12 +595,8 @@ func (t *Transport) tryPutIdleConn(pconn *persistConn) error {
        if pconn.isBroken() {
                return errConnBroken
        }
-       key := pconn.cacheKey
-       max := t.MaxIdleConnsPerHost
-       if max == 0 {
-               max = DefaultMaxIdleConnsPerHost
-       }
        pconn.markReused()
+       key := pconn.cacheKey
 
        t.idleMu.Lock()
        defer t.idleMu.Unlock()
@@ -611,17 +624,22 @@ func (t *Transport) tryPutIdleConn(pconn *persistConn) error {
                t.idleConn = make(map[connectMethodKey][]*persistConn)
        }
        idles := t.idleConn[key]
-       if len(idles) >= max {
-               return errTooManyIdle
+       if len(idles) >= t.maxIdleConnsPerHost() {
+               return errTooManyIdleHost
        }
        for _, exist := range idles {
                if exist == pconn {
                        log.Fatalf("dup idle pconn %p in freelist", pconn)
                }
        }
-
        t.idleConn[key] = append(idles, pconn)
        t.idleCount++
+       t.idleLRU.add(pconn)
+       if t.MaxIdleConns != 0 && t.idleLRU.len() > t.MaxIdleConns {
+               oldest := t.idleLRU.removeOldest()
+               oldest.close(errTooManyIdle)
+               t.removeIdleConnLocked(oldest)
+       }
        pconn.idleAt = time.Now()
        return nil
 }
@@ -661,12 +679,13 @@ func (t *Transport) getIdleConn(cm connectMethod) (pconn *persistConn, idleSince
                        pconn = pconns[0]
                        delete(t.idleConn, key)
                } else {
-                       // 2 or more cached connections; pop last
-                       // TODO: queue?
+                       // 2 or more cached connections; use the most
+                       // recently used one.
                        pconn = pconns[len(pconns)-1]
                        t.idleConn[key] = pconns[:len(pconns)-1]
                }
                t.idleCount--
+               t.idleLRU.remove(pconn)
                if pconn.isBroken() {
                        // There is a tiny window where this is
                        // possible, between the connecting dying and
@@ -681,10 +700,15 @@ func (t *Transport) getIdleConn(cm connectMethod) (pconn *persistConn, idleSince
 
 // removeIdleConn marks pconn as dead.
 func (t *Transport) removeIdleConn(pconn *persistConn) {
-       key := pconn.cacheKey
        t.idleMu.Lock()
        defer t.idleMu.Unlock()
+       t.removeIdleConnLocked(pconn)
+}
 
+// t.idleMu must be held.
+func (t *Transport) removeIdleConnLocked(pconn *persistConn) {
+       t.idleLRU.remove(pconn)
+       key := pconn.cacheKey
        pconns, _ := t.idleConn[key]
        switch len(pconns) {
        case 0:
@@ -695,7 +719,6 @@ func (t *Transport) removeIdleConn(pconn *persistConn) {
                        delete(t.idleConn, key)
                }
        default:
-               // TODO(bradfitz): map into LRU element?
                for i, v := range pconns {
                        if v != pconn {
                                continue
@@ -1944,3 +1967,42 @@ func cloneTLSClientConfig(cfg *tls.Config) *tls.Config {
                Renegotiation:            cfg.Renegotiation,
        }
 }
+
+type connLRU struct {
+       ll *list.List // list.Element.Value type is of *persistConn
+       m  map[*persistConn]*list.Element
+}
+
+// addO adds pc to the head of the linked list.
+func (cl *connLRU) add(pc *persistConn) {
+       if cl.ll == nil {
+               cl.ll = list.New()
+               cl.m = make(map[*persistConn]*list.Element)
+       }
+       ele := cl.ll.PushFront(pc)
+       if _, ok := cl.m[pc]; ok {
+               panic("persistConn was already in LRU")
+       }
+       cl.m[pc] = ele
+}
+
+func (cl *connLRU) removeOldest() *persistConn {
+       ele := cl.ll.Back()
+       pc := ele.Value.(*persistConn)
+       cl.ll.Remove(ele)
+       delete(cl.m, pc)
+       return pc
+}
+
+// remove removes pc from cl.
+func (cl *connLRU) remove(pc *persistConn) {
+       if ele, ok := cl.m[pc]; ok {
+               cl.ll.Remove(ele)
+               delete(cl.m, pc)
+       }
+}
+
+// len returns the number of items in the cache.
+func (cl *connLRU) len() int {
+       return len(cl.m)
+}
index 3f6ab7b01b7af0c036bfeabeec7fd03a3d21c576..67f0b74ba05a26addf99b9153cb612715fb5db1d 100644 (file)
@@ -383,8 +383,8 @@ func TestTransportMaxPerHostIdleConns(t *testing.T) {
                }
        }))
        defer ts.Close()
-       maxIdleConns := 2
-       tr := &Transport{DisableKeepAlives: false, MaxIdleConnsPerHost: maxIdleConns}
+       maxIdleConnsPerHost := 2
+       tr := &Transport{DisableKeepAlives: false, MaxIdleConnsPerHost: maxIdleConnsPerHost}
        c := &Client{Transport: tr}
 
        // Start 3 outstanding requests and wait for the server to get them.
@@ -429,14 +429,14 @@ func TestTransportMaxPerHostIdleConns(t *testing.T) {
 
        resch <- "res2"
        <-donech
-       if e, g := 2, tr.IdleConnCountForTesting(cacheKey); e != g {
-               t.Errorf("after second response, expected %d idle conns; got %d", e, g)
+       if g, w := tr.IdleConnCountForTesting(cacheKey), 2; g != w {
+               t.Errorf("after second response, idle conns = %d; want %d", g, w)
        }
 
        resch <- "res3"
        <-donech
-       if e, g := maxIdleConns, tr.IdleConnCountForTesting(cacheKey); e != g {
-               t.Errorf("after third response, still expected %d idle conns; got %d", e, g)
+       if g, w := tr.IdleConnCountForTesting(cacheKey), maxIdleConnsPerHost; g != w {
+               t.Errorf("after third response, idle conns = %d; want %d", g, w)
        }
 }
 
@@ -3229,10 +3229,6 @@ func TestTransportEventTrace(t *testing.T) {
                        t.Errorf("unexpected DNS host lookup for %q", host)
                        return nil, nil
                }
-               if err != nil {
-                       t.Error(err)
-                       return nil, err
-               }
                return []net.IPAddr{net.IPAddr{IP: net.ParseIP(ip)}}, nil
        })
 
@@ -3291,6 +3287,61 @@ func TestTransportEventTrace(t *testing.T) {
        }
 }
 
+func TestTransportMaxIdleConns(t *testing.T) {
+       defer afterTest(t)
+       ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+               // No body for convenience.
+       }))
+       defer ts.Close()
+       tr := &Transport{
+               MaxIdleConns: 4,
+       }
+       defer tr.CloseIdleConnections()
+
+       ip, port, err := net.SplitHostPort(ts.Listener.Addr().String())
+       if err != nil {
+               t.Fatal(err)
+       }
+       c := &Client{Transport: tr}
+       ctx := context.WithValue(context.Background(), nettrace.LookupIPAltResolverKey{}, func(ctx context.Context, host string) ([]net.IPAddr, error) {
+               return []net.IPAddr{net.IPAddr{IP: net.ParseIP(ip)}}, nil
+       })
+
+       hitHost := func(n int) {
+               req, _ := NewRequest("GET", fmt.Sprintf("http://host-%d.dns-is-faked.golang:"+port, n), nil)
+               req = req.WithContext(ctx)
+               res, err := c.Do(req)
+               if err != nil {
+                       t.Fatal(err)
+               }
+               res.Body.Close()
+       }
+       for i := 0; i < 4; i++ {
+               hitHost(i)
+       }
+       want := []string{
+               "|http|host-0.dns-is-faked.golang:" + port,
+               "|http|host-1.dns-is-faked.golang:" + port,
+               "|http|host-2.dns-is-faked.golang:" + port,
+               "|http|host-3.dns-is-faked.golang:" + port,
+       }
+       if got := tr.IdleConnKeysForTesting(); !reflect.DeepEqual(got, want) {
+               t.Fatalf("idle conn keys mismatch.\n got: %q\nwant: %q\n", got, want)
+       }
+
+       // Now hitting the 5th host should kick out the first host:
+       hitHost(4)
+       want = []string{
+               "|http|host-1.dns-is-faked.golang:" + port,
+               "|http|host-2.dns-is-faked.golang:" + port,
+               "|http|host-3.dns-is-faked.golang:" + port,
+               "|http|host-4.dns-is-faked.golang:" + port,
+       }
+       if got := tr.IdleConnKeysForTesting(); !reflect.DeepEqual(got, want) {
+               t.Fatalf("idle conn keys mismatch after 5th host.\n got: %q\nwant: %q\n", got, want)
+       }
+}
+
 var errFakeRoundTrip = errors.New("fake roundtrip")
 
 type funcRoundTripper func()