Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: support variable log_bin #9343

Merged
merged 21 commits into from
Feb 20, 2019
Merged

*: support variable log_bin #9343

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
62606d6
Support variable `log_bin`
aliiohs Feb 18, 2019
3eb96d3
Support variable `log_bin`,Because the scope is not determined，so the…
aliiohs Feb 18, 2019
b3af774
set ScopeNone as the scope of `log_bin`
aliiohs Feb 18, 2019
195eaa8
set `log_bin in setGlobalVars method
aliiohs Feb 18, 2019
af59973
Support variable `log_bin`
aliiohs Feb 18, 2019
b682f93
add a Blank in Comment of variable 'LogBin'
aliiohs Feb 18, 2019
6b8a504
abstract a method transfer bool to status str ,like "ON" or "OFF"
aliiohs Feb 18, 2019
13d63e9
add Comment for BoolToStatusStr
aliiohs Feb 18, 2019
8d8d596
add Comment for BoolToStatusStr
aliiohs Feb 18, 2019
4566842
import Alias
aliiohs Feb 18, 2019
054cb18
please add a blank after ,
aliiohs Feb 18, 2019
feea00e
add a test case for 'log_bin'
aliiohs Feb 19, 2019
f862b4c
move merge case 'LogBin' to case 'TiDBEnableTablePartition'
aliiohs Feb 19, 2019
82e7be3
'LogBin' still keep an independent case
aliiohs Feb 19, 2019
fe01745
change LogBin Status from "ON" or "OFF" to "0" or "1".
aliiohs Feb 19, 2019
c1c1748
Merge branch 'master' into support_log_bin_2019_02_18_1
aliiohs Feb 19, 2019
65943a9
change status of'LogBin' from 'on/off' to '0/1'.
aliiohs Feb 19, 2019
4f7b4b0
eep the sys libs seperate with the 3rd libs.
aliiohs Feb 20, 2019
bcbe263
Merge branch 'master' into support_log_bin_2019_02_18_1
jackysp Feb 20, 2019
889fbd9
Merge branch 'master' into support_log_bin_2019_02_18_1
ngaut Feb 20, 2019
1cde77b
Merge branch 'master' into support_log_bin_2019_02_18_1
zz-jason Feb 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions executor/set_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package executor_test

import (
"context"
"github.com/pingcap/tidb/config"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the sys libs seperate with the 3rd libs. Please move it down to line 20.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i already finished it


. "github.com/pingcap/check"
"github.com/pingcap/parser/terror"
Expand Down Expand Up @@ -231,6 +232,9 @@ func (s *testSuite2) TestSetVar(c *C) {
tk.MustExec("set @@sql_log_bin = on")
tk.MustQuery(`select @@session.sql_log_bin;`).Check(testkit.Rows("1"))

tk.MustQuery(`select @@global.log_bin;`).Check(testkit.Rows(variable.BoolToStatusStr(config.GetGlobalConfig().Binlog.Enable)))
tk.MustQuery(`select @@log_bin;`).Check(testkit.Rows(variable.BoolToStatusStr(config.GetGlobalConfig().Binlog.Enable)))

tk.MustExec("set @@tidb_general_log = 1")
tk.MustExec("set @@tidb_general_log = 0")

Expand Down
12 changes: 11 additions & 1 deletion sessionctx/variable/sysvar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,14 @@ func boolToIntStr(b bool) string {
return "0"
}

// BoolToStatusStr converts bool to string, for example "ON" or "OFF".
func BoolToStatusStr(b bool) string {
if b {
return "ON"
}
return "OFF"
}

// we only support MySQL now
var defaultSysVars = []*SysVar{
{ScopeGlobal, "gtid_mode", "OFF"},
Expand Down Expand Up @@ -386,7 +394,7 @@ var defaultSysVars = []*SysVar{
{ScopeGlobal, "log_syslog_include_pid", ""},
{ScopeSession, "last_insert_id", ""},
{ScopeNone, "innodb_ft_cache_size", "8000000"},
{ScopeNone, "log_bin", "OFF"},
{ScopeNone, LogBin, "OFF"},
{ScopeGlobal, "innodb_disable_sort_file_cache", "OFF"},
{ScopeGlobal, "log_error_verbosity", ""},
{ScopeNone, "performance_schema_hosts_size", "100"},
Expand Down Expand Up @@ -741,6 +749,8 @@ const (
InnodbLockWaitTimeout = "innodb_lock_wait_timeout"
// SQLLogBin is the name for 'sql_log_bin' system variable.
SQLLogBin = "sql_log_bin"
// LogBin is the name for 'log_bin' system variable.
LogBin = "log_bin"
// MaxSortLength is the name for 'max_sort_length' system variable.
MaxSortLength = "max_sort_length"
// MaxSpRecursionDepth is the name for 'max_sp_recursion_depth' system variable.
Expand Down
9 changes: 9 additions & 0 deletions sessionctx/variable/varsutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,15 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string,
return "0", nil
}
return value, ErrWrongValueForVar.GenWithStackByArgs(name, value)
case LogBin:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we put it to line 335? Then we can remove line344-351.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LogBin need to return "ON" or "OFF", not "0" or "1"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zimulala so i will merge 'LogBin' and 'TiDBEnableTablePartition'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I mean you can merge LogBin with GeneralLog. TiDBEnableTablePartition has the value of AUTO, which is different from LogBin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return value is "0" or “1”, is it expected? @zimulala

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's OK. We can handle LogBin like SQLLogBin. Then we can remove BoolToStatusStr.
We will handle #9357 in the next PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, i will change the code later. @zimulala

if strings.EqualFold(value, "ON") || value == "1" {
return "ON", nil
}
if strings.EqualFold(value, "OFF") || value == "0" {
return "OFF", nil
}
return value, ErrWrongValueForVar.GenWithStackByArgs(name, value)

case AutocommitVar, TiDBSkipUTF8Check, TiDBOptAggPushDown,
TiDBOptInSubqToJoinAndAgg,
TiDBBatchInsert, TiDBDisableTxnAutoRetry, TiDBEnableStreaming,
Expand Down
1 change: 1 addition & 0 deletions tidb-server/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ func setGlobalVars() {

variable.SysVars[variable.TIDBMemQuotaQuery].Value = strconv.FormatInt(cfg.MemQuotaQuery, 10)
variable.SysVars["lower_case_table_names"].Value = strconv.Itoa(cfg.LowerCaseTableNames)
variable.SysVars[variable.LogBin].Value = variable.BoolToStatusStr(config.GetGlobalConfig().Binlog.Enable)

// For CI environment we default enable prepare-plan-cache.
plannercore.SetPreparedPlanCache(config.CheckTableBeforeDrop || cfg.PreparedPlanCache.Enabled)
Expand Down