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.

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

config: make EnableSlowLog atomic #30346

Merged
merged 6 commits into from
Dec 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 43 additions & 7 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/pingcap/tidb/util/versioninfo"
tikvcfg "github.com/tikv/client-go/v2/config"
tracing "github.com/uber/jaeger-client-go/config"
atomicutil "go.uber.org/atomic"
"go.uber.org/zap"
)

Expand Down Expand Up @@ -301,6 +302,41 @@ func (b *nullableBool) UnmarshalJSON(data []byte) error {
return err
}

// AtomicBool is a helper type for atomic operations on a boolean value.
type AtomicBool struct {
hawkingrei marked this conversation as resolved.
Show resolved Hide resolved
atomicutil.Bool
}

// NewAtomicBool creates an AtomicBool.
func NewAtomicBool(v bool) *AtomicBool {
return &AtomicBool{*atomicutil.NewBool(v)}
}

// MarshalText implements the encoding.TextMarshaler interface.
func (b AtomicBool) MarshalText() ([]byte, error) {
if b.Load() {
return []byte("true"), nil
}
return []byte("false"), nil
}

// UnmarshalText implements the encoding.TextUnmarshaler interface.
func (b *AtomicBool) UnmarshalText(text []byte) error {
str := string(text)
switch str {
case "", "null":
*b = AtomicBool{*atomicutil.NewBool(false)}
case "true":
*b = AtomicBool{*atomicutil.NewBool(true)}
case "false":
*b = AtomicBool{*atomicutil.NewBool(false)}
default:
*b = AtomicBool{*atomicutil.NewBool(false)}
return errors.New("Invalid value for bool type: " + str)
}
return nil
}

