diff --git a/.travis.yml b/.travis.yml index e4fa39d..1db9513 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,9 +9,9 @@ script: - go test ./... go: - - 1.9.x - 1.10.x - 1.11.x + - 1.12.x - master cache: diff --git a/CHANGELOG b/CHANGELOG index 6ebe50f..175ceda 100644 --- a/CHANGELOG +++ b/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 diff --git a/groupcache.go b/groupcache.go index e70981f..a45661c 100644 --- a/groupcache.go +++ b/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 } diff --git a/groupcache_test.go b/groupcache_test.go index ad26250..77402d7 100644 --- a/groupcache_test.go +++ b/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. diff --git a/http.go b/http.go index d84d5ae..39e0703 100644 --- a/http.go +++ b/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 }