From 76b3cd49b6f0049111a01aa5fb6d0294a3e36a76 Mon Sep 17 00:00:00 2001 From: "Derrick J. Wippler" Date: Tue, 4 Jun 2019 14:53:24 -0500 Subject: [PATCH] Reverted change to associate `Transport` to `httpGetter`. This change caused a data race during async calls to groupcache.Get(). Also discovered `DefaultTransport` has per address connection pooling only when the request was a success, which is sufficient for most use cases. --- CHANGELOG | 8 +++++--- http.go | 14 ++++---------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 6126dc5..eb590cf 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,19 +4,21 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [2.0.0-rc.3] - 2019-06-03 +## [2.0.0-rc.4] - 2019-06-04 ### Changes * 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 there is no need to - call `Transport` function for each request. * 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. * Changed import paths to /v2 in accordance with go modules rules * Fixed Issue where `DefaultTransport` was always used even if `Transport` was specified by the user. +### Removed +* Reverted change to associate `Transport` to `httpGetter`, Which caused a data + race. Also discovered `DefaultTransport` has per address connection pooling + only when the request was a success, which is sufficient for most use cases. ## [1.3.0] - 2019-05-23 ### Added diff --git a/http.go b/http.go index 24854ce..a792357 100644 --- a/http.go +++ b/http.go @@ -222,7 +222,6 @@ func (p *HTTPPool) ServeHTTP(w http.ResponseWriter, r *http.Request) { type httpGetter struct { getTransport func(context.Context) http.RoundTripper - transport http.RoundTripper baseURL string } @@ -245,17 +244,12 @@ func (h *httpGetter) makeRequest(ctx context.Context, method string, in *pb.GetR // Pass along the context to the RoundTripper req = req.WithContext(ctx) - // Associate the transport with this peer so we don't need to - // call getTransport() every time a request is made. - if h.transport == nil { - if h.getTransport != nil { - h.transport = h.getTransport(ctx) - } else { - h.transport = http.DefaultTransport - } + tr := http.DefaultTransport + if h.getTransport != nil { + tr = h.getTransport(ctx) } - res, err := h.transport.RoundTrip(req) + res, err := tr.RoundTrip(req) if err != nil { return err }