fix(backend): Fix connection lifetime default and variable names. (#6509

) The apiserver config parser uses `viper.GetDuration` to parse the mysql connection lifetime variable. The `viper.GetDuration` function uses `cast.ToDuration`, which uses `cast.ToDurationE`, which assumes durations to be in nanoseconds if they don't explictly include a unit. Since the default lifetime in the kustomize manifest is `120`, we expire connections after 120ns, which is probably unintended. To make this more clear, this patch includes duration units in the default values, and drops the `Secs` suffix from the configuration variables, since the code doesn't assume that durations are in seconds. See https://github.com/spf13/cast/blob/22b2b540ce2e240dd3565d1238df82136be02051/caste.go#L68-L72.
kubeflow · Sep 8, 2021 · e58aff7 · e58aff7
1 parent 0fba85c
commit e58aff7
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 7 deletions.
diff --git a/backend/src/apiserver/client_manager.go b/backend/src/apiserver/client_manager.go
@@ -50,7 +50,7 @@ const (
 	mysqlExtraParams       = "DBConfig.ExtraParams"
 	archiveLogFileName     = "ARCHIVE_CONFIG_LOG_FILE_NAME"
 	archiveLogPathPrefix   = "ARCHIVE_CONFIG_LOG_PATH_PREFIX"
-	dbConMaxLifeTimeSec    = "DBConfig.ConMaxLifeTimeSec"
+	dbConMaxLifeTime       = "DBConfig.ConMaxLifeTime"
 
 	visualizationServiceHost = "ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_HOST"
 	visualizationServicePort = "ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_PORT"
@@ -159,7 +159,7 @@ func (c *ClientManager) Authenticators() []auth.Authenticator {
 func (c *ClientManager) init() {
 	glog.Info("Initializing client manager")
 	db := initDBClient(common.GetDurationConfig(initConnectionTimeout))
-	db.SetConnMaxLifetime(common.GetDurationConfig(dbConMaxLifeTimeSec))
+	db.SetConnMaxLifetime(common.GetDurationConfig(dbConMaxLifeTime))
 
 	// time
 	c.time = util.NewRealTime()

diff --git a/backend/src/apiserver/config/config.json b/backend/src/apiserver/config/config.json
@@ -4,7 +4,7 @@
     "DataSourceName": "",
     "DBName": "mlpipeline",
     "GroupConcatMaxLen": "4194304",
-    "ConMaxLifeTimeSec": "120"
+    "ConMaxLifeTime": "120s"
   },
   "ObjectStoreConfig": {
     "AccessKey": "minio",

diff --git a/manifests/kustomize/base/installs/generic/pipeline-install-config.yaml b/manifests/kustomize/base/installs/generic/pipeline-install-config.yaml
@@ -68,7 +68,10 @@ data:
   ## any node and avoid defaulting to specific nodes. Allowed values are:
   ## 'false' and 'true'.
   cacheNodeRestrictions: "false"
-  ## ConMaxLifeTimeSec will set the connection max lifetime for MySQL
+  ## ConMaxLifeTime will set the connection max lifetime for MySQL
   ## this is very important to setup when using external databases.
   ## See this issue for more details: https://github.com/kubeflow/pipelines/issues/5329
-  ConMaxLifeTimeSec: "120"
+  ## Note: this value should be a string that can be parsed by `time.ParseDuration`.
+  ## If this value doesn't include a unit abbreviation, the units will be assumed
+  ## to be nanoseconds.
+  ConMaxLifeTime: "120s"
diff --git a/manifests/kustomize/base/pipeline/ml-pipeline-apiserver-deployment.yaml b/manifests/kustomize/base/pipeline/ml-pipeline-apiserver-deployment.yaml
@@ -58,11 +58,11 @@ spec:
             configMapKeyRef:
               name: pipeline-install-config
               key: dbPort
-        - name: DBCONFIG_CONMAXLIFETIMESEC
+        - name: DBCONFIG_CONMAXLIFETIME
           valueFrom:
             configMapKeyRef:
               name: pipeline-install-config
-              key: ConMaxLifeTimeSec
+              key: ConMaxLifeTime
         - name: OBJECTSTORECONFIG_ACCESSKEY
           valueFrom:
             secretKeyRef: