From ef5ac519d9ce0f2ee393d0a7f6a567788e1631f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Thu, 7 May 2020 14:11:05 +0200 Subject: [PATCH] Disable read/search db replicas in TE/E0 (#14400) * Disable read/search db replicas in TE/E0 * fixing tests * Removing unnecesary text. * Updating without-license read-replicas config before store initialization * Reconnecting to database after remove read replicas --- app/server.go | 19 ++++++++++-- app/server_app_adapters.go | 3 +- app/server_test.go | 63 +++++++++++++++++++++++++++++++++++++- config/utils.go | 25 +++++++++------ config/utils_test.go | 4 +-- model/config.go | 2 ++ model/config_test.go | 2 ++ 7 files changed, 101 insertions(+), 17 deletions(-) diff --git a/app/server.go b/app/server.go index 06d01330ec..6c6cbb282d 100644 --- a/app/server.go +++ b/app/server.go @@ -41,6 +41,7 @@ import ( "github.com/mattermost/mattermost-server/v5/services/timezones" "github.com/mattermost/mattermost-server/v5/services/tracing" "github.com/mattermost/mattermost-server/v5/store" + "github.com/mattermost/mattermost-server/v5/store/sqlstore" "github.com/mattermost/mattermost-server/v5/utils" ) @@ -48,6 +49,7 @@ var MaxNotificationsPerChannelDefault int64 = 1000000 type Server struct { Store store.Store + sqlStore *sqlstore.SqlSupplier WebSocketRouter *WebSocketRouter // RootRouter is the starting point for all HTTP requests to the server. @@ -276,11 +278,22 @@ func NewServer(options ...Option) (*Server, error) { license := s.License() - if license == nil && len(s.Config().SqlSettings.DataSourceReplicas) > 1 { - mlog.Warn("More than 1 read replica functionality disabled by current license. Please contact your system administrator about upgrading your enterprise license.") + if license == nil && len(s.Config().SqlSettings.DataSourceReplicas) > 0 { + mlog.Warn("Read replicas functionality disabled by current license. Please contact your system administrator about upgrading your enterprise license.") s.UpdateConfig(func(cfg *model.Config) { - cfg.SqlSettings.DataSourceReplicas = cfg.SqlSettings.DataSourceReplicas[:1] + cfg.SqlSettings.DataSourceReplicas = []string{} }) + s.Store.Close() + s.Store = s.newStore() + } + + if license == nil && len(s.Config().SqlSettings.DataSourceSearchReplicas) > 0 { + mlog.Warn("Search replicas functionality disabled by current license. Please contact your system administrator about upgrading your enterprise license.") + s.UpdateConfig(func(cfg *model.Config) { + cfg.SqlSettings.DataSourceSearchReplicas = []string{} + }) + s.Store.Close() + s.Store = s.newStore() } if license == nil { diff --git a/app/server_app_adapters.go b/app/server_app_adapters.go index eb3252757c..b9ba169d1c 100644 --- a/app/server_app_adapters.go +++ b/app/server_app_adapters.go @@ -61,10 +61,11 @@ func (s *Server) RunOldAppInitialization() error { if s.newStore == nil { s.newStore = func() store.Store { + s.sqlStore = sqlstore.NewSqlSupplier(s.Config().SqlSettings, s.Metrics) return store.NewTimerLayer( searchlayer.NewSearchLayer( localcachelayer.NewLocalCacheLayer( - sqlstore.NewSqlSupplier(s.Config().SqlSettings, s.Metrics), + s.sqlStore, s.Metrics, s.Cluster, s.CacheProvider, diff --git a/app/server_test.go b/app/server_test.go index c6e8b8e59d..2e1284b237 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -6,7 +6,6 @@ package app import ( "bufio" "crypto/tls" - "github.com/mattermost/mattermost-server/v5/mlog" "io/ioutil" "net" "net/http" @@ -16,6 +15,8 @@ import ( "strings" "testing" + "github.com/mattermost/mattermost-server/v5/mlog" + "github.com/mattermost/mattermost-server/v5/config" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/utils/fileutils" @@ -36,6 +37,66 @@ func TestStartServerSuccess(t *testing.T) { require.NoError(t, serverErr) } +func TestReadReplicaDisabledBasedOnLicense(t *testing.T) { + t.Run("Read Replicas with no License", func(t *testing.T) { + s, err := NewServer(func(server *Server) error { + configStore, _ := config.NewFileStore("config.json", true) + server.configStore = configStore + server.UpdateConfig(func(cfg *model.Config) { + cfg.SqlSettings.DataSourceReplicas = []string{*cfg.SqlSettings.DataSource} + }) + return nil + }) + require.NoError(t, err) + require.Equal(t, s.sqlStore.GetMaster(), s.sqlStore.GetReplica()) + require.Len(t, s.Config().SqlSettings.DataSourceReplicas, 0) + }) + + t.Run("Read Replicas With License", func(t *testing.T) { + s, err := NewServer(func(server *Server) error { + configStore, _ := config.NewFileStore("config.json", true) + server.configStore = configStore + server.licenseValue.Store(model.NewTestLicense()) + server.UpdateConfig(func(cfg *model.Config) { + cfg.SqlSettings.DataSourceReplicas = []string{*cfg.SqlSettings.DataSource} + }) + return nil + }) + require.NoError(t, err) + require.NotEqual(t, s.sqlStore.GetMaster(), s.sqlStore.GetReplica()) + require.Len(t, s.Config().SqlSettings.DataSourceReplicas, 1) + }) + + t.Run("Search Replicas with no License", func(t *testing.T) { + s, err := NewServer(func(server *Server) error { + configStore, _ := config.NewFileStore("config.json", true) + server.configStore = configStore + server.UpdateConfig(func(cfg *model.Config) { + cfg.SqlSettings.DataSourceSearchReplicas = []string{*cfg.SqlSettings.DataSource} + }) + return nil + }) + require.NoError(t, err) + require.Equal(t, s.sqlStore.GetMaster(), s.sqlStore.GetSearchReplica()) + require.Len(t, s.Config().SqlSettings.DataSourceSearchReplicas, 0) + }) + + t.Run("Search Replicas With License", func(t *testing.T) { + s, err := NewServer(func(server *Server) error { + configStore, _ := config.NewFileStore("config.json", true) + server.configStore = configStore + server.licenseValue.Store(model.NewTestLicense()) + server.UpdateConfig(func(cfg *model.Config) { + cfg.SqlSettings.DataSourceSearchReplicas = []string{*cfg.SqlSettings.DataSource} + }) + return nil + }) + require.NoError(t, err) + require.NotEqual(t, s.sqlStore.GetMaster(), s.sqlStore.GetSearchReplica()) + require.Len(t, s.Config().SqlSettings.DataSourceSearchReplicas, 1) + }) +} + func TestStartServerRateLimiterCriticalError(t *testing.T) { // Attempt to use Rate Limiter with an invalid config ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{ diff --git a/config/utils.go b/config/utils.go index 387f212bed..5021718695 100644 --- a/config/utils.go +++ b/config/utils.go @@ -35,6 +35,21 @@ func desanitize(actual, target *model.Config) { if *target.SqlSettings.DataSource == model.FAKE_SETTING { *target.SqlSettings.DataSource = *actual.SqlSettings.DataSource } + + if len(target.SqlSettings.DataSourceReplicas) == 1 && target.SqlSettings.DataSourceReplicas[0] == model.FAKE_SETTING { + target.SqlSettings.DataSourceReplicas = make([]string, len(actual.SqlSettings.DataSourceReplicas)) + for i := range target.SqlSettings.DataSourceReplicas { + target.SqlSettings.DataSourceReplicas[i] = actual.SqlSettings.DataSourceReplicas[i] + } + } + + if len(target.SqlSettings.DataSourceSearchReplicas) == 1 && target.SqlSettings.DataSourceSearchReplicas[0] == model.FAKE_SETTING { + target.SqlSettings.DataSourceSearchReplicas = make([]string, len(actual.SqlSettings.DataSourceSearchReplicas)) + for i := range target.SqlSettings.DataSourceSearchReplicas { + target.SqlSettings.DataSourceSearchReplicas[i] = actual.SqlSettings.DataSourceSearchReplicas[i] + } + } + if *target.SqlSettings.AtRestEncryptKey == model.FAKE_SETTING { target.SqlSettings.AtRestEncryptKey = actual.SqlSettings.AtRestEncryptKey } @@ -42,16 +57,6 @@ func desanitize(actual, target *model.Config) { if *target.ElasticsearchSettings.Password == model.FAKE_SETTING { *target.ElasticsearchSettings.Password = *actual.ElasticsearchSettings.Password } - - target.SqlSettings.DataSourceReplicas = make([]string, len(actual.SqlSettings.DataSourceReplicas)) - for i := range target.SqlSettings.DataSourceReplicas { - target.SqlSettings.DataSourceReplicas[i] = actual.SqlSettings.DataSourceReplicas[i] - } - - target.SqlSettings.DataSourceSearchReplicas = make([]string, len(actual.SqlSettings.DataSourceSearchReplicas)) - for i := range target.SqlSettings.DataSourceSearchReplicas { - target.SqlSettings.DataSourceSearchReplicas[i] = actual.SqlSettings.DataSourceSearchReplicas[i] - } } // fixConfig patches invalid or missing data in the configuration, returning true if changed. diff --git a/config/utils_test.go b/config/utils_test.go index 4d883102e0..5678e04558 100644 --- a/config/utils_test.go +++ b/config/utils_test.go @@ -50,8 +50,8 @@ func TestDesanitize(t *testing.T) { target.SqlSettings.DataSource = sToP(model.FAKE_SETTING) target.SqlSettings.AtRestEncryptKey = sToP(model.FAKE_SETTING) target.ElasticsearchSettings.Password = sToP(model.FAKE_SETTING) - target.SqlSettings.DataSourceReplicas = append(target.SqlSettings.DataSourceReplicas, "old_replica0") - target.SqlSettings.DataSourceSearchReplicas = append(target.SqlSettings.DataSourceReplicas, "old_search_replica0") + target.SqlSettings.DataSourceReplicas = []string{model.FAKE_SETTING} + target.SqlSettings.DataSourceSearchReplicas = []string{model.FAKE_SETTING} actualClone := actual.Clone() desanitize(actual, target) diff --git a/model/config.go b/model/config.go index 8c2da57c7c..8c67207213 100644 --- a/model/config.go +++ b/model/config.go @@ -3342,6 +3342,8 @@ func (o *Config) Sanitize() { } *o.SqlSettings.DataSource = FAKE_SETTING + o.SqlSettings.DataSourceReplicas = []string{FAKE_SETTING} + o.SqlSettings.DataSourceSearchReplicas = []string{FAKE_SETTING} *o.SqlSettings.AtRestEncryptKey = FAKE_SETTING *o.ElasticsearchSettings.Password = FAKE_SETTING diff --git a/model/config_test.go b/model/config_test.go index 78a098dfd1..c9151389d7 100644 --- a/model/config_test.go +++ b/model/config_test.go @@ -1239,6 +1239,8 @@ func TestConfigSanitize(t *testing.T) { assert.Equal(t, FAKE_SETTING, *c.EmailSettings.SMTPPassword) assert.Equal(t, FAKE_SETTING, *c.GitLabSettings.Secret) assert.Equal(t, FAKE_SETTING, *c.SqlSettings.DataSource) + assert.Equal(t, []string{FAKE_SETTING}, c.SqlSettings.DataSourceReplicas) + assert.Equal(t, []string{FAKE_SETTING}, c.SqlSettings.DataSourceSearchReplicas) assert.Equal(t, FAKE_SETTING, *c.SqlSettings.AtRestEncryptKey) assert.Equal(t, FAKE_SETTING, *c.ElasticsearchSettings.Password) assert.Equal(t, FAKE_SETTING, c.SqlSettings.DataSourceReplicas[0])