From 2846ad9f934c72109976a118e7336e26c33aeb43 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 28 Jan 2021 09:37:15 +0530 Subject: [PATCH] MM-31391: Add max connection idle timeout to config (#16792) * Update config.go * Update telemetry.go * Update store and store test * Update settings.go * use new error code * Trigger CI Co-authored-by: Haardik Dharma --- go.tools.mod | 2 +- go.tools.sum | 2 ++ i18n/en.json | 4 ++++ model/config.go | 9 +++++++++ services/telemetry/telemetry.go | 1 + store/sqlstore/store.go | 1 + store/sqlstore/store_test.go | 2 ++ store/storetest/settings.go | 2 ++ 8 files changed, 22 insertions(+), 1 deletion(-) diff --git a/go.tools.mod b/go.tools.mod index 21e6b07865..28bd9e6891 100644 --- a/go.tools.mod +++ b/go.tools.mod @@ -4,7 +4,7 @@ go 1.14 require ( github.com/jstemmer/go-junit-report v0.9.1 // indirect - github.com/mattermost/mattermost-utilities/mmgotool v0.0.0-20201216181404-3faa6075089a // indirect + github.com/mattermost/mattermost-utilities/mmgotool v0.0.0-20210122154757-d807da7d142a // indirect github.com/philhofer/fwd v1.0.0 // indirect github.com/reflog/struct2interface v0.6.1 // indirect github.com/tinylib/msgp v1.1.2 // indirect diff --git a/go.tools.sum b/go.tools.sum index 5afd3ad27b..006022fc17 100644 --- a/go.tools.sum +++ b/go.tools.sum @@ -77,6 +77,8 @@ github.com/mattermost/mattermost-utilities/mmgotool v0.0.0-20201013204121-e463fc github.com/mattermost/mattermost-utilities/mmgotool v0.0.0-20201216181404-3faa6075089a/go.mod h1:3gKozJI8n2Y/vW37GfnFWAdehGXe5yZlt+HykK6Y3DM= github.com/mattermost/mattermost-utilities/mmgotool v0.0.0-20210103185547-4c12aa739237 h1:w6GQs7SU6abD0QXKoqwd4bpEzNaW1xKwpeftBpIX1Do= github.com/mattermost/mattermost-utilities/mmgotool v0.0.0-20210103185547-4c12aa739237/go.mod h1:3gKozJI8n2Y/vW37GfnFWAdehGXe5yZlt+HykK6Y3DM= +github.com/mattermost/mattermost-utilities/mmgotool v0.0.0-20210122154757-d807da7d142a h1:H8g7UkbNEpYE6UD7ej8UWp2W9q0eW+7BtvUl1v+bGMc= +github.com/mattermost/mattermost-utilities/mmgotool v0.0.0-20210122154757-d807da7d142a/go.mod h1:3gKozJI8n2Y/vW37GfnFWAdehGXe5yZlt+HykK6Y3DM= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= diff --git a/i18n/en.json b/i18n/en.json index 28c91ebfca..81f78de859 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -7466,6 +7466,10 @@ "id": "model.config.is_valid.sitename_length.app_error", "translation": "Site name must be less than or equal to {{.MaxLength}} characters." }, + { + "id": "model.config.is_valid.sql_conn_max_idle_time_milliseconds.app_error", + "translation": "Invalid connection maximum idle time for SQL settings. Must be a non-negative number." + }, { "id": "model.config.is_valid.sql_conn_max_lifetime_milliseconds.app_error", "translation": "Invalid connection maximum lifetime for SQL settings. Must be a non-negative number." diff --git a/model/config.go b/model/config.go index 7211b21679..af43745188 100644 --- a/model/config.go +++ b/model/config.go @@ -1091,6 +1091,7 @@ type SqlSettings struct { DataSourceSearchReplicas []string `access:"environment,write_restrictable,cloud_restrictable"` MaxIdleConns *int `access:"environment,write_restrictable,cloud_restrictable"` ConnMaxLifetimeMilliseconds *int `access:"environment,write_restrictable,cloud_restrictable"` + ConnMaxIdleTimeMilliseconds *int `access:"environment,write_restrictable,cloud_restrictable"` MaxOpenConns *int `access:"environment,write_restrictable,cloud_restrictable"` Trace *bool `access:"environment,write_restrictable,cloud_restrictable"` AtRestEncryptKey *string `access:"environment,write_restrictable,cloud_restrictable"` @@ -1137,6 +1138,10 @@ func (s *SqlSettings) SetDefaults(isUpdate bool) { s.ConnMaxLifetimeMilliseconds = NewInt(3600000) } + if s.ConnMaxIdleTimeMilliseconds == nil { + s.ConnMaxIdleTimeMilliseconds = NewInt(300000) + } + if s.Trace == nil { s.Trace = NewBool(false) } @@ -3241,6 +3246,10 @@ func (s *SqlSettings) isValid() *AppError { return NewAppError("Config.IsValid", "model.config.is_valid.sql_conn_max_lifetime_milliseconds.app_error", nil, "", http.StatusBadRequest) } + if *s.ConnMaxIdleTimeMilliseconds < 0 { + return NewAppError("Config.IsValid", "model.config.is_valid.sql_conn_max_idle_time_milliseconds.app_error", nil, "", http.StatusBadRequest) + } + if *s.QueryTimeout <= 0 { return NewAppError("Config.IsValid", "model.config.is_valid.sql_query_timeout.app_error", nil, "", http.StatusBadRequest) } diff --git a/services/telemetry/telemetry.go b/services/telemetry/telemetry.go index 54050b3a13..a318914bd0 100644 --- a/services/telemetry/telemetry.go +++ b/services/telemetry/telemetry.go @@ -474,6 +474,7 @@ func (ts *TelemetryService) trackConfig() { "trace": cfg.SqlSettings.Trace, "max_idle_conns": *cfg.SqlSettings.MaxIdleConns, "conn_max_lifetime_milliseconds": *cfg.SqlSettings.ConnMaxLifetimeMilliseconds, + "conn_max_idletime_milliseconds": *cfg.SqlSettings.ConnMaxIdleTimeMilliseconds, "max_open_conns": *cfg.SqlSettings.MaxOpenConns, "data_source_replicas": len(cfg.SqlSettings.DataSourceReplicas), "data_source_search_replicas": len(cfg.SqlSettings.DataSourceSearchReplicas), diff --git a/store/sqlstore/store.go b/store/sqlstore/store.go index e5a1385d3e..57ef62fa27 100644 --- a/store/sqlstore/store.go +++ b/store/sqlstore/store.go @@ -268,6 +268,7 @@ func setupConnection(con_type string, dataSource string, settings *model.SqlSett db.SetMaxIdleConns(*settings.MaxIdleConns) db.SetMaxOpenConns(*settings.MaxOpenConns) db.SetConnMaxLifetime(time.Duration(*settings.ConnMaxLifetimeMilliseconds) * time.Millisecond) + db.SetConnMaxIdleTime(time.Duration(*settings.ConnMaxIdleTimeMilliseconds) * time.Millisecond) var dbmap *gorp.DbMap diff --git a/store/sqlstore/store_test.go b/store/sqlstore/store_test.go index d599f6c66d..282628c59e 100644 --- a/store/sqlstore/store_test.go +++ b/store/sqlstore/store_test.go @@ -515,6 +515,7 @@ func makeSqliteSettings() *model.SqlSettings { dataSource := ":memory:" maxIdleConns := 1 connMaxLifetimeMilliseconds := 3600000 + connMaxIdleTimeMilliseconds := 300000 maxOpenConns := 1 queryTimeout := 5 @@ -523,6 +524,7 @@ func makeSqliteSettings() *model.SqlSettings { DataSource: &dataSource, MaxIdleConns: &maxIdleConns, ConnMaxLifetimeMilliseconds: &connMaxLifetimeMilliseconds, + ConnMaxIdleTimeMilliseconds: &connMaxIdleTimeMilliseconds, MaxOpenConns: &maxOpenConns, QueryTimeout: &queryTimeout, } diff --git a/store/storetest/settings.go b/store/storetest/settings.go index 1d714d5842..c0201c5f71 100644 --- a/store/storetest/settings.go +++ b/store/storetest/settings.go @@ -133,6 +133,7 @@ func databaseSettings(driver, dataSource string) *model.SqlSettings { DataSourceSearchReplicas: []string{}, MaxIdleConns: new(int), ConnMaxLifetimeMilliseconds: new(int), + ConnMaxIdleTimeMilliseconds: new(int), MaxOpenConns: new(int), Trace: model.NewBool(false), AtRestEncryptKey: model.NewString(model.NewRandomString(32)), @@ -140,6 +141,7 @@ func databaseSettings(driver, dataSource string) *model.SqlSettings { } *settings.MaxIdleConns = 10 *settings.ConnMaxLifetimeMilliseconds = 3600000 + *settings.ConnMaxIdleTimeMilliseconds = 300000 *settings.MaxOpenConns = 100 *settings.QueryTimeout = 60