*: Move tikv gc configuration to sysvars #21988
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,8 +94,27 @@ var ( | |
| sessionExecuteCompileDurationGeneral = metrics.SessionExecuteCompileDuration.WithLabelValues(metrics.LblGeneral) | ||
| sessionExecuteParseDurationInternal = metrics.SessionExecuteParseDuration.WithLabelValues(metrics.LblInternal) | ||
| sessionExecuteParseDurationGeneral = metrics.SessionExecuteParseDuration.WithLabelValues(metrics.LblGeneral) | ||
|
|
||
| tiKVGCAutoConcurrency = "tikv_gc_auto_concurrency" | ||
| ) | ||
|
|
||
| var gcVariableComments = map[string]string{ | ||
| variable.TiDBGCRunInterval: "GC run interval, at least 10m, in Go format.", | ||
| variable.TiDBGCLifetime: "All versions within life time will not be collected by GC, at least 10m, in Go format.", | ||
| variable.TiDBGCConcurrency: "How many goroutines used to do GC parallel, [1, 128], default 2", | ||
| variable.TiDBGCEnable: "Current GC enable status", | ||
| tiKVGCAutoConcurrency: "Let TiDB pick the concurrency automatically. If set false, tikv_gc_concurrency will be used", | ||
| variable.TiDBGCScanLockMode: "Mode of scanning locks, \"physical\" or \"legacy\"", | ||
| } | ||
|
|
||
| var gcVariableMap = map[string]string{ | ||
| variable.TiDBGCRunInterval: "tikv_gc_run_interval", | ||
| variable.TiDBGCLifetime: "tikv_gc_life_time", | ||
| variable.TiDBGCConcurrency: "tikv_gc_concurrency", | ||
| variable.TiDBGCEnable: "tikv_gc_enable", | ||
| variable.TiDBGCScanLockMode: "tikv_gc_scan_lock_mode", | ||
| } | ||
|
|
||
| // Session context, it is consistent with the lifecycle of a client connection. | ||
| type Session interface { | ||
| sessionctx.Context | ||
|
|
@@ -988,6 +1007,14 @@ func (s *session) getExecRet(ctx sessionctx.Context, sql string) (string, error) | |
| return value, nil | ||
| } | ||
|
|
||
| func (s *session) varFromTiDBTable(name string) bool { | ||
| switch name { | ||
| case variable.TiDBGCConcurrency, variable.TiDBGCEnable, variable.TiDBGCRunInterval, variable.TiDBGCLifetime, variable.TiDBGCScanLockMode: | ||
| return true | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| // GetAllSysVars implements GlobalVarAccessor.GetAllSysVars interface. | ||
| func (s *session) GetAllSysVars() (map[string]string, error) { | ||
| if s.Value(sessionctx.Initing) != nil { | ||
|
|
@@ -1002,7 +1029,13 @@ func (s *session) GetAllSysVars() (map[string]string, error) { | |
| ret := make(map[string]string, len(rows)) | ||
| for _, r := range rows { | ||
| k, v := r.GetString(0), r.GetString(1) | ||
| if s.varFromTiDBTable(k) { | ||
| if v, err = s.getTiDBTableValue(k, v); err != nil { | ||
| ret[k] = v | ||
| } | ||
| } else { | ||
| ret[k] = v | ||
| } | ||
|
There was a problem hiding this comment. If we decides not to introduce the concept of TiKV global sysvar, the function's name seems better to be updated as well. There was a problem hiding this comment. It passes every variable through the function, but for non-tikv vars, it is effectively a noop. The functions don't need to be exported, so I should fix that. I have also renamed them to There was a problem hiding this comment. I prefer they are invoked only when the variable is a GC configuration item, which I think has better readability. There was a problem hiding this comment. OK, I've made this change. PTAL :-) |
||
| } | ||
| return ret, nil | ||
| } | ||
|
|
@@ -1029,6 +1062,10 @@ func (s *session) GetGlobalSysVar(name string) (string, error) { | |
| } | ||
| return "", err | ||
| } | ||
| // Fetch mysql.tidb values if required | ||
| if s.varFromTiDBTable(name) { | ||
| return s.getTiDBTableValue(name, sysVar) | ||
| } | ||
| return sysVar, nil | ||
| } | ||
|
|
||
|
|
@@ -1056,13 +1093,111 @@ func (s *session) SetGlobalSysVar(name, value string) error { | |
| return err | ||
| } | ||
| name = strings.ToLower(name) | ||
| // update mysql.tidb if required. | ||
| if s.varFromTiDBTable(name) { | ||
| if err = s.setTiDBTableValue(name, sVal); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| variable.CheckDeprecationSetSystemVar(s.sessionVars, name) | ||
| sql := fmt.Sprintf(`REPLACE %s.%s VALUES ('%s', '%s');`, | ||
| mysql.SystemDB, mysql.GlobalVariablesTable, name, escapeUserString(sVal)) | ||
| _, _, err = s.ExecRestrictedSQL(sql) | ||
| return err | ||
| } | ||
|
|
||
| // escape user supplied string for internal SQL. Not safe for all cases, since it doesn't | ||
| // handle quote-type, sql-mode, character set breakout. | ||
| func escapeUserString(str string) string { | ||
| return strings.ReplaceAll(str, `'`, `\'`) | ||
| } | ||
|
There was a problem hiding this comment. This still doesn't looks safe enough 😕 I think we need better way to do this in the future There was a problem hiding this comment. Yes, I agree. I searched and couldn't find anything reliable that could be imported in |
||
|
|
||
| // setTiDBTableValue handles tikv_* sysvars which need to update mysql.tidb | ||
| // for backwards compatibility. Validation has already been performed. | ||
| func (s *session) setTiDBTableValue(name, val string) error { | ||
| if name == variable.TiDBGCConcurrency { | ||
| autoConcurrency := "false" | ||
| if val == "-1" { | ||
| autoConcurrency = "true" | ||
| } | ||
| sql := fmt.Sprintf(`INSERT INTO mysql.tidb (variable_name, variable_value, comment) VALUES ('%[1]s', '%[2]s', '%[3]s') | ||
| ON DUPLICATE KEY UPDATE variable_value = '%[2]s'`, tiKVGCAutoConcurrency, autoConcurrency, gcVariableComments[name]) | ||
MyonKeminta marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| _, _, err := s.ExecRestrictedSQL(sql) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
| val = onOffToTrueFalse(val) | ||
| sql := fmt.Sprintf(`INSERT INTO mysql.tidb (variable_name, variable_value, comment) VALUES ('%[1]s', '%[2]s', '%[3]s') | ||
| ON DUPLICATE KEY UPDATE variable_value = '%[2]s'`, gcVariableMap[name], escapeUserString(val), gcVariableComments[name]) | ||
| _, _, err := s.ExecRestrictedSQL(sql) | ||
| return err | ||
| } | ||
|
|
||
| // In mysql.tidb the convention has been to store the string value "true"/"false", | ||
| // but sysvars use the convention ON/OFF. | ||
| func trueFalseToOnOff(str string) string { | ||
| if strings.EqualFold("true", str) { | ||
| return variable.BoolOn | ||
| } else if strings.EqualFold("false", str) { | ||
| return variable.BoolOff | ||
| } | ||
| return str | ||
| } | ||
|
|
||
| // In mysql.tidb the convention has been to store the string value "true"/"false", | ||
| // but sysvars use the convention ON/OFF. | ||
| func onOffToTrueFalse(str string) string { | ||
| if strings.EqualFold("ON", str) { | ||
| return "true" | ||
| } else if strings.EqualFold("OFF", str) { | ||
| return "false" | ||
| } | ||
| return str | ||
| } | ||
|
|
||
| // getTiDBTableValue handles tikv_* sysvars which need | ||
| // to read from mysql.tidb for backwards compatibility. | ||
| func (s *session) getTiDBTableValue(name, val string) (string, error) { | ||
| if name == variable.TiDBGCConcurrency { | ||
| // Check if autoconcurrency is set | ||
| sql := fmt.Sprintf(`SELECT VARIABLE_VALUE FROM mysql.tidb WHERE VARIABLE_NAME='%s';`, tiKVGCAutoConcurrency) | ||
| autoConcurrencyVal, err := s.getExecRet(s, sql) | ||
| if err == nil && strings.EqualFold(autoConcurrencyVal, "true") { | ||
| return "-1", nil // convention for "AUTO" | ||
| } | ||
| } | ||
| sql := fmt.Sprintf(`SELECT VARIABLE_VALUE FROM mysql.tidb WHERE VARIABLE_NAME='%s';`, gcVariableMap[name]) | ||
| tblValue, err := s.getExecRet(s, sql) | ||
| if err != nil { | ||
| return val, nil // mysql.tidb value does not exist. | ||
| } | ||
| // Run validation on the tblValue. This will return an error if it can't be validated, | ||
| // but will also make it more consistent: disTribuTeD -> DISTRIBUTED etc | ||
| tblValue = trueFalseToOnOff(tblValue) | ||
| validatedVal, err := variable.ValidateSetSystemVar(s.sessionVars, name, tblValue, variable.ScopeGlobal) | ||
| if err != nil { | ||
| logutil.Logger(context.Background()).Warn("restoring sysvar value since validating mysql.tidb value failed", | ||
| zap.Error(err), | ||
| zap.String("name", name), | ||
| zap.String("tblName", gcVariableMap[name]), | ||
| zap.String("tblValue", tblValue), | ||
| zap.String("restoredValue", val)) | ||
| sql := fmt.Sprintf(`REPLACE INTO mysql.tidb (variable_name, variable_value, comment) | ||
| VALUES ('%s', '%s', '%s')`, gcVariableMap[name], escapeUserString(val), gcVariableComments[name]) | ||
| _, _, err = s.ExecRestrictedSQL(sql) | ||
| return val, err | ||
| } | ||
| if validatedVal != val { | ||
| // The sysvar value is out of sync. | ||
| sql := fmt.Sprintf(`REPLACE %s.%s VALUES ('%s', '%s');`, | ||
| mysql.SystemDB, mysql.GlobalVariablesTable, gcVariableMap[name], escapeUserString(validatedVal)) | ||
| _, _, err = s.ExecRestrictedSQL(sql) | ||
| return validatedVal, err | ||
| } | ||
| return validatedVal, nil | ||
| } | ||
|
|
||
| func (s *session) ensureFullGlobalStats() error { | ||
| rows, _, err := s.ExecRestrictedSQL(`select count(1) from information_schema.tables t where t.create_options = 'partitioned' | ||
| and not exists (select 1 from mysql.stats_meta m where m.table_id = t.tidb_table_id)`) | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this behavior changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was caused by the changes in
util/gcutil/gcutil.go. It now reads the sysvars to get the state of if GC is running, which check the system tables but use the default if there are no values.