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
Этот коммит содержится в:
Jesse Hallam
2018-04-16 21:23:20 -04:00
коммит произвёл Christopher Speller
родитель ba6f9075c1
Коммит 3a4c7603b3
32 изменённых файлов: 220461 добавлений и 33 удалений

Просмотреть файл

@@ -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

Просмотреть файл

@@ -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)

229
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()))
})
}
}

Просмотреть файл

@@ -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)
//}
}