From 56beac6508df0660b5dee7aa5022949ad3d36294 Mon Sep 17 00:00:00 2001 From: Ta-Ching Chen Date: Fri, 12 Jul 2019 12:12:04 +0800 Subject: [PATCH] Make router keep-alive setting configurable (#1225) --- charts/README.md | 7 ++++ charts/fission-all/templates/deployment.yaml | 42 ++++++++++--------- charts/fission-all/values.yaml | 32 +++++++++++++- charts/fission-core/templates/deployment.yaml | 22 +++++----- charts/fission-core/values.yaml | 32 +++++++++++++- pkg/router/functionHandler.go | 18 ++++---- pkg/router/functionHandler_test.go | 1 - pkg/router/router.go | 26 ++++++++---- pkg/router/router_test.go | 1 - 9 files changed, 131 insertions(+), 50 deletions(-) diff --git a/charts/README.md b/charts/README.md index a9e5a3e7..00e7d95f 100644 --- a/charts/README.md +++ b/charts/README.md @@ -65,6 +65,13 @@ Parameter | Description | Default `prometheusDeploy` | Set to true if prometheus needs to be deployed along with fission | `true` in `fission-all`, `false` in `fission-core` `canaryDeployment.enabled` | Set to true if you need canary deployment feature | `true` in `fission-all`, `false` in `fission-core` `extraCoreComponmentPodConfig` | Extend the container specs for the core fission pods. Can be used to add things like affinty/tolerations/nodeSelectors/etc. | None +`router.svcAddressMaxRetries` | Max retries times for router to retry on a certain service URL returns from cache/executor | `5` +`router.svcAddressUpdateTimeout` | The length of update lock expiry time for router to get a service URL returns from executor | `30` +`router.roundTrip.disableKeepAlive` | Disable transport keep-alive for fast switching function version | `true` +`router.roundTrip.keepAliveTime` | The keep-alive period for an active network connection to function pod | `30s` +`router.roundTrip.timeout` | HTTP transport request timeout | `50ms` +`router.roundTrip.timeoutExponent` | The length of request timeout will multiply with timeoutExponent after each retry | `2` +`router.roundTrip.maxRetries` | Max retries times of a failed request | `10` ### Extra configuration for `fission-all` diff --git a/charts/fission-all/templates/deployment.yaml b/charts/fission-all/templates/deployment.yaml index c03a883e..bebc4790 100644 --- a/charts/fission-all/templates/deployment.yaml +++ b/charts/fission-all/templates/deployment.yaml @@ -197,26 +197,28 @@ spec: command: ["/fission-bundle"] args: ["--routerPort", "8888", "--executorUrl", "http://executor.{{ .Release.Namespace }}", "--collectorEndpoint", "{{ .Values.traceCollectorEndpoint }}"] env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - - name: ROUTER_ROUND_TRIP_TIMEOUT - value: {{ .Values.routerRoundTripTimeout | default "50ms" | quote }} - - name: ROUTER_ROUNDTRIP_TIMEOUT_EXPONENT - value: {{ .Values.routerRoundTripTimeoutExponent | default 2 | quote }} - - name: ROUTER_ROUND_TRIP_KEEP_ALIVE_TIME - value: {{ .Values.routerRoundTripKeepAliveTime | default "30s" | quote }} - - name: ROUTER_ROUND_TRIP_MAX_RETRIES - value: {{ .Values.routerRoundTripMaxRetries | default 10 | quote }} - - name: ROUTER_ROUND_TRIP_SVC_ADDRESS_MAX_RETRIES - value: {{ .Values.routerRoundTripSvcAddressMaxRetries | default 5 | quote }} - - name: ROUTER_ROUND_TRIP_SVC_ADDRESS_UPDATE_TIMEOUT - value: {{ .Values.routerRoundTripSvcAddressUpdateTimeout | default 30 | quote }} - - name: DEBUG_ENV - value: {{ .Values.debugEnv | quote }} - - name: TRACING_SAMPLING_RATE - value: {{ .Values.traceSamplingRate | default "0.5" | quote }} + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: ROUTER_ROUND_TRIP_TIMEOUT + value: {{ .Values.router.roundTrip.timeout | default "50ms" | quote }} + - name: ROUTER_ROUNDTRIP_TIMEOUT_EXPONENT + value: {{ .Values.router.roundTrip.timeoutExponent | default 2 | quote }} + - name: ROUTER_ROUND_TRIP_KEEP_ALIVE_TIME + value: {{ .Values.router.roundTrip.keepAliveTime | default "30s" | quote }} + - name: ROUTER_ROUND_TRIP_DISABLE_KEEP_ALIVE + value: {{ .Values.router.roundTrip.disableKeepAlive | default true | quote }} + - name: ROUTER_ROUND_TRIP_MAX_RETRIES + value: {{ .Values.router.roundTrip.maxRetries | default 10 | quote }} + - name: ROUTER_SVC_ADDRESS_MAX_RETRIES + value: {{ .Values.router.svcAddressMaxRetries | default 5 | quote }} + - name: ROUTER_SVC_ADDRESS_UPDATE_TIMEOUT + value: {{ .Values.router.svcAddressUpdateTimeout | default "30s" | quote }} + - name: DEBUG_ENV + value: {{ .Values.debugEnv | quote }} + - name: TRACING_SAMPLING_RATE + value: {{ .Values.traceSamplingRate | default "0.5" | quote }} {{ if .Values.analytics }} - name: ANALYTICS_URL value: "https://g.fission.sh/metrics" diff --git a/charts/fission-all/values.yaml b/charts/fission-all/values.yaml index 7c9b9ff5..62ab861c 100644 --- a/charts/fission-all/values.yaml +++ b/charts/fission-all/values.yaml @@ -57,6 +57,36 @@ logger: fluentdImage: fluent/fluent-bit fluentdImageTag: 1.0.4 +## Router config +router: + svcAddressMaxRetries: 5 + svcAddressUpdateTimeout: 30s + roundTrip: + ## If true, router will disable the HTTP keep-alive which result in performance degradation. + ## But it ensures that router can redirect new coming requests to new function pods. + ## + ## If false, router will enable transport keep-alive feature for better performance. + ## However, the drawback is it takes longer to switch to newly created function pods + ## if using newdeploy as executor type for function. If you want to preserve the + ## performance while keeping the short switching time to new function, you can create + ## an environment with short grace period by setting flag "--graceperiod" (default 360s), + ## so that kubernetes will be able to reap old function pod quickly. + ## + ## For details, see https://github.com/fission/fission/issues/723 + disableKeepAlive: false + + ## The keep-alive period for an active network connection to function pod. + keepAliveTime: 30s + + ## HTTP transport request timeout + timeout: 50ms + + ## The length of request timeout will multiply with timeoutExponent after each retry + timeoutExponent: 2 + + ## Max retries times of a failed request + maxRetries: 10 + ## Message queue trigger config ### NATS Streaming, enabled by default nats: @@ -146,7 +176,6 @@ preUpgradeChecksImage: fission/pre-upgrade-checks ## summary is returned as part of http response debugEnv: false - ## set this flag to true if prometheus needs to be deployed along with fission prometheusDeploy: true @@ -154,7 +183,6 @@ prometheusDeploy: true canaryDeployment: enabled: true - # Use these flags to enable opentracing, the variable is endpoint of Jaeger collector in the format shown below #traceCollectorEndpoint: "http://jaeger-collector.jaeger.svc:14268/api/traces?format=jaeger.thrift" #traceSamplingRate: 0.75 diff --git a/charts/fission-core/templates/deployment.yaml b/charts/fission-core/templates/deployment.yaml index 75acc29d..485950dd 100644 --- a/charts/fission-core/templates/deployment.yaml +++ b/charts/fission-core/templates/deployment.yaml @@ -201,22 +201,24 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - - name: TRACING_SAMPLING_RATE - value: {{ .Values.traceSamplingRate | default "0.5" | quote }} - name: ROUTER_ROUND_TRIP_TIMEOUT - value: {{ .Values.routerRoundTripTimeout | default "50ms" | quote }} + value: {{ .Values.router.roundTrip.timeout | default "50ms" | quote }} - name: ROUTER_ROUNDTRIP_TIMEOUT_EXPONENT - value: {{ .Values.routerRoundTripTimeoutExponent | default 2 | quote }} + value: {{ .Values.router.roundTrip.timeoutExponent | default 2 | quote }} - name: ROUTER_ROUND_TRIP_KEEP_ALIVE_TIME - value: {{ .Values.routerRoundTripKeepAliveTime | default "30s" | quote }} + value: {{ .Values.router.roundTrip.keepAliveTime | default "30s" | quote }} + - name: ROUTER_ROUND_TRIP_DISABLE_KEEP_ALIVE + value: {{ .Values.router.roundTrip.disableKeepAlive | default true | quote }} - name: ROUTER_ROUND_TRIP_MAX_RETRIES - value: {{ .Values.routerRoundTripMaxRetries | default 10 | quote }} - - name: ROUTER_ROUND_TRIP_SVC_ADDRESS_MAX_RETRIES - value: {{ .Values.routerRoundTripSvcAddressMaxRetries | default 5 | quote }} - - name: ROUTER_ROUND_TRIP_SVC_ADDRESS_UPDATE_TIMEOUT - value: {{ .Values.routerRoundTripSvcAddressUpdateTimeout | default 30 | quote }} + value: {{ .Values.router.roundTrip.maxRetries | default 10 | quote }} + - name: ROUTER_SVC_ADDRESS_MAX_RETRIES + value: {{ .Values.router.svcAddressMaxRetries | default 5 | quote }} + - name: ROUTER_SVC_ADDRESS_UPDATE_TIMEOUT + value: {{ .Values.router.svcAddressUpdateTimeout | default "30s" | quote }} - name: DEBUG_ENV value: {{ .Values.debugEnv | quote }} + - name: TRACING_SAMPLING_RATE + value: {{ .Values.traceSamplingRate | default "0.5" | quote }} {{ if .Values.analytics }} - name: ANALYTICS_URL value: "https://g.fission.sh/metrics" diff --git a/charts/fission-core/values.yaml b/charts/fission-core/values.yaml index e3b9bf78..1965b4ed 100644 --- a/charts/fission-core/values.yaml +++ b/charts/fission-core/values.yaml @@ -44,6 +44,36 @@ builderNamespace: fission-builder ## Enable istio integration enableIstio: false +## Router config +router: + svcAddressMaxRetries: 5 + svcAddressUpdateTimeout: 30s + roundTrip: + ## If true, router will disable the HTTP keep-alive which result in performance degradation. + ## But it ensures that router can redirect new coming requests to new function pods. + ## + ## If false, router will enable transport keep-alive feature for better performance. + ## However, the drawback is it takes longer to switch to newly created function pods + ## if using newdeploy as executor type for function. If you want to preserve the + ## performance while keeping the short switching time to new function, you can create + ## an environment with short grace period by setting flag "--graceperiod" (default 360s), + ## so that kubernetes will be able to reap old function pod quickly. + ## + ## For details, see https://github.com/fission/fission/issues/723 + disableKeepAlive: true + + ## The keep-alive period for an active network connection to function pod. + keepAliveTime: 30s + + ## HTTP transport request timeout + timeout: 50ms + + ## The length of request timeout will multiply with timeoutExponent after each retry + timeoutExponent: 2 + + ## Max retries times of a failed request + maxRetries: 10 + ## Persist data to a persistent volume. persistence: ## If true, fission will create/use a Persistent Volume Claim @@ -114,4 +144,4 @@ canaryDeployment: # Use these flags to enable opentracing, the variable is endpoint of Jaeger collector in the format shown below #traceCollectorEndpoint: "http://jaeger-collector.jaeger.svc:14268/api/traces?format=jaeger.thrift" -#traceSamplingRate: 0.75 \ No newline at end of file +#traceSamplingRate: 0.75 diff --git a/pkg/router/functionHandler.go b/pkg/router/functionHandler.go index 96be0414..14826a7f 100644 --- a/pkg/router/functionHandler.go +++ b/pkg/router/functionHandler.go @@ -70,9 +70,10 @@ type ( } tsRoundTripperParams struct { - timeout time.Duration - timeoutExponent int - keepAlive time.Duration + timeout time.Duration + timeoutExponent int + disableKeepAlive bool + keepAliveTime time.Duration // maxRetires is the max times for RetryingRoundTripper to retry a request. // Default maxRetries is 10, which means router will retry for @@ -273,7 +274,7 @@ func (roundTripper RetryingRoundTripper) RoundTrip(req *http.Request) (resp *htt // over-riding default settings. transport.DialContext = (&net.Dialer{ Timeout: executingTimeout, - KeepAlive: roundTripper.funcHandler.tsRoundTripperParams.keepAlive, + KeepAlive: roundTripper.funcHandler.tsRoundTripperParams.keepAliveTime, }).DialContext overhead := time.Since(startTime) @@ -430,14 +431,17 @@ func (fh functionHandler) handler(responseWriter http.ResponseWriter, request *h Proxy: http.ProxyFromEnvironment, DialContext: (&net.Dialer{ Timeout: fh.tsRoundTripperParams.timeout, - KeepAlive: fh.tsRoundTripperParams.keepAlive, + KeepAlive: fh.tsRoundTripperParams.keepAliveTime, }).DialContext, MaxIdleConns: 100, IdleConnTimeout: 90 * time.Second, TLSHandshakeTimeout: 10 * time.Second, ExpectContinueTimeout: 1 * time.Second, - // Disables caching, Please refer to issue and specifically comment: https://github.com/fission/fission/issues/723#issuecomment-398781995 - DisableKeepAlives: true, + // Default disables caching, Please refer to issue and specifically comment: + // https://github.com/fission/fission/issues/723#issuecomment-398781995 + // You can change it by setting environment variable "ROUTER_ROUND_TRIP_DISABLE_KEEP_ALIVE" + // of router or helm variable "disableKeepAlive" before installation to false. + DisableKeepAlives: fh.tsRoundTripperParams.disableKeepAlive, }, }, }, diff --git a/pkg/router/functionHandler_test.go b/pkg/router/functionHandler_test.go index c04353d1..0a6a4933 100644 --- a/pkg/router/functionHandler_test.go +++ b/pkg/router/functionHandler_test.go @@ -81,7 +81,6 @@ func TestFunctionProxying(t *testing.T) { tsRoundTripperParams: &tsRoundTripperParams{ timeout: 50 * time.Millisecond, timeoutExponent: 2, - keepAlive: 30 * time.Second, maxRetries: 10, }, httpTrigger: httpTrigger, diff --git a/pkg/router/router.go b/pkg/router/router.go index da98a3d3..92bdc494 100644 --- a/pkg/router/router.go +++ b/pkg/router/router.go @@ -132,12 +132,21 @@ func Start(logger *zap.Logger, port int, executorUrl string) { zap.String("value", timeoutExponentStr)) } - keepAliveStr := os.Getenv("ROUTER_ROUND_TRIP_KEEP_ALIVE_TIME") - keepAlive, err := time.ParseDuration(keepAliveStr) + keepAliveTimeStr := os.Getenv("ROUTER_ROUND_TRIP_KEEP_ALIVE_TIME") + keepAliveTime, err := time.ParseDuration(keepAliveTimeStr) if err != nil { logger.Fatal("failed to parse keep alive duration from 'ROUTER_ROUND_TRIP_KEEP_ALIVE_TIME'", zap.Error(err), - zap.String("value", keepAliveStr)) + zap.String("value", keepAliveTimeStr)) + } + + disableKeepAliveStr := os.Getenv("ROUTER_ROUND_TRIP_DISABLE_KEEP_ALIVE") + disableKeepAlive, err := strconv.ParseBool(disableKeepAliveStr) + if err != nil { + disableKeepAlive = true + logger.Fatal("failed to parse enable keep alive from 'ROUTER_ROUND_TRIP_DISABLE_KEEP_ALIVE'", + zap.Error(err), + zap.String("value", disableKeepAliveStr)) } maxRetriesStr := os.Getenv("ROUTER_ROUND_TRIP_MAX_RETRIES") @@ -157,11 +166,11 @@ func Start(logger *zap.Logger, port int, executorUrl string) { } // svcAddrRetryCount is the max times for RetryingRoundTripper to retry with a specific service address - svcAddrRetryCountStr := os.Getenv("ROUTER_ROUND_TRIP_SVC_ADDRESS_MAX_RETRIES") + svcAddrRetryCountStr := os.Getenv("ROUTER_SVC_ADDRESS_MAX_RETRIES") svcAddrRetryCount, err := strconv.Atoi(svcAddrRetryCountStr) if err != nil { svcAddrRetryCount = 5 - logger.Info("failed to parse service address retry count from 'ROUTER_ROUND_TRIP_SVC_ADDRESS_MAX_RETRIES' - set to the default value", + logger.Info("failed to parse service address retry count from 'ROUTER_SVC_ADDRESS_MAX_RETRIES' - set to the default value", zap.Error(err), zap.String("value", svcAddrRetryCountStr), zap.Int("default", svcAddrRetryCount)) @@ -169,8 +178,8 @@ func Start(logger *zap.Logger, port int, executorUrl string) { // svcAddrUpdateTimeout is the timeout setting for a goroutine to wait for the update of a service entry. // If the update process cannot be done within the timeout window, consider it failed. - svcAddrUpdateTimeoutStr := os.Getenv("ROUTER_ROUND_TRIP_SVC_ADDRESS_UPDATE_TIMEOUT") - svcAddrUpdateTimeout, err := time.ParseDuration(os.Getenv("ROUTER_ROUND_TRIP_SVC_ADDRESS_UPDATE_TIMEOUT")) + svcAddrUpdateTimeoutStr := os.Getenv("ROUTER_SVC_ADDRESS_UPDATE_TIMEOUT") + svcAddrUpdateTimeout, err := time.ParseDuration(os.Getenv("ROUTER_SVC_ADDRESS_UPDATE_TIMEOUT")) if err != nil { svcAddrUpdateTimeout = 30 * time.Second logger.Info("failed to parse service address update timeout duration from 'ROUTER_ROUND_TRIP_SVC_ADDRESS_UPDATE_TIMEOUT' - set to the default value", @@ -182,7 +191,8 @@ func Start(logger *zap.Logger, port int, executorUrl string) { triggers, _, fnStore := makeHTTPTriggerSet(logger.Named("triggerset"), fmap, frmap, trmap, fissionClient, kubeClient, executor, restClient, &tsRoundTripperParams{ timeout: timeout, timeoutExponent: timeoutExponent, - keepAlive: keepAlive, + disableKeepAlive: disableKeepAlive, + keepAliveTime: keepAliveTime, maxRetries: maxRetries, svcAddrRetryCount: svcAddrRetryCount, }, isDebugEnv, throttler.MakeThrottler(svcAddrUpdateTimeout)) diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go index f6609a3f..a6d0d810 100644 --- a/pkg/router/router_test.go +++ b/pkg/router/router_test.go @@ -60,7 +60,6 @@ func TestRouter(t *testing.T) { &tsRoundTripperParams{ timeout: 50 * time.Millisecond, timeoutExponent: 2, - keepAlive: 30 * time.Second, maxRetries: 10, }, false, throttler.MakeThrottler(30*time.Second)) triggerUrl := "/foo"