From 3fa9ef17512b60f1c6834621bb18dd6bbf5466bf Mon Sep 17 00:00:00 2001 From: Krithika Sundararajan Date: Mon, 30 Jan 2023 16:27:07 +0800 Subject: [PATCH 1/3] Add DB connection config --- management-service/config/config.go | 6 ++++ management-service/config/config_test.go | 36 ++++++++++++------- management-service/database/database.go | 17 ++++++++- .../services/configuration_service.go | 3 +- 4 files changed, 47 insertions(+), 15 deletions(-) diff --git a/management-service/config/config.go b/management-service/config/config.go index b74dd86c..691276f0 100644 --- a/management-service/config/config.go +++ b/management-service/config/config.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "time" "github.com/gojek/mlp/api/pkg/instrumentation/newrelic" "github.com/gojek/mlp/api/pkg/instrumentation/sentry" @@ -40,6 +41,11 @@ type DatabaseConfig struct { Password string `default:"xp"` Database string `default:"xp"` MigrationsPath string `default:"file://database/db-migrations"` + + ConnMaxIdleTime time.Duration `default:"0s"` + ConnMaxLifetime time.Duration `default:"0s"` + MaxIdleConns int `default:"0"` + MaxOpenConns int `default:"0"` } // MLPConfig captures the configuration used to connect to the MLP API server diff --git a/management-service/config/config_test.go b/management-service/config/config_test.go index 926f50a1..c1905e78 100644 --- a/management-service/config/config_test.go +++ b/management-service/config/config_test.go @@ -3,6 +3,7 @@ package config import ( "strings" "testing" + "time" "github.com/gojek/mlp/api/pkg/instrumentation/newrelic" "github.com/gojek/mlp/api/pkg/instrumentation/sentry" @@ -11,6 +12,7 @@ import ( ) func TestDefaultConfigs(t *testing.T) { + zeroSecond, _ := time.ParseDuration("0s") emptyInterfaceMap := make(map[string]interface{}) emptyStringMap := make(map[string]string) defaultCfg := Config{ @@ -21,12 +23,16 @@ func TestDefaultConfigs(t *testing.T) { URL: "", }, DbConfig: &DatabaseConfig{ - Host: "localhost", - Port: 5432, - User: "xp", - Password: "xp", - Database: "xp", - MigrationsPath: "file://database/db-migrations", + Host: "localhost", + Port: 5432, + User: "xp", + Password: "xp", + Database: "xp", + MigrationsPath: "file://database/db-migrations", + ConnMaxIdleTime: zeroSecond, + ConnMaxLifetime: zeroSecond, + MaxIdleConns: 0, + MaxOpenConns: 0, }, SegmenterConfig: make(map[string]interface{}), MLPConfig: &MLPConfig{ @@ -64,6 +70,8 @@ func TestDefaultConfigs(t *testing.T) { // TestLoadConfigFiles verifies that when multiple configs are passed in // they are consumed in the correct order func TestLoadConfigFiles(t *testing.T) { + oneSecond, _ := time.ParseDuration("1s") + twoSecond, _ := time.ParseDuration("2s") tests := []struct { name string configFiles []string @@ -81,12 +89,16 @@ func TestLoadConfigFiles(t *testing.T) { URL: "test-authz-server", }, DbConfig: &DatabaseConfig{ - Host: "localhost", - Port: 5432, - User: "admin", - Password: "password", - Database: "xp", - MigrationsPath: "file://test-db-migrations", + Host: "localhost", + Port: 5432, + User: "admin", + Password: "password", + Database: "xp", + MigrationsPath: "file://test-db-migrations", + ConnMaxIdleTime: oneSecond, + ConnMaxLifetime: twoSecond, + MaxIdleConns: 3, + MaxOpenConns: 4, }, SegmenterConfig: map[string]interface{}{ "s2_ids": map[string]interface{}{ diff --git a/management-service/database/database.go b/management-service/database/database.go index c4b716bf..d8bd4a10 100644 --- a/management-service/database/database.go +++ b/management-service/database/database.go @@ -27,10 +27,25 @@ func ConnectionString(cfg *config.DatabaseConfig) string { } func Open(cfg *config.DatabaseConfig) (*gorm.DB, error) { - return gorm.Open(pg.Open(ConnectionString(cfg)), + db, err := gorm.Open(pg.Open(ConnectionString(cfg)), &gorm.Config{ Logger: logger.Default.LogMode(logger.Silent), }) + if err != nil { + return nil, err + } + + // Get the underlying SQL DB and apply connection properties + sqlDB, err := db.DB() + if err != nil { + return nil, err + } + sqlDB.SetConnMaxIdleTime(cfg.ConnMaxIdleTime) + sqlDB.SetConnMaxLifetime(cfg.ConnMaxLifetime) + sqlDB.SetMaxIdleConns(cfg.MaxIdleConns) + sqlDB.SetMaxOpenConns(cfg.MaxOpenConns) + + return db, nil } func Migrate(cfg *config.DatabaseConfig) error { diff --git a/management-service/services/configuration_service.go b/management-service/services/configuration_service.go index c5f634ff..ae721a4d 100644 --- a/management-service/services/configuration_service.go +++ b/management-service/services/configuration_service.go @@ -14,8 +14,7 @@ type configurationService struct { } func NewConfigurationService(cfg *config.Config) ConfigurationService { - var segmenterConfig schema.SegmenterConfig - segmenterConfig = cfg.SegmenterConfig + var segmenterConfig schema.SegmenterConfig = cfg.SegmenterConfig return &configurationService{ treatmentServiceConfig: schema.TreatmentServiceConfig{ From 76ad367ba192776fd89af4a16344bc3e65e84511 Mon Sep 17 00:00:00 2001 From: Krithika Sundararajan Date: Mon, 30 Jan 2023 17:03:37 +0800 Subject: [PATCH 2/3] Add testdata file --- management-service/testdata/config1.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/management-service/testdata/config1.yaml b/management-service/testdata/config1.yaml index 3154a833..2f3b29f6 100644 --- a/management-service/testdata/config1.yaml +++ b/management-service/testdata/config1.yaml @@ -12,6 +12,10 @@ DbConfig: User: user Password: password MigrationsPath: file://test-db-migrations + ConnMaxIdleTime: 1s + ConnMaxLifetime: 2s + MaxIdleConns: 3 + MaxOpenConns: 4 MLPConfig: URL: test-mlp-url From 53a7ed5895773e195bcd9ef4eb6f0ec86e0b200d Mon Sep 17 00:00:00 2001 From: Krithika Sundararajan Date: Mon, 30 Jan 2023 18:03:31 +0800 Subject: [PATCH 3/3] Add a small comment update for the treatment service --- treatment-service/services/experiment_service.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/treatment-service/services/experiment_service.go b/treatment-service/services/experiment_service.go index 8792716f..25af3f36 100644 --- a/treatment-service/services/experiment_service.go +++ b/treatment-service/services/experiment_service.go @@ -61,11 +61,13 @@ func (es *experimentService) GetExperiment( func(matches []*models.ExperimentMatch) []*models.ExperimentMatch { return es.filterByMatchStrength(matches, projectSettings.Segmenters.Names) }, - // Resolve granularity of Segmenters + // Resolve lookup order. At this point, comparing by each segmenter, we should be left with one or more + // experiments which are either all exact or all weak. Where there are multiple transformed values returned + // by the segmenter, we pick the first transformed value that has a match, to filter the pool of experiments. func(matches []*models.ExperimentMatch) []*models.ExperimentMatch { return es.filterByLookupOrder(matches, requestFilter, projectSettings.Segmenters.Names, segmentersTypeMapping) }, - // Resolve tiers - at this point, we should ideally only be left with 1 experiment or 2 + // Resolve tiers. At this point, we should ideally only be left with 1 experiment or 2 // (in different tiers), based on the orthogonality rules enforced by the management service. es.filterByTierPriority, }