From aabb9a8ae258f03f11013dedd5da7aae5c9a64bb Mon Sep 17 00:00:00 2001 From: Fahri Nurul Hidayat Date: Fri, 16 Aug 2019 18:51:16 +0700 Subject: [PATCH] Feature/sidecarproxy timeout (#30) * Separate main&canary and sidecar proxy client timeouts in config * Add MultiHTTPClientConfig comment * Remove duplicate log --- cmd/main.go | 2 +- cmd/root.go | 15 +++++++++++---- cmd/root_test.go | 15 ++++++++++++--- config.template.json | 16 ++++++++++++---- config/config.go | 8 +++++++- server/server.go | 14 +++++++------- 6 files changed, 50 insertions(+), 20 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 0572e34..fa3ae9c 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -3,8 +3,8 @@ package main import ( "log" - routerversion "github.com/tiket-libre/canary-router/version" "github.com/juju/errors" + routerversion "github.com/tiket-libre/canary-router/version" ) var ( diff --git a/cmd/root.go b/cmd/root.go index 77e54d5..96af209 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -23,10 +23,17 @@ var defaultConfig = config.Config{ WriteTimeout: 15, IdleTimeout: 120, }, - Client: config.HTTPClientConfig{ - Timeout: 5, - MaxIdleConns: 100, - IdleConnTimeout: 30, + Client: config.MultiHTTPClientConfig{ + MainAndCanary: config.HTTPClientConfig{ + Timeout: 5, + MaxIdleConns: 1000, + IdleConnTimeout: 30, + }, + Sidecar: config.HTTPClientConfig{ + Timeout: 2, + MaxIdleConns: 1000, + IdleConnTimeout: 30, + }, }, } diff --git a/cmd/root_test.go b/cmd/root_test.go index a692641..5af1ad5 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -10,12 +10,18 @@ import ( func Test_mergeConfig(t *testing.T) { defaultConfig := config.Config{ Server: config.HTTPServerConfig{ReadTimeout: 5, WriteTimeout: 15, IdleTimeout: 120}, - Client: config.HTTPClientConfig{Timeout: 5, MaxIdleConns: 100, IdleConnTimeout: 30}, + Client: config.MultiHTTPClientConfig{ + MainAndCanary: config.HTTPClientConfig{Timeout: 5, MaxIdleConns: 1000, IdleConnTimeout: 30}, + Sidecar: config.HTTPClientConfig{Timeout: 2, MaxIdleConns: 1000, IdleConnTimeout: 30}, + }, } allSetConfig := config.Config{ Server: config.HTTPServerConfig{ReadTimeout: 6, WriteTimeout: 7, IdleTimeout: 130}, - Client: config.HTTPClientConfig{Timeout: 8, MaxIdleConns: 200, IdleConnTimeout: 40}, + Client: config.MultiHTTPClientConfig{ + MainAndCanary: config.HTTPClientConfig{Timeout: 6, MaxIdleConns: 2000, IdleConnTimeout: 40}, + Sidecar: config.HTTPClientConfig{Timeout: 8, MaxIdleConns: 3000, IdleConnTimeout: 50}, + }, } type args struct { @@ -31,7 +37,10 @@ func Test_mergeConfig(t *testing.T) { {name: "if not set, set default", args: args{ targetConfig: &config.Config{ Server: config.HTTPServerConfig{ReadTimeout: 0, WriteTimeout: 0, IdleTimeout: 0}, - Client: config.HTTPClientConfig{Timeout: 0, MaxIdleConns: 0, IdleConnTimeout: 0}, + Client: config.MultiHTTPClientConfig{ + MainAndCanary: config.HTTPClientConfig{Timeout: 0, MaxIdleConns: 0, IdleConnTimeout: 0}, + Sidecar: config.HTTPClientConfig{Timeout: 0, MaxIdleConns: 0, IdleConnTimeout: 0}, + }, }, defaultConfig: defaultConfig, }, wantConfig: defaultConfig}, {name: "if all set, leave it", args: args{ diff --git a/config.template.json b/config.template.json index a54b57b..855b1b6 100644 --- a/config.template.json +++ b/config.template.json @@ -18,9 +18,17 @@ "idle-timeout": 120 }, "proxy-client": { - "timeout": 5, - "max-idle-conns": 100, - "idle-conn-timeout": 30, - "disable-compression": true + "to-main-and-canary": { + "timeout": 5, + "max-idle-conns": 1000, + "idle-conn-timeout": 30, + "disable-compression": true + }, + "to-sidecar": { + "timeout": 2, + "max-idle-conns": 1000, + "idle-conn-timeout": 30, + "disable-compression": true + }, } } \ No newline at end of file diff --git a/config/config.go b/config/config.go index 3104ca0..829b585 100644 --- a/config/config.go +++ b/config/config.go @@ -21,7 +21,7 @@ type Config struct { CircuitBreaker CircuitBreaker `mapstructure:"circuit-breaker"` Instrumentation InstrumentationConfig `mapstructure:"instrumentation"` Server HTTPServerConfig `mapstructure:"router-server"` - Client HTTPClientConfig `mapstructure:"proxy-client"` + Client MultiHTTPClientConfig `mapstructure:"proxy-client"` } // InstrumentationConfig holds the configuration values specific to the instrumentation aspect. @@ -44,6 +44,12 @@ type HTTPServerConfig struct { IdleTimeout int `mapstructure:"idle-timeout"` } +// MultiHTTPClientConfig holds the configuration for instantiating main&canary and sidecar proxy http.Client +type MultiHTTPClientConfig struct { + MainAndCanary HTTPClientConfig `mapstructure:"to-main-and-canary"` + Sidecar HTTPClientConfig `mapstructure:"to-sidecar"` +} + // HTTPClientConfig holds the configuration for instantiating http.Client type HTTPClientConfig struct { Timeout int `mapstructure:"timeout"` diff --git a/server/server.go b/server/server.go index 5a9d3a4..c22f6c7 100644 --- a/server/server.go +++ b/server/server.go @@ -42,16 +42,17 @@ type Server struct { func NewServer(config config.Config) (*Server, error) { server := &Server{config: config} - proxies, err := canaryrouter.BuildProxies(config.Client, config.MainTarget, config.CanaryTarget) + proxies, err := canaryrouter.BuildProxies(config.Client.MainAndCanary, config.MainTarget, config.CanaryTarget) if err != nil { return nil, errors.Trace(err) } server.proxies = proxies - tr := &http.Transport{ - MaxIdleConns: config.Client.MaxIdleConns, - IdleConnTimeout: time.Duration(config.Client.IdleConnTimeout) * time.Second, - DisableCompression: config.Client.DisableCompression, + sidecarTransport := &http.Transport{ + ResponseHeaderTimeout: time.Duration(config.Client.Sidecar.Timeout) * time.Second, + MaxIdleConns: config.Client.Sidecar.MaxIdleConns, + IdleConnTimeout: time.Duration(config.Client.Sidecar.IdleConnTimeout) * time.Second, + DisableCompression: config.Client.Sidecar.DisableCompression, } sidecarURL, err := url.Parse(server.config.SidecarURL) @@ -59,9 +60,8 @@ func NewServer(config config.Config) (*Server, error) { return nil, fmt.Errorf("Failed when creating proxy to sidecar: %v", err) } server.sidecarProxy = httputil.NewSingleHostReverseProxy(sidecarURL) - server.sidecarProxy.Transport = tr + server.sidecarProxy.Transport = sidecarTransport server.sidecarProxy.ErrorHandler = func(w http.ResponseWriter, req *http.Request, err error) { - log.Printf("sidecar proxy error: %+v", err) w.WriteHeader(StatusSidecarError) _, errWrite := w.Write([]byte(err.Error())) if errWrite != nil {