diff --git a/pkg/services/sqlstore/database_config.go b/pkg/services/sqlstore/database_config.go index 71d37bd603c..e469455670f 100644 --- a/pkg/services/sqlstore/database_config.go +++ b/pkg/services/sqlstore/database_config.go @@ -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 +} diff --git a/pkg/services/sqlstore/database_config_test.go b/pkg/services/sqlstore/database_config_test.go index 08174d8307e..5dd882f85b6 100644 --- a/pkg/services/sqlstore/database_config_test.go +++ b/pkg/services/sqlstore/database_config_test.go @@ -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` diff --git a/pkg/services/sqlstore/replstore.go b/pkg/services/sqlstore/replstore.go index ff30b10f60b..a4c38f1d508 100644 --- a/pkg/services/sqlstore/replstore.go +++ b/pkg/services/sqlstore/replstore.go @@ -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 } diff --git a/pkg/services/sqlstore/replstore_test.go b/pkg/services/sqlstore/replstore_test.go index 8fa50625cdc..6cb475b3b06 100644 --- a/pkg/services/sqlstore/replstore_test.go +++ b/pkg/services/sqlstore/replstore_test.go @@ -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`