Repository: fission/fission (language: go)

Issue:
Performance issues on concurrent function requests
<!-- Please answer these questions before submitting your issue. Thanks! -->

<!-- Documentation URL: https://docs.fission.io/ -->
<!-- Troubleshooting guide: https://docs.fission.io/trouble-shooting/ -->

**Fission/Kubernetes version**

<!-- If you tested with other services, for example Istio, please also provide the version of service as well. -->

<pre>
$ fission --version
client:
  fission/core:
    BuildDate: "2020-10-20T14:43:45Z"
    GitCommit: 52508e19343ef93da4ce8960908c5569f44da225
    Version: 1.11.2
server:
  fission/core:
    BuildDate: "2020-10-20T14:43:45Z"
    GitCommit: 52508e19343ef93da4ce8960908c5569f44da225
    Version: 1.11.2

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.8", GitCommit:"9f2892aab98fe339f3bd70e3c470144299398ace", GitTreeState:"clean", BuildDate:"2020-08-13T16:12:48Z", GoVersion:"go1.13.15", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"16+", GitVersion:"v1.16.15-gke.4901", GitCommit:"7ed5ddc0e67cb68296994f0b754cec45450d6a64", GitTreeState:"clean", BuildDate:"2020-11-06T18:22:22Z", GoVersion:"go1.13.15b4", Compiler:"gc", Platform:"linux/amd64"}
</pre>

**Kubernetes platform (e.g. Google Kubernetes Engine)**
* Google Kubernetes Engine
* Amazon EKS
* K3s

**Describe the bug**
<!--A clear and concise description of what the bug is.-->
Concurrent requests to a deployed function on Fission >=1.10 are very slow (take more than 2 seconds) and fission crashes in high-load situations. This problem does not exist in Fission 1.9. 

Also tried to run the current master and the fix provided in https://github.com/fission/fission/pull/1890 without any success.

**To Reproduce**
* Install Kubernetes Cluster
* Install Fission (Helm-Chart)
* Install Fission CLI
* Install Fission Demo-Function (hello)
* Create a Route for Demo function
* Run load tests (using hey) (concurrency 5)
<!-- Please provide steps for reproducing the error. -->

**Expected result**
<!--A clear and concise description of what you expected to happen.-->
* Response Times ~500msec

**Actual result**
* Response Times > 2sec

**Screenshots/Dump file**
<!--If applicable, add screenshots/fission dump file to help explain your problem.-->

<pre>
$ fission support dump
Has already been submitted
</pre>



Discussion / hints:
We tracked down the performance issue to communication between Executor and Kubernetes API. Executor makes two requests per user request. IMO, the result of those requests can be cache to a certain extent.

The response times can be greatly improved by introducing caches to the executor and pool manager. See https://github.com/baichinger/fission/commit/12896eb34fc9274045d4d7408a72577bd7b24c71 for a very rough workaround.
Hi @baichinger We would love to make this change upstream - do you mind raising a PR when you get a chance!
Hi @vishal-biyani! My hacky improvement lacks quite a bit on quality. There are no tests, it uses a randomly picked cache and the parameters to it are just wild guesses. Most importantly, it does not handle new/changed/deleted functions and therefore would lead to weird bugs.

My comment was intended as food for thought. The problems introduced by excessive usage of the Kubernetes API (custom resources) needs to be properly addressed. I can think of two approaches here: Sophisticated usage of caching or switch from get/list operations to watches. Both techniques have their downsides. It could even be a combined solution.

