-
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
config, sysvar: add config instance.enable_ddl
and sysvar tidb_enable_ddl
#35425
Conversation
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
/run-unit-test |
Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/af356ccca1af73d3d44eba855ca0cfd193906fec |
/cc morgo bb7133 zimulala xiongjiwei |
ddl/ddl.go
Outdated
// Otherwise, we needn't do that. | ||
if RunWorker { | ||
if config.GetGlobalConfig().Instance.EnableDDL.Load() { |
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.
seems we still can not disable the node to be owner after setting it to false, because the ddl started from here on the server initialized
sessionctx/variable/sysvar.go
Outdated
// EnableDDL indicates whether the tidb-server runs DDL statements, | ||
EnableDDL = "enable_ddl" |
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.
Can we name it TiDBEnableDDL
? For PluginDir
: this variable is actually a MySQL compatible variable, but for EnableDDL
, MySQL has no such notion of this.
ddl/ddl.go
Outdated
|
||
d.wg.Run(d.limitDDLJobs) | ||
d.sessPool = newSessionPool(ctxPool, d.store) | ||
d.workers = make(map[workerType]*worker, 2) |
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.
Then you will still enable the runner. I suggest you to make *ddl
having a real Start
and Stop
. Start()
at L470 is more like an one-shot Run()/Begin()
.
/merge |
@CbcWestwolf: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: f47dd96
|
/merge |
This pull request has been accepted and is ready to merge. Commit hash: af356cc
|
TiDB MergeCI notify✅ Well Done! New fixed [1] after this pr merged.
|
* upstream/master: ddl/schematracker: fix SetDDL will cause data race (pingcap#36768) planner: check virtual column for tiflash (pingcap#36771) *: replace defer clean with t.Cleanup (pingcap#36722) ddl: fix inaccurate row_count for `admin show ddl jobs` (pingcap#36716) config, sysvar: add config `instance.enable_ddl` and sysvar `tidb_enable_ddl` (pingcap#35425) *: enable part revive for all code (pingcap#36703)
What problem does this PR solve?
Issue Number: ref #34960
Problem Summary:
Before, if we want to disable DDL in a node/instance, we have to change the configuration and restart the node.
What is changed and how it works?
In this PR we introduce an instance scope variable
enable_ddl
and the corresponding config iteminstance.enable_ddl
.Check List
Tests
Side effects
Documentation
Manual Test Steps
Test 1
4000
and4001
):tiup playground --db 2 --db.binpath ~/tidb/bin/tidb-server
curl http://127.0.0.1:10081/info/all
to check which instance is the owner (assume4000
)set @@global.tidb_enable_ddl = 0/1
to disable/enable ddl. When disabling ddl in the "owner", another instance should be the owner by checkingcurl http://127.0.0.1:10081/info/all
curl -X POST http://127.0.0.1:10080/ddl/owner/resign
to let the owner(4000
) start a new campaign. the instance disabled ddl should not be an owner, and the instance enabled ddl could be an owner.Test 2
tidb-a
(DDL owner) andtidb-b
.tidb-a
:set @@global.tidb_enable_ddl = false;
tidb-b
, or just send a resign signal totidb-b
tidb-a
:set @@global.tidb_enable_ddl = true;
tidb-a
:create table t (a int);
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.