Project

General

Profile

Idea #9005

Updated by Tom Clegg almost 7 years ago

h2. Background 

 As an example, keep-web currently 
 * gets an arvadosclient.ArvadosClient from an arvadosclient.ClientPool 
 * configures it with the current token 
 * passes it to keepclient.MakeKeepClient() in order to get a keepclient.Client 

 (The public APIs for creating a keepclient.Client all involve passing in an arvadosclient.ArvadosClient, so we can't just make a pool of keepclient.Clients and assume they won't leak credentials across requests.) 

 At this point, keepclient.MakeKeepClient() creates a new http.Client object. This means 
 * Consecutive keep-web requests cannot reuse http connections to keep services 
 * (Currently) the HTTP connections to keep services are left open anyway, because we don't tell the http.Client that we won't be reusing it 

 In general, sharing an http.Client across requests is beneficial because it reduces TCP and TLS handshaking. 

 h2. Proposed improvements 

 keepclient MakeKeepClient should use kc.Arvados.Client instead of its own Client field unless the caller specifies otherwise. 
 * keepclient.New() should leave the Client field set http.Client handed to nil by default. 
 * keepclient methods that use an http client should get it from a private httpClient() method that works along the lines of the one in source:sdk/go/arvados/client.go: use the KeepClient arvadosclient.ArvadosClient object. The Client field is already exported so the caller can override it later if non-nil, otherwise the Arvados one. 

 (This desired. When an arvadosclient.ClientPool is used, this change alone should reduce the proliferation of http.Client objects significantly.) significantly. 

 Also, arvadosclient.MakeArvadosClient should reuse the same two http.Client objects for all new ArvadosClient objects -- e.g., arvadosclient.DefaultHTTPClient when ApiInsecure is false, arvadosclient.DefaultInsecureHTTPClient when true. The caller can override it later if desired. 

 arvadosclient.DefaultHTTPClient should default to http.DefaultClient. 

 (This change should reduce the proliferation of http.Client objects further, especially in programs that don't use an arvados client pool.) 

Back