From c981bdef8c02843434c510e17d0ea67cc12d8a1e Mon Sep 17 00:00:00 2001 From: James Phillips Date: Wed, 16 Dec 2015 22:27:07 -0800 Subject: [PATCH 1/2] Adds client and transport pooling in the API so we don't leak connections. --- api/api.go | 42 +++++++++++++++++++++++++++++------------- api/api_test.go | 19 +++++++++++-------- 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/api/api.go b/api/api.go index 6736aecd2..51b73b483 100644 --- a/api/api.go +++ b/api/api.go @@ -13,6 +13,7 @@ import ( "os" "strconv" "strings" + "sync" "time" "github.com/hashicorp/go-cleanhttp" @@ -122,12 +123,20 @@ type Config struct { Token string } +var defaultHttpClient = cleanhttp.DefaultClient() + +var defaultInsecureTransport = &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, +} + // DefaultConfig returns a default configuration for the client func DefaultConfig() *Config { config := &Config{ Address: "127.0.0.1:8500", Scheme: "http", - HttpClient: cleanhttp.DefaultClient(), + HttpClient: defaultHttpClient, } if addr := os.Getenv("CONSUL_HTTP_ADDR"); addr != "" { @@ -172,11 +181,7 @@ func DefaultConfig() *Config { } if !doVerify { - config.HttpClient.Transport = &http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true, - }, - } + config.HttpClient.Transport = defaultInsecureTransport } } @@ -188,6 +193,9 @@ type Client struct { config Config } +var unixClients = make(map[string]*http.Client) +var unixClientsLock sync.Mutex + // NewClient returns a new client func NewClient(config *Config) (*Client, error) { // bootstrap the config @@ -206,14 +214,22 @@ func NewClient(config *Config) (*Client, error) { } if parts := strings.SplitN(config.Address, "unix://", 2); len(parts) == 2 { - trans := cleanhttp.DefaultTransport() - trans.Dial = func(_, _ string) (net.Conn, error) { - return net.Dial("unix", parts[1]) - } - config.HttpClient = &http.Client{ - Transport: trans, - } config.Address = parts[1] + + unixClientsLock.Lock() + if client, ok := unixClients[config.Address]; ok { + config.HttpClient = client + } else { + trans := cleanhttp.DefaultTransport() + trans.Dial = func(_, _ string) (net.Conn, error) { + return net.Dial("unix", config.Address) + } + config.HttpClient = &http.Client{ + Transport: trans, + } + unixClients[config.Address] = config.HttpClient + } + unixClientsLock.Unlock() } client := &Client{ diff --git a/api/api_test.go b/api/api_test.go index 314a89b14..4f13d0ae2 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -244,13 +244,16 @@ func TestAPI_UnixSocket(t *testing.T) { }) defer s.Stop() - agent := c.Agent() - - info, err := agent.Self() - if err != nil { - t.Fatalf("err: %s", err) - } - if info["Config"]["NodeName"] == "" { - t.Fatalf("bad: %v", info) + // Run a few iterations to test the path where we use the pooled + // connection. + for i := 0; i < 3; i++ { + agent := c.Agent() + info, err := agent.Self() + if err != nil { + t.Fatalf("err: %s", err) + } + if info["Config"]["NodeName"] == "" { + t.Fatalf("bad: %v", info) + } } } From 79aabd0b9e10a154d805b0ef5ce01ca3517f0e03 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Thu, 17 Dec 2015 06:42:07 -0800 Subject: [PATCH 2/2] Makes the insecure transport work like the default one. --- api/api.go | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/api/api.go b/api/api.go index 51b73b483..f34fafbf6 100644 --- a/api/api.go +++ b/api/api.go @@ -123,13 +123,21 @@ type Config struct { Token string } +// defaultHttpClient is a shared client instance that is used to prevent apps +// that create multiple clients from opening multiple connections, which would +// leak file descriptors. var defaultHttpClient = cleanhttp.DefaultClient() -var defaultInsecureTransport = &http.Transport{ - TLSClientConfig: &tls.Config{ +// defaultInsecureTransport is a shared transport that will get injected into +// the defaultHttpClient if the CONSUL_HTTP_SSL_VERIFY environment variable is +// set to true. +var defaultInsecureTransport = func() *http.Transport { + trans := cleanhttp.DefaultTransport() + trans.TLSClientConfig = &tls.Config{ InsecureSkipVerify: true, - }, -} + } + return trans +}() // DefaultConfig returns a default configuration for the client func DefaultConfig() *Config { @@ -193,7 +201,13 @@ type Client struct { config Config } +// unixClients contains a set of shared UNIX socket clients, indexed by address. +// These shared instances are used to prevent apps that create multiple clients +// from opening multiple connections, which would leak file descriptors. var unixClients = make(map[string]*http.Client) + +// unixClientsLock serializes access to the unixClients map, since most users +// would expect NewClient to be thread-safe. var unixClientsLock sync.Mutex // NewClient returns a new client