-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
*: add session var 'tidb_ddl_reorg_worker_cnt' to control ddl reorg workers count #6441
*: add session var 'tidb_ddl_reorg_worker_cnt' to control ddl reorg workers count #6441
Conversation
…db into add_reorg_worker_cnt_variable
/run-all-tests |
executor/set.go
Outdated
@@ -186,6 +186,8 @@ func (e *SetExecutor) setSysVariable(name string, v *expression.VarAssignment) e | |||
if isoLevel == ast.ReadCommitted { | |||
e.ctx.Txn().SetOption(kv.IsolationLevel, kv.RC) | |||
} | |||
} else if name == variable.TiDBDDLReorgWorkerCount { | |||
domain.GetDomain(e.ctx).DDL().WorkerVars().DDLReorgWorkerCount = sessionVars.DDLReorgWorkerCount |
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.
If this is a session-scope variable, we have to find the instance which is ddl-owner and run the set statement on it. We should make it a global scope variable.
(And actually, it is a server-scope variable.)
@shenli Done |
Please fix the CI. |
sessionctx/variable/tidb_vars.go
Outdated
) | ||
|
||
// Process global variables. | ||
var ( | ||
ProcessGeneralLog uint32 | ||
ProcessGeneralLog uint32 | ||
DDLReorgWorkerCounter int32 = DefTiDBDDLReorgWorkerCount |
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.
s/DDLReorgWorkerCounter/ddlReorgWorkerCounter ?
We use GetDDLReorgWorkerCounter
and SetDDLReorgWorkerCounter
to change its value. So I think we'd better use the lowercase variable.
@@ -531,6 +531,9 @@ func (s *SessionVars) SetSystemVar(name string, val string) error { | |||
s.EnableStreaming = TiDBOptOn(val) | |||
case TiDBOptimizerSelectivityLevel: | |||
s.OptimizerSelectivityLevel = tidbOptPositiveInt32(val, DefTiDBOptimizerSelectivityLevel) | |||
case TiDBDDLReorgWorkerCount: | |||
workerCnt := tidbOptPositiveInt32(val, DefTiDBDDLReorgWorkerCount) |
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.
Do we need to have an upper limit?
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.
Done
Please resolve the conflicts @winkyao . |
@winkyao Please resolve the conflicts. |
ddl/index.go
Outdated
// TODO: make workerCnt can be modified by system variable. | ||
workerCnt := defaultWorkers | ||
// variable.ddlReorgWorkerCounter can be modified by system variable "tidb_ddl_reorg_worker_cnt". | ||
workerCnt := variable.GetDDLReorgWorkerCounter() |
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.
Could we support updating worker counter when the workers are running?
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.
Will be fixed later, not this PR.
Please resolve the conflicts. |
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.
LGTM
/run-all-tests |
LGTM |
/run-all-tests |
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.
LGTM
After this PR, you can use below command to modify the count of ddl reorg workers, to
increase add index concurrency: