Make router keep-alive setting configurable (#1225)

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`
 

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
+          - name: POD_NAMESPACE
-          valueFrom:
+            valueFrom:
-            fieldRef:
+              fieldRef:
-              fieldPath: metadata.namespace
+                fieldPath: metadata.namespace
-        - name: ROUTER_ROUND_TRIP_TIMEOUT
+          - name: ROUTER_ROUND_TRIP_TIMEOUT
-          value: {{ .Values.routerRoundTripTimeout | default "50ms" | quote }}
+            value: {{ .Values.router.roundTrip.timeout | default "50ms" | quote }}
-        - name: ROUTER_ROUNDTRIP_TIMEOUT_EXPONENT
+          - 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
+          - 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_MAX_RETRIES
+          - name: ROUTER_ROUND_TRIP_DISABLE_KEEP_ALIVE
-          value: {{ .Values.routerRoundTripMaxRetries | default 10 | quote }}
+            value: {{ .Values.router.roundTrip.disableKeepAlive | default true | quote }}
-        - name: ROUTER_ROUND_TRIP_SVC_ADDRESS_MAX_RETRIES
+          - name: ROUTER_ROUND_TRIP_MAX_RETRIES
-          value: {{ .Values.routerRoundTripSvcAddressMaxRetries | default 5 | quote }}
+            value: {{ .Values.router.roundTrip.maxRetries | default 10 | quote }}
-        - name: ROUTER_ROUND_TRIP_SVC_ADDRESS_UPDATE_TIMEOUT
+          - name: ROUTER_SVC_ADDRESS_MAX_RETRIES
-          value: {{ .Values.routerRoundTripSvcAddressUpdateTimeout | default 30 | quote }}
+            value: {{ .Values.router.svcAddressMaxRetries | default 5 | quote }}
-        - name: DEBUG_ENV
+          - name: ROUTER_SVC_ADDRESS_UPDATE_TIMEOUT
-          value: {{ .Values.debugEnv | quote }}
+            value: {{ .Values.router.svcAddressUpdateTimeout | default "30s" | quote }}
-        - name: TRACING_SAMPLING_RATE
+          - name: DEBUG_ENV
-          value: {{ .Values.traceSamplingRate | default "0.5" | quote }}
+            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"

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

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 }}
+            value: {{ .Values.router.roundTrip.maxRetries | default 10 | quote }}
-          - name: ROUTER_ROUND_TRIP_SVC_ADDRESS_MAX_RETRIES
+          - name: ROUTER_SVC_ADDRESS_MAX_RETRIES
-            value: {{ .Values.routerRoundTripSvcAddressMaxRetries | default 5 | quote }}
+            value: {{ .Values.router.svcAddressMaxRetries | default 5 | quote }}
-          - name: ROUTER_ROUND_TRIP_SVC_ADDRESS_UPDATE_TIMEOUT
+          - name: ROUTER_SVC_ADDRESS_UPDATE_TIMEOUT
-            value: {{ .Values.routerRoundTripSvcAddressUpdateTimeout | default 30 | quote }}
+            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"

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

pkg/router/functionHandler.go

@@ -70,9 +70,10 @@ type (
 	}
 
 	tsRoundTripperParams struct {
-		timeout         time.Duration
+		timeout          time.Duration
-		timeoutExponent int
+		timeoutExponent  int
-		keepAlive       time.Duration
+		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
+					// Default disables caching, Please refer to issue and specifically comment:
-					DisableKeepAlives: true,
+					// 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,
 				},
 			},
 		},

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,

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")
+	keepAliveTimeStr := os.Getenv("ROUTER_ROUND_TRIP_KEEP_ALIVE_TIME")
-	keepAlive, err := time.ParseDuration(keepAliveStr)
+	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")
+	svcAddrUpdateTimeoutStr := os.Getenv("ROUTER_SVC_ADDRESS_UPDATE_TIMEOUT")
-	svcAddrUpdateTimeout, err := time.ParseDuration(os.Getenv("ROUTER_ROUND_TRIP_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))

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"