*: replace conf item pprof_sql_cpu with srv var tidb_pprof_sql_cpu
#14385
Conversation
lysu
added
type/enhancement
|
/run-all-tests |
| @@ -706,6 +706,7 @@ var defaultSysVars = []*SysVar{ | |||
| {ScopeGlobal | ScopeSession, TiDBSkipIsolationLevelCheck, BoolToIntStr(DefTiDBSkipIsolationLevelCheck)}, | |||
| /* The following variable is defined as session scope but is actually server scope. */ | |||
| {ScopeSession, TiDBGeneralLog, strconv.Itoa(DefTiDBGeneralLog)}, | |||
| {ScopeSession, TiDBPProfSQLCPU, strconv.Itoa(DefTiDBPProfSQLCPU)}, | |||
There was a problem hiding this comment.
As https://github.com/pingcap/tidb/pull/14312/files#diff-dc417f5322100e2e83a8818c70a25878R810, Is it really in session scope?
There was a problem hiding this comment.
Could we set util.EnablePProfSQLCPU into session.sessionVars?
There was a problem hiding this comment.
there are no benefit to attach a server scope var to per session, but we can move it into variable package...moved
There was a problem hiding this comment.
As https://github.com/pingcap/tidb/pull/14312/files#diff-dc417f5322100e2e83a8818c70a25878R810, Is it really in session scope?
/* The following variable is defined as session scope but is actually server scope. */
the comments above those variable talk some about this and it seems infra-team have plan to make a real server scope variable in roadmap
SunRunAway
added
status/can-merge
|
Your auto merge job has been accepted, waiting for 14389 |
|
/run-all-tests |
|
cherry pick to release-3.0 failed |
What problem does this PR solve?
This PR improves #14312 because modifying the config item and reloading it is hard to use, both for human or external tools.
What is changed and how it works?
Replace the config item
[performance] pprof_sql_cpuwith a server-scope variabletidb_pprof_sql_cpu.We can execute
set tidb_pprof_sql_cpu = 1before go profiler collection and executeset tidb_pprof_sql_cpu = 0after tuning is finished.Check List
Tests
Code changes
Side effects
Related changes
Release note
This change is