Client keep-alives and hotcache changes

* Now Associating the transport with peer `httpGetter` so we take advantage of
  connection reuse.  This lowers the impact on DNS and improves preformance for
  high request volume low latency applications.
* Now always populating the hotcache. A more complex algorithm is unnecessary
  when the LRU cache will ensure the most used values remain in the cache. The
  evict code ensures the hotcache does not over crowd the maincache.

.travis.yml

@@ -9,9 +9,9 @@ script:
   - go test ./...
 
 go:
-  - 1.9.x
   - 1.10.x
   - 1.11.x
+  - 1.12.x
   - master
 
 cache:

CHANGELOG

@@ -6,8 +6,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [2.0.0] - 2019-05-30
 ### Changes
-* Now using golang standard `context.Context` instead of groupcache.Context.
-* http requests now respect `context.Context` done.
+* Now using golang standard `context.Context` instead of `groupcache.Context`.
+* HTTP requests made by `httpGetter` now respect `context.Context` done.
+* Moved `HTTPPool` config `Context` and `Transport` to `HTTPPoolOptions` for consist configuration.
+* Now Associating the transport with peer `httpGetter` so we take advantage of
+  connection reuse.  This lowers the impact on DNS and improves performance for
+  high request volume low latency applications.
+* Now always populating the hotcache. A more complex algorithm is unnecessary
+  when the LRU cache will ensure the most used values remain in the cache. The
+  evict code ensures the hotcache does not overcrowd the maincache.
 
 ## [1.3.0] - 2019-05-23
 ### Added

groupcache.go

@@ -27,7 +27,6 @@ package groupcache
 import (
 	"context"
 	"errors"
-	"math/rand"
 	"strconv"
 	"sync"
 	"sync/atomic"
@@ -376,12 +375,9 @@ func (g *Group) getFromPeer(ctx context.Context, peer ProtoGetter, key string) (
 	}
 
 	value := ByteView{b: res.Value, e: expire}
-	// TODO(bradfitz): use res.MinuteQps or something smart to
-	// conditionally populate hotCache.  For now just do it some
-	// percentage of the time.
-	if rand.Intn(10) == 0 {
-		g.populateCache(key, value, &g.hotCache)
-	}
+
+	// Always populate the hot cache
+	g.populateCache(key, value, &g.hotCache)
 	return value, nil
 }
 

groupcache_test.go

@@ -23,7 +23,6 @@ import (
 	"errors"
 	"fmt"
 	"hash/crc32"
-	"math/rand"
 	"reflect"
 	"sync"
 	"testing"
@@ -33,7 +32,7 @@ import (
 	"github.com/golang/protobuf/proto"
 
 	pb "github.com/mailgun/groupcache/groupcachepb"
-	testpb "github.com/mailgun/groupcache/testpb"
+	"github.com/mailgun/groupcache/testpb"
 )
 
 var (
@@ -289,7 +288,6 @@ func (p fakePeers) GetAll() []ProtoGetter {
 // tests that peers (virtual, in-process) are hit, and how much.
 func TestPeers(t *testing.T) {
 	once.Do(testSetup)
-	rand.Seed(123)
 	peer0 := &fakePeer{}
 	peer1 := &fakePeer{}
 	peer2 := &fakePeer{}
@@ -339,9 +337,9 @@ func TestPeers(t *testing.T) {
 	resetCacheSize(1 << 20)
 	run("base", 200, "localHits = 49, peers = 51 49 51")
 
-	// Verify cache was hit.  All localHits are gone, and some of
-	// the peer hits (the ones randomly selected to be maybe hot)
-	run("cached_base", 200, "localHits = 0, peers = 49 47 48")
+	// Verify cache was hit.  All localHits and peers are gone as the hotCache has
+	// the data we need
+	run("cached_base", 200, "localHits = 0, peers = 0 0 0")
 	resetCacheSize(0)
 
 	// With one of the peers being down.

http.go

@@ -38,16 +38,6 @@ const defaultReplicas = 50
 
 // HTTPPool implements PeerPicker for a pool of HTTP peers.
 type HTTPPool struct {
-	// Context optionally specifies a context for the server to use when it
-	// receives a request.
-	// If nil, the server uses a nil Context.
-	Context func(*http.Request) context.Context
-
-	// Transport optionally specifies an http.RoundTripper for the client
-	// to use when it makes a request.
-	// If nil, the client uses http.DefaultTransport.
-	Transport func(context.Context) http.RoundTripper
-
 	// this peer's base URL, e.g. "https://example.net:8000"
 	self string
 
@@ -72,6 +62,16 @@ type HTTPPoolOptions struct {
 	// HashFn specifies the hash function of the consistent hash.
 	// If blank, it defaults to crc32.ChecksumIEEE.
 	HashFn consistenthash.Hash
+
+	// Transport optionally specifies an http.RoundTripper for the client
+	// to use when it makes a request.
+	// If nil, the client uses http.DefaultTransport.
+	Transport func(context.Context) http.RoundTripper
+
+	// Context optionally specifies a context for the server to use when it
+	// receives a request.
+	// If nil, uses the http.Request.Context()
+	Context func(*http.Request) context.Context
 }
 
 // NewHTTPPool initializes an HTTP pool of peers, and registers itself as a PeerPicker.
@@ -124,7 +124,7 @@ func (p *HTTPPool) Set(peers ...string) {
 	p.peers.Add(peers...)
 	p.httpGetters = make(map[string]*httpGetter, len(peers))
 	for _, peer := range peers {
-		p.httpGetters[peer] = &httpGetter{transport: p.Transport, baseURL: peer + p.opts.BasePath}
+		p.httpGetters[peer] = &httpGetter{getTransport: p.opts.Transport, baseURL: peer + p.opts.BasePath}
 	}
 }
 
@@ -174,8 +174,10 @@ func (p *HTTPPool) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	var ctx context.Context
-	if p.Context != nil {
-		ctx = p.Context(r)
+	if p.opts.Context != nil {
+		ctx = p.opts.Context(r)
+	} else {
+		ctx = r.Context()
 	}
 
 	group.Stats.ServerRequests.Add(1)
@@ -216,8 +218,9 @@ func (p *HTTPPool) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 }
 
 type httpGetter struct {
-	transport func(context.Context) http.RoundTripper
-	baseURL   string
+	getTransport func(context.Context) http.RoundTripper
+	transport    http.RoundTripper
+	baseURL      string
 }
 
 var bufferPool = sync.Pool{
@@ -239,11 +242,18 @@ func (h *httpGetter) makeRequest(ctx context.Context, method string, in *pb.GetR
 	// Pass along the context to the RoundTripper
 	req = req.WithContext(ctx)
 
-	tr := http.DefaultTransport
-	if h.transport != nil {
-		tr = h.transport(ctx)
+	// Associate the transport with this peer so we take advantage of connection reuse.
+	if h.transport == nil {
+		if h.getTransport != nil {
+			h.transport = h.getTransport(ctx)
+		}
+		// Ensure we have a copy of the default transport and not just a reference.
+		tr := http.DefaultTransport.(*http.Transport)
+		trCopy := http.Transport(*tr)
+		h.transport = &trCopy
 	}
-	res, err := tr.RoundTrip(req)
+
+	res, err := h.transport.RoundTrip(req)
 	if err != nil {
 		return err
 	}