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.
This commit is contained in:
Derrick J. Wippler 2019-06-04 14:53:24 -05:00
parent 2869a0ce28
commit 76b3cd49b6
2 changed files with 9 additions and 13 deletions

View File

@ -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/), 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). 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 ### Changes
* Now using golang standard `context.Context` instead of `groupcache.Context`. * Now using golang standard `context.Context` instead of `groupcache.Context`.
* HTTP requests made by `httpGetter` now respect `context.Context` done. * HTTP requests made by `httpGetter` now respect `context.Context` done.
* Moved `HTTPPool` config `Context` and `Transport` to `HTTPPoolOptions` for consist configuration. * 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 * 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 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. evict code ensures the hotcache does not overcrowd the maincache.
* Changed import paths to /v2 in accordance with go modules rules * Changed import paths to /v2 in accordance with go modules rules
* Fixed Issue where `DefaultTransport` was always used even if `Transport` was * Fixed Issue where `DefaultTransport` was always used even if `Transport` was
specified by the user. 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 ## [1.3.0] - 2019-05-23
### Added ### Added

14
http.go
View File

@ -222,7 +222,6 @@ func (p *HTTPPool) ServeHTTP(w http.ResponseWriter, r *http.Request) {
type httpGetter struct { type httpGetter struct {
getTransport func(context.Context) http.RoundTripper getTransport func(context.Context) http.RoundTripper
transport http.RoundTripper
baseURL string 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 // Pass along the context to the RoundTripper
req = req.WithContext(ctx) req = req.WithContext(ctx)
// Associate the transport with this peer so we don't need to tr := http.DefaultTransport
// call getTransport() every time a request is made. if h.getTransport != nil {
if h.transport == nil { tr = h.getTransport(ctx)
if h.getTransport != nil {
h.transport = h.getTransport(ctx)
} else {
h.transport = http.DefaultTransport
}
} }
res, err := h.transport.RoundTrip(req) res, err := tr.RoundTrip(req)
if err != nil { if err != nil {
return err return err
} }