MM-10020: avoid duplicating unique indexes (#8587)

* unit test TestGet(Search)Replica

This adds partial, testing-focused support for SQLite, as well as removing
some translated log messages that required initializing i18n just for
testing.

* avoid returning master twice when no replicas are configured

* remove duplicate indexes

* unit test GetAllConns

4 files changed, 265 insertions, 16 deletions

diff --git a/store/sqlstore/preference_store.go b/store/sqlstore/preference_store.go
index a97c3aea7..791b51d2f 100644
--- a/store/sqlstore/preference_store.go
+++ b/store/sqlstore/preference_store.go
@@ -11,7 +11,6 @@ import (
 
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/store"
-	"github.com/mattermost/mattermost-server/utils"
 )
 
 type SqlPreferenceStore struct {
@@ -39,7 +38,7 @@ func (s SqlPreferenceStore) CreateIndexesIfNotExists() {
 }
 
 func (s SqlPreferenceStore) DeleteUnusedFeatures() {
-	l4g.Debug(utils.T("store.sql_preference.delete_unused_features.debug"))
+	l4g.Debug("Deleting any unused pre-release features")
 
 	sql := `DELETE
 		FROM Preferences
diff --git a/store/sqlstore/supplier.go b/store/sqlstore/supplier.go
index 5e43ee0f0..99b35a664 100644
--- a/store/sqlstore/supplier.go
+++ b/store/sqlstore/supplier.go
@@ -59,6 +59,8 @@ const (
 	EXIT_REMOVE_INDEX_MYSQL          = 122
 	EXIT_REMOVE_INDEX_MISSING        = 123
 	EXIT_REMOVE_TABLE                = 134
+	EXIT_CREATE_INDEX_SQLITE         = 135
+	EXIT_REMOVE_INDEX_SQLITE         = 136
 )
 
 type SqlSupplierOldStores struct {
@@ -185,7 +187,7 @@ func (s *SqlSupplier) Next() store.LayeredStoreSupplier {
 func setupConnection(con_type string, dataSource string, settings *model.SqlSettings) *gorp.DbMap {
 	db, err := dbsql.Open(*settings.DriverName, dataSource)
 	if err != nil {
-		l4g.Critical(utils.T("store.sql.open_conn.critical"), err)
+		l4g.Critical("Failed to open SQL connection to err:%v", err.Error())
 		time.Sleep(time.Second)
 		os.Exit(EXIT_DB_OPEN)
 	}
@@ -217,7 +219,7 @@ func setupConnection(con_type string, dataSource string, settings *model.SqlSett
 
 	connectionTimeout := time.Duration(*settings.QueryTimeout) * time.Second
 
-	if *settings.DriverName == "sqlite3" {
+	if *settings.DriverName == model.DATABASE_DRIVER_SQLITE {
 		dbmap = &gorp.DbMap{Db: db, TypeConverter: mattermConverter{}, Dialect: gorp.SqliteDialect{}, QueryTimeout: connectionTimeout}
 	} else if *settings.DriverName == model.DATABASE_DRIVER_MYSQL {
 		dbmap = &gorp.DbMap{Db: db, TypeConverter: mattermConverter{}, Dialect: gorp.MySQLDialect{Engine: "InnoDB", Encoding: "UTF8MB4"}, QueryTimeout: connectionTimeout}
@@ -239,19 +241,14 @@ func setupConnection(con_type string, dataSource string, settings *model.SqlSett
 func (s *SqlSupplier) initConnection() {
 	s.master = setupConnection("master", *s.settings.DataSource, s.settings)
 
-	if len(s.settings.DataSourceReplicas) == 0 {
-		s.replicas = make([]*gorp.DbMap, 1)
-		s.replicas[0] = s.master
-	} else {
+	if len(s.settings.DataSourceReplicas) > 0 {
 		s.replicas = make([]*gorp.DbMap, len(s.settings.DataSourceReplicas))
 		for i, replica := range s.settings.DataSourceReplicas {
 			s.replicas[i] = setupConnection(fmt.Sprintf("replica-%v", i), replica, s.settings)
 		}
 	}
 
-	if len(s.settings.DataSourceSearchReplicas) == 0 {
-		s.searchReplicas = s.replicas
-	} else {
+	if len(s.settings.DataSourceSearchReplicas) > 0 {
 		s.searchReplicas = make([]*gorp.DbMap, len(s.settings.DataSourceSearchReplicas))
 		for i, replica := range s.settings.DataSourceSearchReplicas {
 			s.searchReplicas[i] = setupConnection(fmt.Sprintf("search-replica-%v", i), replica, s.settings)
@@ -273,11 +270,19 @@ func (ss *SqlSupplier) GetMaster() *gorp.DbMap {
 }
 
 func (ss *SqlSupplier) GetSearchReplica() *gorp.DbMap {
+	if len(ss.settings.DataSourceSearchReplicas) == 0 {
+		return ss.GetReplica()
+	}
+
 	rrNum := atomic.AddInt64(&ss.srCounter, 1) % int64(len(ss.searchReplicas))
 	return ss.searchReplicas[rrNum]
 }
 
 func (ss *SqlSupplier) GetReplica() *gorp.DbMap {
+	if len(ss.settings.DataSourceReplicas) == 0 {
+		return ss.GetMaster()
+	}
+
 	rrNum := atomic.AddInt64(&ss.rrCounter, 1) % int64(len(ss.replicas))
 	return ss.replicas[rrNum]
 }
@@ -287,7 +292,6 @@ func (ss *SqlSupplier) TotalMasterDbConnections() int {
 }
 
 func (ss *SqlSupplier) TotalReadDbConnections() int {
-
 	if len(ss.settings.DataSourceReplicas) == 0 {
 		return 0
 	}
@@ -598,8 +602,7 @@ func (ss *SqlSupplier) createIndexIfNotExists(indexName string, tableName string
 
 		_, err := ss.GetMaster().ExecNoTimeout(query)
 		if err != nil {
-			l4g.Critical(utils.T("store.sql.create_index.critical"), errExists)
-			l4g.Critical(utils.T("store.sql.create_index.critical"), err)
+			l4g.Critical("Failed to create index %v, %v", errExists, err)
 			time.Sleep(time.Second)
 			os.Exit(EXIT_CREATE_INDEX_POSTGRES)
 		}
@@ -623,10 +626,17 @@ func (ss *SqlSupplier) createIndexIfNotExists(indexName string, tableName string
 
 		_, err = ss.GetMaster().ExecNoTimeout("CREATE  " + uniqueStr + fullTextIndex + " INDEX " + indexName + " ON " + tableName + " (" + strings.Join(columnNames, ", ") + ")")
 		if err != nil {
-			l4g.Critical(utils.T("store.sql.create_index.critical"), err)
+			l4g.Critical("Failed to create index %v", err)
 			time.Sleep(time.Second)
 			os.Exit(EXIT_CREATE_INDEX_FULL_MYSQL)
 		}
+	} else if ss.DriverName() == model.DATABASE_DRIVER_SQLITE {
+		_, err := ss.GetMaster().ExecNoTimeout("CREATE INDEX IF NOT EXISTS " + indexName + " ON " + tableName + " (" + strings.Join(columnNames, ", ") + ")")
+		if err != nil {
+			l4g.Critical("Failed to create index %v", err)
+			time.Sleep(time.Second)
+			os.Exit(EXIT_CREATE_INDEX_SQLITE)
+		}
 	} else {
 		l4g.Critical(utils.T("store.sql.create_index_missing_driver.critical"))
 		time.Sleep(time.Second)
@@ -672,6 +682,13 @@ func (ss *SqlSupplier) RemoveIndexIfExists(indexName string, tableName string) b
 			time.Sleep(time.Second)
 			os.Exit(EXIT_REMOVE_INDEX_MYSQL)
 		}
+	} else if ss.DriverName() == model.DATABASE_DRIVER_SQLITE {
+		_, err := ss.GetMaster().ExecNoTimeout("DROP INDEX IF EXISTS " + indexName)
+		if err != nil {
+			l4g.Critical(utils.T("store.sql.remove_index.critical"), err)
+			time.Sleep(time.Second)
+			os.Exit(EXIT_REMOVE_INDEX_SQLITE)
+		}
 	} else {
 		l4g.Critical(utils.T("store.sql.create_index_missing_driver.critical"))
 		time.Sleep(time.Second)
diff --git a/store/sqlstore/supplier_test.go b/store/sqlstore/supplier_test.go
new file mode 100644
index 000000000..5aacfbe6a
--- /dev/null
+++ b/store/sqlstore/supplier_test.go
@@ -0,0 +1,229 @@
+package sqlstore_test
+
+import (
+	"testing"
+
+	"github.com/mattermost/gorp"
+	_ "github.com/mattn/go-sqlite3"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/mattermost/mattermost-server/model"
+	"github.com/mattermost/mattermost-server/store/sqlstore"
+)
+
+func TestGetReplica(t *testing.T) {
+	t.Parallel()
+	testCases := []struct {
+		Description              string
+		DataSourceReplicas       []string
+		DataSourceSearchReplicas []string
+	}{
+		{
+			"no replicas",
+			[]string{},
+			[]string{},
+		},
+		{
+			"one source replica",
+			[]string{":memory:"},
+			[]string{},
+		},
+		{
+			"multiple source replicas",
+			[]string{":memory:", ":memory:", ":memory:"},
+			[]string{},
+		},
+		{
+			"one source search replica",
+			[]string{},
+			[]string{":memory:"},
+		},
+		{
+			"multiple source search replicas",
+			[]string{},
+			[]string{":memory:", ":memory:", ":memory:"},
+		},
+		{
+			"one source replica, one source search replica",
+			[]string{":memory:"},
+			[]string{":memory:"},
+		},
+		{
+			"one source replica, multiple source search replicas",
+			[]string{":memory:"},
+			[]string{":memory:", ":memory:", ":memory:"},
+		},
+		{
+			"multiple source replica, one source search replica",
+			[]string{":memory:", ":memory:", ":memory:"},
+			[]string{":memory:"},
+		},
+		{
+			"multiple source replica, multiple source search replicas",
+			[]string{":memory:", ":memory:", ":memory:"},
+			[]string{":memory:", ":memory:", ":memory:"},
+		},
+	}
+
+	for _, testCase := range testCases {
+		testCase := testCase
+		t.Run(testCase.Description, func(t *testing.T) {
+			t.Parallel()
+
+			driverName := model.DATABASE_DRIVER_SQLITE
+			dataSource := ":memory:"
+			maxIdleConns := 1
+			maxOpenConns := 1
+			queryTimeout := 5
+
+			settings := model.SqlSettings{
+				DriverName:               &driverName,
+				DataSource:               &dataSource,
+				MaxIdleConns:             &maxIdleConns,
+				MaxOpenConns:             &maxOpenConns,
+				QueryTimeout:             &queryTimeout,
+				DataSourceReplicas:       testCase.DataSourceReplicas,
+				DataSourceSearchReplicas: testCase.DataSourceSearchReplicas,
+			}
+			supplier := sqlstore.NewSqlSupplier(settings, nil)
+
+			replicas := make(map[*gorp.DbMap]bool)
+			for i := 0; i < 5; i++ {
+				replicas[supplier.GetReplica()] = true
+			}
+
+			searchReplicas := make(map[*gorp.DbMap]bool)
+			for i := 0; i < 5; i++ {
+				searchReplicas[supplier.GetSearchReplica()] = true
+			}
+
+			if len(testCase.DataSourceReplicas) > 0 {
+				// If replicas were defined, ensure none are the master.
+				assert.Len(t, replicas, len(testCase.DataSourceReplicas))
+
+				for replica := range replicas {
+					assert.NotEqual(t, supplier.GetMaster(), replica)
+				}
+
+			} else if assert.Len(t, replicas, 1) {
+				// Otherwise ensure the replicas contains only the master.
+				for replica := range replicas {
+					assert.Equal(t, supplier.GetMaster(), replica)
+				}
+			}
+
+			if len(testCase.DataSourceSearchReplicas) > 0 {
+				// If search replicas were defined, ensure none are the master nor the replicas.
+				assert.Len(t, searchReplicas, len(testCase.DataSourceSearchReplicas))
+
+				for searchReplica := range searchReplicas {
+					assert.NotEqual(t, supplier.GetMaster(), searchReplica)
+					for replica := range replicas {
+						assert.NotEqual(t, searchReplica, replica)
+					}
+				}
+
+			} else if len(testCase.DataSourceReplicas) > 0 {
+				// If no search replicas were defined, but replicas were, ensure they are equal.
+				assert.Equal(t, replicas, searchReplicas)
+
+			} else if assert.Len(t, searchReplicas, 1) {
+				// Otherwise ensure the search replicas contains the master.
+				for searchReplica := range searchReplicas {
+					assert.Equal(t, supplier.GetMaster(), searchReplica)
+				}
+			}
+		})
+	}
+}
+
+func TestGetAllConns(t *testing.T) {
+	t.Parallel()
+	testCases := []struct {
+		Description              string
+		DataSourceReplicas       []string
+		DataSourceSearchReplicas []string
+		ExpectedNumConnections   int
+	}{
+		{
+			"no replicas",
+			[]string{},
+			[]string{},
+			1,
+		},
+		{
+			"one source replica",
+			[]string{":memory:"},
+			[]string{},
+			2,
+		},
+		{
+			"multiple source replicas",
+			[]string{":memory:", ":memory:", ":memory:"},
+			[]string{},
+			4,
+		},
+		{
+			"one source search replica",
+			[]string{},
+			[]string{":memory:"},
+			1,
+		},
+		{
+			"multiple source search replicas",
+			[]string{},
+			[]string{":memory:", ":memory:", ":memory:"},
+			1,
+		},
+		{
+			"one source replica, one source search replica",
+			[]string{":memory:"},
+			[]string{":memory:"},
+			2,
+		},
+		{
+			"one source replica, multiple source search replicas",
+			[]string{":memory:"},
+			[]string{":memory:", ":memory:", ":memory:"},
+			2,
+		},
+		{
+			"multiple source replica, one source search replica",
+			[]string{":memory:", ":memory:", ":memory:"},
+			[]string{":memory:"},
+			4,
+		},
+		{
+			"multiple source replica, multiple source search replicas",
+			[]string{":memory:", ":memory:", ":memory:"},
+			[]string{":memory:", ":memory:", ":memory:"},
+			4,
+		},
+	}
+
+	for _, testCase := range testCases {
+		testCase := testCase
+		t.Run(testCase.Description, func(t *testing.T) {
+			t.Parallel()
+
+			driverName := model.DATABASE_DRIVER_SQLITE
+			dataSource := ":memory:"
+			maxIdleConns := 1
+			maxOpenConns := 1
+			queryTimeout := 5
+
+			settings := model.SqlSettings{
+				DriverName:               &driverName,
+				DataSource:               &dataSource,
+				MaxIdleConns:             &maxIdleConns,
+				MaxOpenConns:             &maxOpenConns,
+				QueryTimeout:             &queryTimeout,
+				DataSourceReplicas:       testCase.DataSourceReplicas,
+				DataSourceSearchReplicas: testCase.DataSourceSearchReplicas,
+			}
+			supplier := sqlstore.NewSqlSupplier(settings, nil)
+
+			assert.Equal(t, testCase.ExpectedNumConnections, len(supplier.GetAllConns()))
+		})
+	}
+}
diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go
index 882ee48ba..059d1a866 100644
--- a/store/sqlstore/upgrade.go
+++ b/store/sqlstore/upgrade.go
@@ -87,7 +87,7 @@ func UpgradeDatabase(sqlStore SqlStore) {
 			os.Exit(EXIT_VERSION_SAVE_MISSING)
 		}
 
-		l4g.Info(utils.T("store.sql.schema_set.info"), model.CurrentVersion)
+		l4g.Info("The database schema has been set to version %v", model.CurrentVersion)
 	}
 
 	// If we're not on the current version then it's too old to be upgraded
@@ -415,6 +415,10 @@ func UpgradeDatabaseToVersion410(sqlStore SqlStore) {
 	// TODO: Uncomment following condition when version 4.10.0 is released
 	//if shouldPerformUpgrade(sqlStore, VERSION_4_9_0, VERSION_4_10_0) {
 
+	sqlStore.RemoveIndexIfExists("Name_2", "Channels")
+	sqlStore.RemoveIndexIfExists("Name_2", "Emoji")
+	sqlStore.RemoveIndexIfExists("ClientId_2", "OAuthAccessData")
+
 	//	saveSchemaVersion(sqlStore, VERSION_4_10_0)
 	//}
 }