Skip to content

Commit

Permalink
config: make EnableSlowLog atomic (#30346)
Browse files Browse the repository at this point in the history
  • Loading branch information
hawkingrei authored Dec 3, 2021
1 parent 9aa7563 commit 891517f
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 14 deletions.
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 {
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

0 comments on commit 891517f

Please sign in to comment.