From 0bb545eea459c34a7c14146b73b5275f5f36feae Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Tue, 21 Jun 2022 13:51:38 -0600 Subject: [PATCH 1/5] *: add an option to run sql commands only on first bootstrap --- config/config.go | 3 +++ session/bootstrap.go | 31 ++++++++++++++++++++++ session/bootstrap_test.go | 54 +++++++++++++++++++++++++++++++++++++++ tidb-server/main.go | 18 ++++++++++--- 4 files changed, 103 insertions(+), 3 deletions(-) diff --git a/config/config.go b/config/config.go index a701b1501bf15..0bc905cadd851 100644 --- a/config/config.go +++ b/config/config.go @@ -256,6 +256,9 @@ type Config struct { BallastObjectSize int `toml:"ballast-object-size" json:"ballast-object-size"` // EnableGlobalKill indicates whether to enable global kill. EnableGlobalKill bool `toml:"enable-global-kill" json:"enable-global-kill"` + // InitializeSQLFile is a file that will be executed after first bootstrap only. + // It can be used to set GLOBAL system variable values + InitializeSQLFile string `toml:"initialize-sql-file" json:"initialize-sql-file"` // The following items are deprecated. We need to keep them here temporarily // to support the upgrade process. They can be removed in future. diff --git a/session/bootstrap.go b/session/bootstrap.go index 960ca9fa3a625..8fdbeb7bebefc 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -23,6 +23,7 @@ import ( "encoding/hex" "flag" "fmt" + "io/ioutil" osuser "os/user" "runtime/debug" "strconv" @@ -444,6 +445,7 @@ func bootstrap(s Session) { if dom.DDL().OwnerManager().IsOwner() { doDDLWorks(s) doDMLWorks(s) + doBootstrapSQLFile(s) logutil.BgLogger().Info("bootstrap successful", zap.Duration("take time", time.Since(startTime))) return @@ -1971,6 +1973,35 @@ func doDDLWorks(s Session) { mustExecute(s, CreateAdvisoryLocks) } +// doBootstrapSQLFile executes SQL commands in a file as the last stage of bootstrap. +// It is useful for setting the initial value of GLOBAL variables. +func doBootstrapSQLFile(s Session) { + sqlFile := config.GetGlobalConfig().InitializeSQLFile + if sqlFile == "" { + return + } + logutil.BgLogger().Info("executing -initialize-sql-file", zap.String("file", sqlFile)) + b, err := ioutil.ReadFile(sqlFile) + if err != nil { + logutil.BgLogger().Fatal("unable to read InitializeSQLFile", zap.Error(err)) + } + stmts, err := s.Parse(context.Background(), string(b)) + if err != nil { + logutil.BgLogger().Fatal("unable to parse InitializeSQLFile", zap.Error(err)) + } + for _, stmt := range stmts { + rs, err := s.ExecuteStmt(context.Background(), stmt) + if err != nil { + logutil.BgLogger().Warn("InitializeSQLFile error", zap.Error(err)) + } + if rs != nil { + // I don't believe we need to drain the result-set in bootstrap mode + // but if required we can do this here in future. + rs.Close() + } + } +} + // doDMLWorks executes DML statements in bootstrap stage. // All the statements run in a single transaction. // TODO: sanitize. diff --git a/session/bootstrap_test.go b/session/bootstrap_test.go index d5438998cc506..73dc233be2016 100644 --- a/session/bootstrap_test.go +++ b/session/bootstrap_test.go @@ -17,11 +17,13 @@ package session import ( "context" "fmt" + "os" "strconv" "strings" "testing" "github.com/pingcap/tidb/bindinfo" + "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/meta" "github.com/pingcap/tidb/parser/auth" @@ -1025,3 +1027,55 @@ func TestUpgradeToVer85(t *testing.T) { require.NoError(t, r.Close()) mustExec(t, se, "delete from mysql.bind_info where default_db = 'test'") } + +func TestInitializeSQLFile(t *testing.T) { + // We create an initialize-sql-file and then bootstrap the server with it. + // The observed behavior should be that tidb_enable_noop_variables is now + // disabled, and the feature works as expected. + initializeSQLFile, err := os.CreateTemp("", "init.sql") + require.NoError(t, err) + defer func() { + path := initializeSQLFile.Name() + err = initializeSQLFile.Close() + require.NoError(t, err) + err = os.Remove(path) + require.NoError(t, err) + }() + // Implicitly test multi-line init files + _, err = initializeSQLFile.WriteString( + "CREATE DATABASE initsqlfiletest;\n" + + "SET GLOBAL tidb_enable_noop_variables = OFF;\n") + require.NoError(t, err) + + // Create a mock store + // Set the config parameter for initialize sql file + store, err := mockstore.NewMockStore() + require.NoError(t, err) + config.GetGlobalConfig().InitializeSQLFile = initializeSQLFile.Name() + defer func() { + require.NoError(t, store.Close()) + config.GetGlobalConfig().InitializeSQLFile = "" + }() + + // Bootstrap with the InitializeSQLFile config option + dom, err := BootstrapSession(store) + require.NoError(t, err) + defer dom.Close() + se := createSessionAndSetID(t, store) + ctx := context.Background() + r := mustExec(t, se, `SHOW VARIABLES LIKE 'query_cache_type'`) + req := r.NewChunk(nil) + err = r.Next(ctx, req) + require.NoError(t, err) + require.Equal(t, 0, req.NumRows()) // not shown in noopvariables mode + require.NoError(t, r.Close()) + + r = mustExec(t, se, `SHOW VARIABLES LIKE 'tidb_enable_noop_variables'`) + req = r.NewChunk(nil) + err = r.Next(ctx, req) + require.NoError(t, err) + require.Equal(t, 1, req.NumRows()) + row := req.GetRow(0) + require.Equal(t, []byte("OFF"), row.GetBytes(1)) + require.NoError(t, r.Close()) +} diff --git a/tidb-server/main.go b/tidb-server/main.go index 40dd3cacfd5a1..a0e9fb49db97f 100644 --- a/tidb-server/main.go +++ b/tidb-server/main.go @@ -114,6 +114,7 @@ const ( nmInitializeSecure = "initialize-secure" nmInitializeInsecure = "initialize-insecure" + nmInitializeSQLFile = "initialize-sql-file" ) var ( @@ -156,9 +157,10 @@ var ( proxyProtocolNetworks = flag.String(nmProxyProtocolNetworks, "", "proxy protocol networks allowed IP or *, empty mean disable proxy protocol support") proxyProtocolHeaderTimeout = flag.Uint(nmProxyProtocolHeaderTimeout, 5, "proxy protocol header read timeout, unit is second.") - // Security + // Bootstrap and security initializeSecure = flagBoolean(nmInitializeSecure, false, "bootstrap tidb-server in secure mode") initializeInsecure = flagBoolean(nmInitializeInsecure, true, "bootstrap tidb-server in insecure mode") + initializeSQLFile = flag.String(nmInitializeSQLFile, "", "SQL file to execute on first bootstrap") ) func main() { @@ -511,7 +513,7 @@ func overrideConfig(cfg *config.Config) { // Sanity check: can't specify both options if actualFlags[nmInitializeSecure] && actualFlags[nmInitializeInsecure] { - err = fmt.Errorf("the options --initialize-insecure and --initialize-secure are mutually exclusive") + err = fmt.Errorf("the options -initialize-insecure and -initialize-secure are mutually exclusive") terror.MustNil(err) } // The option --initialize-secure=true ensures that a secure bootstrap is used. @@ -527,9 +529,19 @@ func overrideConfig(cfg *config.Config) { // which is not supported on windows. Only the insecure bootstrap // method is supported. if runtime.GOOS == "windows" && cfg.Security.SecureBootstrap { - err = fmt.Errorf("the option --initialize-secure is not supported on Windows") + err = fmt.Errorf("the option -initialize-secure is not supported on Windows") terror.MustNil(err) } + // Initialize SQL File is used to run a set of SQL statements after first bootstrap. + // It is important in the use case that you want to set GLOBAL variables, which + // are persisted to the cluster and not read from a config file. + if actualFlags[nmInitializeSQLFile] { + if _, err := os.Stat(*initializeSQLFile); err != nil { + err = fmt.Errorf("can not access -initialize-sql-file %s", *initializeSQLFile) + terror.MustNil(err) + } + cfg.InitializeSQLFile = *initializeSQLFile + } } func setVersions() { From bdd8f1f2108900515df05325a253bfc443c776a8 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Fri, 23 Dec 2022 08:12:54 -0700 Subject: [PATCH 2/5] Update patch from suggestions --- session/bootstrap.go | 10 +++++++--- session/session.go | 10 ++++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index 478bc2e0657b5..618b88ab7ad0b 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -527,7 +527,7 @@ func bootstrap(s Session) { if dom.DDL().OwnerManager().IsOwner() { doDDLWorks(s) doDMLWorks(s) - doBootstrapSQLFile(s) + runBootstrapSQLFile = true logutil.BgLogger().Info("bootstrap successful", zap.Duration("take time", time.Since(startTime))) return @@ -746,6 +746,9 @@ var currentBootstrapVersion int64 = version109 // DDL owner key's expired time is ManagerSessionTTL seconds, we should wait the time and give more time to have a chance to finish it. var internalSQLTimeout = owner.ManagerSessionTTL + 15 +// whether to run the sql file in bootstrap. +var runBootstrapSQLFile = false + var ( bootstrapVersion = []func(Session, int64){ upgradeToVer2, @@ -2315,6 +2318,7 @@ func doDDLWorks(s Session) { // It is useful for setting the initial value of GLOBAL variables. func doBootstrapSQLFile(s Session) { sqlFile := config.GetGlobalConfig().InitializeSQLFile + ctx := kv.WithInternalSourceType(context.Background(), kv.InternalTxnBootstrap) if sqlFile == "" { return } @@ -2323,12 +2327,12 @@ func doBootstrapSQLFile(s Session) { if err != nil { logutil.BgLogger().Fatal("unable to read InitializeSQLFile", zap.Error(err)) } - stmts, err := s.Parse(context.Background(), string(b)) + stmts, err := s.Parse(ctx, string(b)) if err != nil { logutil.BgLogger().Fatal("unable to parse InitializeSQLFile", zap.Error(err)) } for _, stmt := range stmts { - rs, err := s.ExecuteStmt(context.Background(), stmt) + rs, err := s.ExecuteStmt(ctx, stmt) if err != nil { logutil.BgLogger().Warn("InitializeSQLFile error", zap.Error(err)) } diff --git a/session/session.go b/session/session.go index d358d761560e2..e58d534197582 100644 --- a/session/session.go +++ b/session/session.go @@ -3297,7 +3297,7 @@ func BootstrapSession(store kv.Storage) (*domain.Domain, error) { analyzeConcurrencyQuota := int(config.GetGlobalConfig().Performance.AnalyzePartitionConcurrencyQuota) concurrency := int(config.GetGlobalConfig().Performance.StatsLoadConcurrency) - ses, err := createSessions(store, 9) + ses, err := createSessions(store, 10) if err != nil { return nil, err } @@ -3397,7 +3397,13 @@ func BootstrapSession(store kv.Storage) (*domain.Domain, error) { // setup historical stats worker dom.SetupHistoricalStatsWorker(ses[8]) dom.StartHistoricalStatsWorker() - + if runBootstrapSQLFile { + pm := &privileges.UserPrivileges{ + Handle: dom.PrivilegeHandle(), + } + privilege.BindPrivilegeManager(ses[9], pm) + doBootstrapSQLFile(ses[9]) + } // A sub context for update table stats, and other contexts for concurrent stats loading. cnt := 1 + concurrency syncStatsCtxs, err := createSessions(store, cnt) From 50d7a3a67377db26543c27abdf84c59a10e51748 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Fri, 23 Dec 2022 08:14:51 -0700 Subject: [PATCH 3/5] remove files added by git hook --- metrics/grafana/performance_overview.json | 2 +- metrics/grafana/tidb.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/metrics/grafana/performance_overview.json b/metrics/grafana/performance_overview.json index a8138a289e621..5ee670584f99e 100644 --- a/metrics/grafana/performance_overview.json +++ b/metrics/grafana/performance_overview.json @@ -6503,4 +6503,4 @@ "title": "Test-Cluster-Performance-Overview", "uid": "eDbRZpnWa", "version": 2 -} +} \ No newline at end of file diff --git a/metrics/grafana/tidb.json b/metrics/grafana/tidb.json index a8ee0b1cb8ccd..0e2ce93934f7c 100644 --- a/metrics/grafana/tidb.json +++ b/metrics/grafana/tidb.json @@ -3706,7 +3706,7 @@ "avg": false, "current": false, "max": false, - "min": false, + "min": false, "show": true, "total": false, "values": false From d1a3762ee045eb96ff85b197b6ce691a232ef810 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Fri, 23 Dec 2022 08:37:23 -0700 Subject: [PATCH 4/5] add gosec linter skip --- session/bootstrap.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index 618b88ab7ad0b..98df9b46a8b55 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -2323,7 +2323,7 @@ func doBootstrapSQLFile(s Session) { return } logutil.BgLogger().Info("executing -initialize-sql-file", zap.String("file", sqlFile)) - b, err := ioutil.ReadFile(sqlFile) + b, err := ioutil.ReadFile(sqlFile) //nolint:gosec if err != nil { logutil.BgLogger().Fatal("unable to read InitializeSQLFile", zap.Error(err)) } From e3c8b11faf91530c2e1da18ea8fce4cfeb30fdb1 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Fri, 23 Dec 2022 08:50:33 -0700 Subject: [PATCH 5/5] fix bad merge --- session/bootstrap.go | 4 +++- session/bootstrap_test.go | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index 98df9b46a8b55..9979b49218d75 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -2339,7 +2339,9 @@ func doBootstrapSQLFile(s Session) { if rs != nil { // I don't believe we need to drain the result-set in bootstrap mode // but if required we can do this here in future. - rs.Close() + if err := rs.Close(); err != nil { + logutil.BgLogger().Fatal("unable to close result", zap.Error(err)) + } } } } diff --git a/session/bootstrap_test.go b/session/bootstrap_test.go index d861a1bb1ee63..ab73be00a02d3 100644 --- a/session/bootstrap_test.go +++ b/session/bootstrap_test.go @@ -1080,14 +1080,16 @@ func TestInitializeSQLFile(t *testing.T) { defer dom.Close() se := createSessionAndSetID(t, store) ctx := context.Background() - r := mustExec(t, se, `SHOW VARIABLES LIKE 'query_cache_type'`) + r, err := exec(se, `SHOW VARIABLES LIKE 'query_cache_type'`) + require.NoError(t, err) req := r.NewChunk(nil) err = r.Next(ctx, req) require.NoError(t, err) require.Equal(t, 0, req.NumRows()) // not shown in noopvariables mode require.NoError(t, r.Close()) - r = mustExec(t, se, `SHOW VARIABLES LIKE 'tidb_enable_noop_variables'`) + r, err = exec(se, `SHOW VARIABLES LIKE 'tidb_enable_noop_variables'`) + require.NoError(t, err) req = r.NewChunk(nil) err = r.Next(ctx, req) require.NoError(t, err)