// Log is the log section of config.
type Log struct {
// Log level.
Expand All @@ -320,12 +356,12 @@ type Log struct {
// File log config.
File logutil.FileLogConfig `toml:"file" json:"file"`

EnableSlowLog bool `toml:"enable-slow-log" json:"enable-slow-log"`
SlowQueryFile string `toml:"slow-query-file" json:"slow-query-file"`
SlowThreshold uint64 `toml:"slow-threshold" json:"slow-threshold"`
ExpensiveThreshold uint `toml:"expensive-threshold" json:"expensive-threshold"`
QueryLogMaxLen uint64 `toml:"query-log-max-len" json:"query-log-max-len"`
RecordPlanInSlowLog uint32 `toml:"record-plan-in-slow-log" json:"record-plan-in-slow-log"`
EnableSlowLog AtomicBool `toml:"enable-slow-log" json:"enable-slow-log"`
SlowQueryFile string `toml:"slow-query-file" json:"slow-query-file"`
SlowThreshold uint64 `toml:"slow-threshold" json:"slow-threshold"`
ExpensiveThreshold uint `toml:"expensive-threshold" json:"expensive-threshold"`
QueryLogMaxLen uint64 `toml:"query-log-max-len" json:"query-log-max-len"`
RecordPlanInSlowLog uint32 `toml:"record-plan-in-slow-log" json:"record-plan-in-slow-log"`
}

func (l *Log) getDisableTimestamp() bool {
Expand Down Expand Up @@ -619,7 +655,7 @@ var defaultConf = Config{
DisableTimestamp: nbUnset, // If both options are nbUnset, getDisableTimestamp() returns false
QueryLogMaxLen: logutil.DefaultQueryLogMaxLen,
RecordPlanInSlowLog: logutil.DefaultRecordPlanInSlowLog,
EnableSlowLog: logutil.DefaultTiDBEnableSlowLog,
EnableSlowLog: *NewAtomicBool(logutil.DefaultTiDBEnableSlowLog),
},
Status: Status{
ReportStatus: true,
Expand Down
29 changes: 28 additions & 1 deletion config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package config

import (
"bytes"
"encoding/json"
"os"
"os/user"
Expand All @@ -31,6 +32,32 @@ import (
tracing "github.com/uber/jaeger-client-go/config"
)

func TestAtomicBoolUnmarshal(t *testing.T) {
t.Parallel()
type data struct {
Ab AtomicBool `toml:"ab"`
}
var d data
var firstBuffer bytes.Buffer
_, err := toml.Decode("ab=true", &d)
require.NoError(t, err)
require.True(t, d.Ab.Load())
err = toml.NewEncoder(&firstBuffer).Encode(d)
require.NoError(t, err)
require.Equal(t, "ab = \"true\"\n", firstBuffer.String())
firstBuffer.Reset()

_, err = toml.Decode("ab=false", &d)
require.NoError(t, err)
require.False(t, d.Ab.Load())
err = toml.NewEncoder(&firstBuffer).Encode(d)
require.NoError(t, err)
require.Equal(t, "ab = \"false\"\n", firstBuffer.String())

_, err = toml.Decode("ab = 1", &d)
require.EqualError(t, err, "Invalid value for bool type: 1")
}

func TestNullableBoolUnmarshal(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -153,7 +180,7 @@ func TestConfig(t *testing.T) {
conf.Performance.TxnTotalSizeLimit = 1000
conf.TiKVClient.CommitTimeout = "10s"
conf.TiKVClient.RegionCacheTTL = 600
conf.Log.EnableSlowLog = logutil.DefaultTiDBEnableSlowLog
conf.Log.EnableSlowLog.Store(logutil.DefaultTiDBEnableSlowLog)
configFile := "config.toml"
_, localFile, _, _ := runtime.Caller(0)
configFile = filepath.Join(filepath.Dir(localFile), configFile)
Expand Down
10 changes: 10 additions & 0 deletions config/config_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ func MergeConfigItems(dstConf, newConf *Config) (acceptedItems, rejectedItems []

func mergeConfigItems(dstConf, newConf reflect.Value, fieldPath string) (acceptedItems, rejectedItems []string) {
t := dstConf.Type()
if t.Name() == "AtomicBool" {
if reflect.DeepEqual(dstConf.Interface().(AtomicBool), newConf.Interface().(AtomicBool)) {
return
}
if _, ok := dynamicConfigItems[fieldPath]; ok {
dstConf.Set(newConf)
return []string{fieldPath}, nil
}
return nil, []string{fieldPath}
}
if t.Kind() == reflect.Ptr {
t = t.Elem()
dstConf = dstConf.Elem()
Expand Down
2 changes: 1 addition & 1 deletion config/config_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestCloneConf(t *testing.T) {

c1.Store = "abc"
c1.Port = 2333
c1.Log.EnableSlowLog = !c1.Log.EnableSlowLog
c1.Log.EnableSlowLog.Store(!c1.Log.EnableSlowLog.Load())
c1.RepairTableList = append(c1.RepairTableList, "abc")
require.NotEqual(t, c2.Store, c1.Store)
require.NotEqual(t, c2.Port, c1.Port)
Expand Down
2 changes: 1 addition & 1 deletion executor/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -939,7 +939,7 @@ func (a *ExecStmt) LogSlowQuery(txnTS uint64, succ bool, hasMoreResults bool) {
cfg := config.GetGlobalConfig()
costTime := time.Since(sessVars.StartTime) + sessVars.DurationParse
threshold := time.Duration(atomic.LoadUint64(&cfg.Log.SlowThreshold)) * time.Millisecond
enable := cfg.Log.EnableSlowLog
enable := cfg.Log.EnableSlowLog.Load()
// if the level is Debug, or trace is enabled, print slow logs anyway
force := level <= zapcore.DebugLevel || trace.IsEnabled()
if (!enable || costTime < threshold) && !force {
Expand Down
2 changes: 1 addition & 1 deletion session/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ var bigCount = 10000

func prepareBenchSession() (Session, *domain.Domain, kv.Storage) {
config.UpdateGlobal(func(cfg *config.Config) {
cfg.Log.EnableSlowLog = false
cfg.Log.EnableSlowLog.Store(false)
})

store, err := mockstore.NewMockStore()
Expand Down
4 changes: 2 additions & 2 deletions sessionctx/variable/sysvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -1618,10 +1618,10 @@ var defaultSysVars = []*SysVar{
return BoolToOnOff(enabled), nil
}},
{Scope: ScopeSession, Name: TiDBEnableSlowLog, Value: BoolToOnOff(logutil.DefaultTiDBEnableSlowLog), Type: TypeBool, skipInit: true, SetSession: func(s *SessionVars, val string) error {
config.GetGlobalConfig().Log.EnableSlowLog = TiDBOptOn(val)
config.GetGlobalConfig().Log.EnableSlowLog.Store(TiDBOptOn(val))
return nil
}, GetSession: func(s *SessionVars) (string, error) {
return BoolToOnOff(config.GetGlobalConfig().Log.EnableSlowLog), nil
return BoolToOnOff(config.GetGlobalConfig().Log.EnableSlowLog.Load()), nil
}},
{Scope: ScopeSession, Name: TiDBQueryLogMaxLen, Value: strconv.Itoa(logutil.DefaultQueryLogMaxLen), Type: TypeInt, MinValue: -1, MaxValue: math.MaxInt64, skipInit: true, SetSession: func(s *SessionVars, val string) error {
atomic.StoreUint64(&config.GetGlobalConfig().Log.QueryLogMaxLen, uint64(tidbOptInt64(val, logutil.DefaultQueryLogMaxLen)))
Expand Down
2 changes: 1 addition & 1 deletion sessionctx/variable/sysvar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ func TestInstanceScopedVars(t *testing.T) {

val, err = GetSessionOrGlobalSystemVar(vars, TiDBEnableSlowLog)
require.NoError(t, err)
require.Equal(t, BoolToOnOff(config.GetGlobalConfig().Log.EnableSlowLog), val)
require.Equal(t, BoolToOnOff(config.GetGlobalConfig().Log.EnableSlowLog.Load()), val)

val, err = GetSessionOrGlobalSystemVar(vars, TiDBQueryLogMaxLen)
require.NoError(t, err)
Expand Down