refactor replCfg to look more like plugins/plugin config (#91142)

* refactor replCfg to look more like plugins/plugin config
* validateReplicaConfigs must handle inconsistencies in type names due to the WithHooks suffix
This commit is contained in:
Kristin Laemmert 2024-07-30 12:09:56 -04:00 committed by GitHub
parent 4c71cadd5f
commit ac0b4bb34d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 63 additions and 16 deletions

View File

@ -236,7 +236,7 @@ func buildExtraConnectionString(sep rune, urlQueryParams map[string][]string) st
return sb.String()
}
func validateReplicaConfigs(primary *DatabaseConfig, cfgs []*DatabaseConfig) error {
func validateReplicaConfigs(primary *DatabaseConfig, cfgs []DatabaseConfig) error {
if cfgs == nil {
return errors.New("cfg cannot be nil")
}
@ -256,8 +256,9 @@ func validateReplicaConfigs(primary *DatabaseConfig, cfgs []*DatabaseConfig) err
}
// Verify that every database is the same type and version, and that it matches the primary database.
// The database Yype may include a "withHooks" suffix, which is used to differentiate drivers for instrumentation and ignored for the purpose of this check.
for _, cfg := range cfgs {
if cfg.Type != primary.Type {
if databaseDriverFromName(cfg.Type) != databaseDriverFromName(primary.Type) {
result = errors.Join(result, fmt.Errorf("the replicas must have the same database type as the primary database (%s != %s)", primary.Type, cfg.Type))
break // Only need to report this once
}
@ -265,3 +266,19 @@ func validateReplicaConfigs(primary *DatabaseConfig, cfgs []*DatabaseConfig) err
return result
}
// databaseDriverFromName strips any suffixes from the driver type that are not relevant to the database driver.
// This is used to remove the "WithHooks" or "ReplWithHooks" suffixes which are used to differentiate drivers for instrumentation.
func databaseDriverFromName(driverTy string) string {
if strings.HasPrefix(driverTy, migrator.MySQL) {
return migrator.MySQL
}
if strings.HasPrefix(driverTy, migrator.Postgres) {
return migrator.Postgres
}
if strings.HasPrefix(driverTy, migrator.SQLite) {
return migrator.SQLite
}
// default
return driverTy
}

View File

@ -237,6 +237,22 @@ func TestValidateReplicaConfigs(t *testing.T) {
require.NoError(t, err)
})
t.Run("valid but awkward config", func(t *testing.T) {
inicfg, err := ini.Load([]byte(testReplCfg))
require.NoError(t, err)
cfg, err := setting.NewCfgFromINIFile(inicfg)
require.NoError(t, err)
dbCfgs, err := NewRODatabaseConfigs(cfg, nil)
require.NoError(t, err)
// The primary is mysql, but the replicas are mysqlWithHooks. This can
// occur when some but not all the replicas (or primary) have
// instrument_queries enabled
err = validateReplicaConfigs(&DatabaseConfig{Type: "mysqlWithHooks"}, dbCfgs)
require.NoError(t, err)
})
t.Run("invalid config: primary database type mismatch", func(t *testing.T) {
// valid repl config, the issue is that the primary has a different type
inicfg, err := ini.Load([]byte(testReplCfg))
@ -284,9 +300,15 @@ name = grafana
user = grafana
password = password
host = 127.0.0.1:3306
[database_replicas.one] =
[database_replica.one]
name = grafana
user = grafana
password = password
type = mysql
host = 127.0.0.1:3306
[database_replicas.two] =
[database_replica.two]
name = grafana
user = grafana
password = password
type = postgres
host = 127.0.0.1:3308`

View File

@ -99,7 +99,7 @@ func ProvideServiceWithReadReplica(primary *SQLStore, cfg *setting.Cfg,
replCfg.Type = WrapDatabaseReplDriverWithHooks(replCfg.Type, uint(i), tracer)
}
s, err := newReadOnlySQLStore(cfg, replCfg, features, bus, tracer)
s, err := newReadOnlySQLStore(cfg, &replCfg, features, bus, tracer)
if err != nil {
return nil, err
}
@ -190,13 +190,13 @@ func (ss *SQLStore) initReadOnlyEngine(engine *xorm.Engine) error {
}
// NewRODatabaseConfig creates a new read-only database configuration.
func NewRODatabaseConfigs(cfg *setting.Cfg, features featuremgmt.FeatureToggles) ([]*DatabaseConfig, error) {
func NewRODatabaseConfigs(cfg *setting.Cfg, features featuremgmt.FeatureToggles) ([]DatabaseConfig, error) {
if cfg == nil {
return nil, errors.New("cfg cannot be nil")
}
// if only one replica is configured in the database_replicas section, use it as the default
defaultReplCfg := &DatabaseConfig{}
// If one replica is configured in the database_replicas section, use it as the default
defaultReplCfg := DatabaseConfig{}
if err := defaultReplCfg.readConfigSection(cfg, "database_replicas"); err != nil {
return nil, err
}
@ -204,13 +204,13 @@ func NewRODatabaseConfigs(cfg *setting.Cfg, features featuremgmt.FeatureToggles)
if err != nil {
return nil, err
}
ret := []*DatabaseConfig{defaultReplCfg}
ret := []DatabaseConfig{defaultReplCfg}
// Check for additional replicas as children of the database_replicas section (e.g. database_replicas.one, database_replicas.cheetara)
repls := cfg.Raw.Section("database_replicas")
// Check for individual replicas in the database_replica section (e.g. database_replica.one, database_replica.cheetara)
repls := cfg.Raw.Section("database_replica")
if len(repls.ChildSections()) > 0 {
for _, sec := range repls.ChildSections() {
replCfg := &DatabaseConfig{}
replCfg := DatabaseConfig{}
if err := replCfg.parseConfigIni(sec); err != nil {
return nil, err
}

View File

@ -40,11 +40,11 @@ func TestNewRODatabaseConfig(t *testing.T) {
dbCfgs, err := NewRODatabaseConfigs(cfg, nil)
require.NoError(t, err)
require.Len(t, dbCfgs, 3)
var connStr = func(port int) string {
return fmt.Sprintf("grafana:password@tcp(127.0.0.1:%d)/grafana?collation=utf8mb4_unicode_ci&allowNativePasswords=true&clientFoundRows=true", port)
}
for i, c := range dbCfgs {
if !cmp.Equal(c.ConnectionString, connStr(i+3306)) {
t.Errorf("wrong result for connection string %d.\nGot: %s,\nWant: %s", i, c.ConnectionString, connStr(i+3306))
@ -60,7 +60,15 @@ name = grafana
user = grafana
password = password
host = 127.0.0.1:3306
[database_replicas.one] =
[database_replica.one]
host = 127.0.0.1:3307
[database_replicas.two] =
host = 127.0.0.1:3308`
type = mysql
name = grafana
user = grafana
password = password
[database_replica.two]
host = 127.0.0.1:3308
type = mysql
name = grafana
user = grafana
password = password`