Fix patch:
diff --git a/pkg/executor/api.go b/pkg/executor/api.go
index b52777f7ef..33afc89a0b 100644
--- a/pkg/executor/api.go
+++ b/pkg/executor/api.go
@@ -28,8 +28,6 @@ import (
 	"github.com/pkg/errors"
 	"go.opencensus.io/plugin/ochttp"
 	"go.uber.org/zap"
-	k8serrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	fv1 "github.com/fission/fission/pkg/apis/core/v1"
 	ferror "github.com/fission/fission/pkg/error"
@@ -44,23 +42,13 @@ func (executor *Executor) getServiceForFunctionAPI(w http.ResponseWriter, r *htt
 	}
 
 	// get function metadata
-	m := metav1.ObjectMeta{}
-	err = json.Unmarshal(body, &m)
+	fn := &fv1.Function{}
+	err = json.Unmarshal(body, &fn)
 	if err != nil {
 		http.Error(w, "Failed to parse request", http.StatusBadRequest)
 		return
 	}
 
-	fn, err := executor.fissionClient.CoreV1().Functions(m.Namespace).Get(m.Name, metav1.GetOptions{})
-	if err != nil {
-		if k8serrors.IsNotFound(err) {
-			http.Error(w, "Failed to find function", http.StatusNotFound)
-		} else {
-			http.Error(w, "Failed to get function", http.StatusInternalServerError)
-		}
-		return
-	}
-
 	t := fn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType
 	et, exists := executor.executorTypes[t]
 	if !exists {
@@ -88,7 +76,7 @@ func (executor *Executor) getServiceForFunctionAPI(w http.ResponseWriter, r *htt
 		code, msg := ferror.GetHTTPError(err)
 		executor.logger.Error("error getting service for function",
 			zap.Error(err),
-			zap.String("function", m.Name),
+			zap.String("function", fn.ObjectMeta.Name),
 			zap.String("fission_http_error", msg))
 		http.Error(w, msg, code)
 		return
@@ -98,7 +86,7 @@ func (executor *Executor) getServiceForFunctionAPI(w http.ResponseWriter, r *htt
 	if err != nil {
 		executor.logger.Error(
 			"error writing HTTP response",
-			zap.String("function", m.Name),
+			zap.String("function", fn.ObjectMeta.Name),
 			zap.Error(err),
 		)
 	}
diff --git a/pkg/executor/client/client.go b/pkg/executor/client/client.go
index 0d7e260adc..06e6828a4c 100644
--- a/pkg/executor/client/client.go
+++ b/pkg/executor/client/client.go
@@ -70,10 +70,10 @@ func MakeClient(logger *zap.Logger, executorURL string) *Client {
 }
 
 // GetServiceForFunction returns the service name for a given function.
-func (c *Client) GetServiceForFunction(ctx context.Context, metadata *metav1.ObjectMeta) (string, error) {
+func (c *Client) GetServiceForFunction(ctx context.Context, fn *fv1.Function) (string, error) {
 	executorURL := c.executorURL + "/v2/getServiceForFunction"
 
-	body, err := json.Marshal(metadata)
+	body, err := json.Marshal(fn)
 	if err != nil {
 		return "", errors.Wrap(err, "could not marshal request body for getting service for function")
 	}
diff --git a/pkg/router/functionHandler.go b/pkg/router/functionHandler.go
index 055d1f7d28..50993a21ca 100644
--- a/pkg/router/functionHandler.go
+++ b/pkg/router/functionHandler.go
@@ -528,7 +528,7 @@ func (fh functionHandler) getServiceEntryFromExecutor() (*url.URL, error) {
 
 	ctx, cancel := context.WithTimeout(context.Background(), timeout)
 	defer cancel()
-	service, err := fh.executor.GetServiceForFunction(ctx, &fh.function.ObjectMeta)
+	service, err := fh.executor.GetServiceForFunction(ctx, fh.function)
 	if err != nil {
 		statusCode, errMsg := ferror.GetHTTPError(err)
 		fh.logger.Error("error from GetServiceForFunction",


Test patch:
diff --git a/pkg/executor/executor_test.go b/pkg/executor/executor_test.go
index cc9bdce3f0..b8ca3abae9 100644
--- a/pkg/executor/executor_test.go
+++ b/pkg/executor/executor_test.go
@@ -256,7 +256,7 @@ func TestExecutor(t *testing.T) {
 
 	// the main test: get a service for a given function
 	t1 := time.Now()
-	svc, err := poolmgrClient.GetServiceForFunction(context.Background(), &f.ObjectMeta)
+	svc, err := poolmgrClient.GetServiceForFunction(context.Background(), f)
 	if err != nil {
 		log.Panicf("failed to get func svc: %v", err)
 	}