diff --git a/server/channels/app/options.go b/server/channels/app/options.go index 51ef683233..fd74616fa3 100644 --- a/server/channels/app/options.go +++ b/server/channels/app/options.go @@ -8,6 +8,7 @@ import ( "github.com/mattermost/mattermost/server/public/shared/mlog" "github.com/mattermost/mattermost/server/v8/channels/app/platform" "github.com/mattermost/mattermost/server/v8/channels/store" + "github.com/mattermost/mattermost/server/v8/channels/store/sqlstore" "github.com/mattermost/mattermost/server/v8/config" "github.com/mattermost/mattermost/server/v8/einterfaces" "github.com/mattermost/mattermost/server/v8/platform/shared/filestore" @@ -33,6 +34,13 @@ func StoreOverrideWithCache(override store.Store) Option { } } +func StoreOption(option sqlstore.Option) Option { + return func(s *Server) error { + s.platformOptions = append(s.platformOptions, platform.StoreOption(option)) + return nil + } +} + // Config applies the given config dsn, whether a path to config.json // or a database connection string. It receives as well a set of // custom defaults that will be applied for any unset property of the @@ -110,8 +118,10 @@ func SkipPostInitialization() Option { } } -type AppOption func(a *App) -type AppOptionCreator func() []AppOption +type ( + AppOption func(a *App) + AppOptionCreator func() []AppOption +) func ServerConnector(ch *Channels) AppOption { return func(a *App) { diff --git a/server/channels/app/platform/options.go b/server/channels/app/platform/options.go index 519e917c53..418185f041 100644 --- a/server/channels/app/platform/options.go +++ b/server/channels/app/platform/options.go @@ -12,6 +12,7 @@ import ( "github.com/mattermost/mattermost/server/public/shared/mlog" "github.com/mattermost/mattermost/server/v8/channels/store" "github.com/mattermost/mattermost/server/v8/channels/store/localcachelayer" + "github.com/mattermost/mattermost/server/v8/channels/store/sqlstore" "github.com/mattermost/mattermost/server/v8/config" "github.com/mattermost/mattermost/server/v8/einterfaces" "github.com/mattermost/mattermost/server/v8/platform/shared/filestore" @@ -63,6 +64,16 @@ func StoreOverrideWithCache(override store.Store) Option { } } +// StoreOption allows passing options when constructing the store. +// +// This option has no effect if StoreOverride or StoreOverrideWithCache is also set. +func StoreOption(option sqlstore.Option) Option { + return func(ps *PlatformService) error { + ps.storeOptions = append(ps.storeOptions, option) + return nil + } +} + // Config applies the given config dsn, whether a path to config.json // or a database connection string. It receives as well a set of // custom defaults that will be applied for any unset property of the diff --git a/server/channels/app/platform/service.go b/server/channels/app/platform/service.go index 93b3a8806d..3289a15c71 100644 --- a/server/channels/app/platform/service.go +++ b/server/channels/app/platform/service.go @@ -38,9 +38,10 @@ import ( // responsible for non-entity related functionalities that are required // by a product such as database access, configuration access, licensing etc. type PlatformService struct { - sqlStore *sqlstore.SqlStore - Store store.Store - newStore func() (store.Store, error) + sqlStore *sqlstore.SqlStore + Store store.Store + newStore func() (store.Store, error) + storeOptions []sqlstore.Option WebSocketRouter *WebSocketRouter @@ -232,7 +233,7 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) { // Timer layer // | // Cache layer - ps.sqlStore, err = sqlstore.New(ps.Config().SqlSettings, ps.Log(), ps.metricsIFace) + ps.sqlStore, err = sqlstore.New(ps.Config().SqlSettings, ps.Log(), ps.metricsIFace, ps.storeOptions...) if err != nil { return nil, err } diff --git a/server/channels/app/plugin_api_tests/test_db_driver/main.go b/server/channels/app/plugin_api_tests/test_db_driver/main.go index 8016cb1bb3..46fe043b2e 100644 --- a/server/channels/app/plugin_api_tests/test_db_driver/main.go +++ b/server/channels/app/plugin_api_tests/test_db_driver/main.go @@ -33,7 +33,7 @@ func (p *MyPlugin) MessageWillBePosted(_ *plugin.Context, _ *model.Post) (*model rctx := request.TestContext(p.t) settings := p.API.GetUnsanitizedConfig().SqlSettings settings.Trace = model.NewPointer(false) - store, err := sqlstore.New(settings, rctx.Logger(), nil) + store, err := sqlstore.New(settings, rctx.Logger(), nil, sqlstore.DisableMorphLogging()) if err != nil { panic(err) } diff --git a/server/channels/store/localcachelayer/layer_test.go b/server/channels/store/localcachelayer/layer_test.go index b02c2f5659..48d54e57be 100644 --- a/server/channels/store/localcachelayer/layer_test.go +++ b/server/channels/store/localcachelayer/layer_test.go @@ -108,7 +108,7 @@ func initStores(logger mlog.LoggerIFace) { eg.Go(func() error { var err error - st.SqlStore, err = sqlstore.New(*st.SqlSettings, logger, nil) + st.SqlStore, err = sqlstore.New(*st.SqlSettings, logger, nil, sqlstore.DisableMorphLogging()) if err != nil { return err } diff --git a/server/channels/store/searchlayer/layer_test.go b/server/channels/store/searchlayer/layer_test.go index fa0a1b0b78..39f13545aa 100644 --- a/server/channels/store/searchlayer/layer_test.go +++ b/server/channels/store/searchlayer/layer_test.go @@ -28,7 +28,7 @@ func TestUpdateConfigRace(t *testing.T) { driverName = model.DatabaseDriverPostgres } settings := storetest.MakeSqlSettings(driverName, false) - store, err := sqlstore.New(*settings, logger, nil) + store, err := sqlstore.New(*settings, logger, nil, sqlstore.DisableMorphLogging()) require.NoError(t, err) cfg := &model.Config{} diff --git a/server/channels/store/sqlstore/migrate.go b/server/channels/store/sqlstore/migrate.go index 853fff0022..05ecb9b999 100644 --- a/server/channels/store/sqlstore/migrate.go +++ b/server/channels/store/sqlstore/migrate.go @@ -6,6 +6,7 @@ package sqlstore import ( "context" "fmt" + "io" "log" "path" "sort" @@ -60,7 +61,7 @@ func NewMigrator(settings model.SqlSettings, logger mlog.LoggerIFace, dryRun boo return nil, fmt.Errorf("error while checking DB collation: %w", err) } - engine, err := ss.initMorph(dryRun) + engine, err := ss.initMorph(dryRun, true) if err != nil { return nil, fmt.Errorf("failed to initialize morph: %w", err) } @@ -95,7 +96,7 @@ func (m *Migrator) GetFileName(plan *models.Plan) (string, error) { return fmt.Sprintf("migration_plan_%d_%d", from, to), nil } -func (ss *SqlStore) initMorph(dryRun bool) (*morph.Morph, error) { +func (ss *SqlStore) initMorph(dryRun, enableLogging bool) (*morph.Morph, error) { assets := db.Assets() assetsList, err := assets.ReadDir(path.Join("migrations", ss.DriverName())) @@ -149,8 +150,15 @@ func (ss *SqlStore) initMorph(dryRun bool) (*morph.Morph, error) { return nil, err } + var logWriter io.Writer + if enableLogging { + logWriter = &morphWriter{} + } else { + logWriter = io.Discard + } + opts := []morph.EngineOption{ - morph.WithLogger(log.New(&morphWriter{}, "", log.Lshortfile)), + morph.WithLogger(log.New(logWriter, "", log.Lshortfile)), morph.WithLock("mm-lock-key"), morph.SetStatementTimeoutInSeconds(*ss.settings.MigrationsStatementTimeoutSeconds), morph.SetDryRun(dryRun), @@ -164,8 +172,8 @@ func (ss *SqlStore) initMorph(dryRun bool) (*morph.Morph, error) { return engine, nil } -func (ss *SqlStore) migrate(direction migrationDirection, dryRun bool) error { - engine, err := ss.initMorph(dryRun) +func (ss *SqlStore) migrate(direction migrationDirection, dryRun, enableMorphLogging bool) error { + engine, err := ss.initMorph(dryRun, enableMorphLogging) if err != nil { return err } diff --git a/server/channels/store/sqlstore/migrate_test.go b/server/channels/store/sqlstore/migrate_test.go index be6458313e..44810d896e 100644 --- a/server/channels/store/sqlstore/migrate_test.go +++ b/server/channels/store/sqlstore/migrate_test.go @@ -31,7 +31,7 @@ func TestUpAndDownMigrations(t *testing.T) { require.NoError(t, err) defer store.Close() - err = store.migrate(migrationsDirectionDown, false) + err = store.migrate(migrationsDirectionDown, false, true) assert.NoError(t, err, "downing migrations should not error") }) } diff --git a/server/channels/store/sqlstore/store.go b/server/channels/store/sqlstore/store.go index 8c49510ecf..a7df22c79e 100644 --- a/server/channels/store/sqlstore/store.go +++ b/server/channels/store/sqlstore/store.go @@ -147,6 +147,7 @@ type SqlStore struct { isBinaryParam bool pgDefaultTextSearchConfig string skipMigrations bool + disableMorphLogging bool quitMonitor chan struct{} wgMonitor *sync.WaitGroup @@ -159,6 +160,13 @@ func SkipMigrations() Option { } } +func DisableMorphLogging() Option { + return func(s *SqlStore) error { + s.disableMorphLogging = true + return nil + } +} + func New(settings model.SqlSettings, logger mlog.LoggerIFace, metrics einterfaces.MetricsInterface, options ...Option) (*SqlStore, error) { store := &SqlStore{ rrCounter: 0, @@ -200,7 +208,7 @@ func New(settings model.SqlSettings, logger mlog.LoggerIFace, metrics einterface } if !store.skipMigrations { - err = store.migrate(migrationsDirectionUp, false) + err = store.migrate(migrationsDirectionUp, false, !store.disableMorphLogging) if err != nil { return nil, errors.Wrap(err, "failed to apply database migrations") } @@ -642,7 +650,6 @@ func (ss *SqlStore) DoesTableExist(tableName string) bool { `SELECT count(relname) FROM pg_class WHERE relname=$1`, strings.ToLower(tableName), ) - if err != nil { mlog.Fatal("Failed to check if table exists", mlog.Err(err)) } @@ -661,7 +668,6 @@ func (ss *SqlStore) DoesTableExist(tableName string) bool { `, tableName, ) - if err != nil { mlog.Fatal("Failed to check if table exists", mlog.Err(err)) } @@ -684,7 +690,6 @@ func (ss *SqlStore) DoesColumnExist(tableName string, columnName string) bool { strings.ToLower(tableName), strings.ToLower(columnName), ) - if err != nil { if err.Error() == "pq: relation \""+strings.ToLower(tableName)+"\" does not exist" { return false @@ -708,7 +713,6 @@ func (ss *SqlStore) DoesColumnExist(tableName string, columnName string) bool { tableName, columnName, ) - if err != nil { mlog.Fatal("Failed to check if column exists", mlog.Err(err)) } @@ -730,7 +734,6 @@ func (ss *SqlStore) DoesTriggerExist(triggerName string) bool { WHERE tgname = $1 `, triggerName) - if err != nil { mlog.Fatal("Failed to check if trigger exists", mlog.Err(err)) } @@ -747,7 +750,6 @@ func (ss *SqlStore) DoesTriggerExist(triggerName string) bool { trigger_schema = DATABASE() AND trigger_name = ? `, triggerName) - if err != nil { mlog.Fatal("Failed to check if trigger exists", mlog.Err(err)) } diff --git a/server/channels/store/sqlstore/store_test.go b/server/channels/store/sqlstore/store_test.go index 64bd551538..55156dd928 100644 --- a/server/channels/store/sqlstore/store_test.go +++ b/server/channels/store/sqlstore/store_test.go @@ -781,7 +781,7 @@ func TestReplicaLagQuery(t *testing.T) { require.NoError(t, store.initConnection()) store.stores.post = newSqlPostStore(store, mockMetrics) - err = store.migrate(migrationsDirectionUp, false) + err = store.migrate(migrationsDirectionUp, false, true) require.NoError(t, err) defer store.Close() @@ -839,8 +839,10 @@ func TestInvalidReplicaLagDataSource(t *testing.T) { } } -var errDriverMismatch = errors.New("database drivers mismatch") -var errDriverUnsupported = errors.New("database driver not supported") +var ( + errDriverMismatch = errors.New("database drivers mismatch") + errDriverUnsupported = errors.New("database driver not supported") +) func makeSqlSettings(driver string) (*model.SqlSettings, error) { // When running under CI, only one database engine container is launched diff --git a/server/channels/store/sqlstore/utils.go b/server/channels/store/sqlstore/utils.go index 22ae4bac4d..9d31c18e26 100644 --- a/server/channels/store/sqlstore/utils.go +++ b/server/channels/store/sqlstore/utils.go @@ -156,7 +156,7 @@ func wrapBinaryParamStringMap(ok bool, props model.StringMap) model.StringMap { type morphWriter struct{} func (l *morphWriter) Write(in []byte) (int, error) { - mlog.Debug(string(in)) + mlog.Debug(strings.TrimSpace(string(in))) return len(in), nil } diff --git a/server/channels/store/storetest/settings.go b/server/channels/store/storetest/settings.go index 9263930852..fc5cc80c51 100644 --- a/server/channels/store/storetest/settings.go +++ b/server/channels/store/storetest/settings.go @@ -5,7 +5,6 @@ package storetest import ( "database/sql" - "flag" "fmt" "net/url" "os" @@ -35,20 +34,6 @@ func getEnv(name, defaultValue string) string { return defaultValue } -func log(message string) { - verbose := false - if verboseFlag := flag.Lookup("test.v"); verboseFlag != nil { - verbose = verboseFlag.Value.String() != "" - } - if verboseFlag := flag.Lookup("v"); verboseFlag != nil { - verbose = verboseFlag.Value.String() != "" - } - - if verbose { - fmt.Println(message) - } -} - func getDefaultMysqlDSN() string { if os.Getenv("IS_CI") == "true" { return strings.ReplaceAll(defaultMysqlDSN, "localhost", "mysql") @@ -69,7 +54,6 @@ func MySQLSettings(withReplica bool) *model.SqlSettings { dsn := os.Getenv("TEST_DATABASE_MYSQL_DSN") if dsn == "" { dsn = getDefaultMysqlDSN() - mlog.Info("No TEST_DATABASE_MYSQL_DSN override, using default", mlog.String("default_dsn", dsn)) } else { mlog.Info("Using TEST_DATABASE_MYSQL_DSN override", mlog.String("dsn", dsn)) } @@ -96,7 +80,6 @@ func PostgreSQLSettings() *model.SqlSettings { dsn := os.Getenv("TEST_DATABASE_POSTGRESQL_DSN") if dsn == "" { dsn = getDefaultPostgresqlDSN() - mlog.Info("No TEST_DATABASE_POSTGRESQL_DSN override, using default", mlog.String("default_dsn", dsn)) } else { mlog.Info("Using TEST_DATABASE_POSTGRESQL_DSN override", mlog.String("dsn", dsn)) } @@ -190,7 +173,7 @@ func databaseSettings(driver, dataSource string) *model.SqlSettings { // execAsRoot executes the given sql as root against the testing database func execAsRoot(settings *model.SqlSettings, sqlCommand string) error { var dsn string - var driver = *settings.DriverName + driver := *settings.DriverName switch driver { case model.DatabaseDriverMysql: @@ -260,14 +243,13 @@ func MakeSqlSettings(driver string, withReplica bool) *model.SqlSettings { panic("unsupported driver " + driver) } - log("Created temporary " + driver + " database " + dbName) settings.ReplicaMonitorIntervalSeconds = model.NewPointer(5) return settings } func CleanupSqlSettings(settings *model.SqlSettings) { - var driver = *settings.DriverName + driver := *settings.DriverName var dbName string switch driver { @@ -282,6 +264,4 @@ func CleanupSqlSettings(settings *model.SqlSettings) { if err := execAsRoot(settings, "DROP DATABASE "+dbName); err != nil { panic("failed to drop temporary database " + dbName + ": " + err.Error()) } - - log("Dropped temporary database " + dbName) } diff --git a/server/channels/testlib/helper.go b/server/channels/testlib/helper.go index 716f7ae06f..8a29ca45b2 100644 --- a/server/channels/testlib/helper.go +++ b/server/channels/testlib/helper.go @@ -143,7 +143,7 @@ func (h *MainHelper) setupStore(withReadReplica bool) { h.ClusterInterface = &FakeClusterInterface{} var err error - h.SQLStore, err = sqlstore.New(*h.Settings, h.Logger, nil) + h.SQLStore, err = sqlstore.New(*h.Settings, h.Logger, nil, sqlstore.DisableMorphLogging()) if err != nil { panic(err) } @@ -160,7 +160,7 @@ func (h *MainHelper) ToggleReplicasOff() { lic := h.SQLStore.GetLicense() var err error - h.SQLStore, err = sqlstore.New(*h.Settings, h.Logger, nil) + h.SQLStore, err = sqlstore.New(*h.Settings, h.Logger, nil, sqlstore.DisableMorphLogging()) if err != nil { panic(err) } @@ -175,7 +175,7 @@ func (h *MainHelper) ToggleReplicasOn() { lic := h.SQLStore.GetLicense() var err error - h.SQLStore, err = sqlstore.New(*h.Settings, h.Logger, nil) + h.SQLStore, err = sqlstore.New(*h.Settings, h.Logger, nil, sqlstore.DisableMorphLogging()) if err != nil { panic(err) } diff --git a/server/platform/services/searchengine/bleveengine/bleve_test.go b/server/platform/services/searchengine/bleveengine/bleve_test.go index 4dc1a0bee0..01b808488d 100644 --- a/server/platform/services/searchengine/bleveengine/bleve_test.go +++ b/server/platform/services/searchengine/bleveengine/bleve_test.go @@ -55,7 +55,7 @@ func (s *BleveEngineTestSuite) setupStore() { s.SQLSettings = storetest.MakeSqlSettings(driverName, false) var err error - s.SQLStore, err = sqlstore.New(*s.SQLSettings, s.Context.Logger(), nil) + s.SQLStore, err = sqlstore.New(*s.SQLSettings, s.Context.Logger(), nil, sqlstore.DisableMorphLogging()) if err != nil { s.Require().FailNow("Cannot initialize store: %s", err.Error()) } diff --git a/server/public/utils/sql/sql_utils.go b/server/public/utils/sql/sql_utils.go index 1434ade67a..87d86e36d6 100644 --- a/server/public/utils/sql/sql_utils.go +++ b/server/public/utils/sql/sql_utils.go @@ -64,13 +64,15 @@ func SetupConnection(logger mlog.LoggerIFace, connType string, dataSource string mlog.String("dataSource", sanitized), ) - for i := 0; i < attempts; i++ { - logger.Info("Pinging SQL") + for attempt := 1; attempt <= attempts; attempt++ { + if attempt > 1 { + logger.Info("Pinging SQL", mlog.Int("attempt", attempt)) + } ctx, cancel := context.WithTimeout(context.Background(), DBPingTimeout) defer cancel() err = db.PingContext(ctx) if err != nil { - if i == attempts-1 { + if attempt == attempts { return nil, err } logger.Error("Failed to ping DB", mlog.Float("retrying in seconds", DBConnRetrySleep.Seconds()), mlog.Err(err))