config, sysvar: add config `instance.enable_ddl` and sysvar `tidb_enable_ddl` #35425

CbcWestwolf · 2022-06-16T03:38:31Z

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 item instance.enable_ddl.

Check List

Tests

Unit test (using previous testing)
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Manual Test Steps

Test 1

start a tiup playground with two TiDB instances (port 4000 and 4001): tiup playground --db 2 --db.binpath ~/tidb/bin/tidb-server
start two sessions connecting to two instances separately
using curl http://127.0.0.1:10081/info/all to check which instance is the owner (assume 4000)
in the session, using set @@global.tidb_enable_ddl = 0/1 to disable/enable ddl. When disabling ddl in the "owner", another instance should be the owner by checking curl http://127.0.0.1:10081/info/all
using 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

Start two TiDB servers, say tidb-a(DDL owner) and tidb-b.
Execute on tidb-a: set @@global.tidb_enable_ddl = false;
Stop tidb-b, or just send a resign signal to tidb-b
Execute on tidb-a: set @@global.tidb_enable_ddl = true;
Execute on tidb-a: create table t (a int);

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

Previously, TiDB users use the config item `run-ddl` to control whether the TiDB cluster could run DDL.

From now on, users can use the config item `instance.tidb_enable_ddl` to control whether the TiDB instance could run DDL, and the instance system variable `tidb_enable_ddl` to change online.

ti-chi-bot · 2022-06-16T03:38:33Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

qw4990
tangenta

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

CbcWestwolf · 2022-06-16T05:50:09Z

/run-unit-test

sre-bot · 2022-06-16T05:55:37Z

Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/af356ccca1af73d3d44eba855ca0cfd193906fec

CbcWestwolf · 2022-06-16T06:03:38Z

/cc morgo bb7133 zimulala xiongjiwei

xiongjiwei · 2022-06-16T06:32:16Z

ddl/ddl.go

 	// Otherwise, we needn't do that.
-	if RunWorker {
+	if config.GetGlobalConfig().Instance.EnableDDL.Load() {


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

morgo · 2022-06-16T13:33:11Z

sessionctx/variable/sysvar.go

+	// EnableDDL indicates whether the tidb-server runs DDL statements,
+	EnableDDL = "enable_ddl"


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.

xhebox · 2022-06-28T04:09:36Z

ddl/ddl.go


 	d.wg.Run(d.limitDDLJobs)
 	d.sessPool = newSessionPool(ctxPool, d.store)
+	d.workers = make(map[workerType]*worker, 2)


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().

CbcWestwolf · 2022-08-01T05:43:45Z

/merge

ti-chi-bot · 2022-08-01T05:43:48Z

@CbcWestwolf: /merge is only allowed for the committers, you can assign this pull request to the committer in list by filling /assign @committer in the comment to help merge this pull request.

In response to this:

/merge

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.

tangenta · 2022-08-01T05:59:30Z

/merge

ti-chi-bot · 2022-08-01T05:59:33Z

This pull request has been accepted and is ready to merge.

Commit hash: f47dd96

xiongjiwei · 2022-08-01T06:46:51Z

/merge

ti-chi-bot · 2022-08-01T06:46:55Z

This pull request has been accepted and is ready to merge.

Commit hash: af356cc

sre-bot · 2022-08-01T07:33:48Z

TiDB MergeCI notify

✅ Well Done! New fixed [1] after this pr merged.

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci-tidb/integration-ddl-test	✅ all 6 tests passed	6 min 18 sec	Fixed
idc-jenkins-ci/integration-cdc-test	🟢 all 36 tests passed	25 min	Existing passed
idc-jenkins-ci-tidb/common-test	🟢 all 11 tests passed	21 min	Existing passed
idc-jenkins-ci-tidb/integration-common-test	🟢 all 17 tests passed	18 min	Existing passed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	7 min 2 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	6 min 18 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2	🟢 all 28 tests passed	5 min 33 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	3 min 34 sec	Existing passed
idc-jenkins-ci-tidb/mybatis-test	🟢 all 1 tests passed	3 min 24 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

…ble_ddl` (pingcap#35425) ref pingcap#34960

* 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)

config, sysvar: map config run-ddl to sysvar enable_ddl

8946074

CbcWestwolf requested a review from a team as a code owner June 16, 2022 03:38

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 16, 2022

ti-chi-bot requested review from bb7133, morgo, xiongjiwei and zimulala June 16, 2022 06:03

xiongjiwei reviewed Jun 16, 2022

View reviewed changes

morgo reviewed Jun 16, 2022

View reviewed changes

CbcWestwolf added 2 commits June 16, 2022 23:06

Rename: add prefix TiDBxxx and tidb_xxx.

cd2a367

Merge branch 'master' of github.com:pingcap/tidb into enable_ddl

1093e31

morgo approved these changes Jun 18, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 18, 2022

Support pause a campaign in owner

d2ed88e

ti-chi-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 23, 2022

Merge branch 'master' of github.com:pingcap/tidb into enable_ddl

1936c53

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2022

Merge branch 'master' of github.com:pingcap/tidb into enable_ddl

90038dc

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 25, 2022

xhebox reviewed Jun 28, 2022

View reviewed changes

Merge branch 'master' of github.com:pingcap/tidb into enable_ddl

ebc7c77

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 1, 2022

make *ddl having a real Start and Stop

54d85fb

tangenta approved these changes Aug 1, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 1, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 1, 2022

ti-chi-bot and others added 3 commits August 1, 2022 13:59

Merge branch 'master' into enable_ddl

c22fa7a

Merge branch 'master' into enable_ddl

5f2c12e

Fix

af356cc

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Aug 1, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 1, 2022

ti-chi-bot merged commit fc217d4 into pingcap:master Aug 1, 2022

CbcWestwolf deleted the enable_ddl branch August 1, 2022 07:06

morgo pushed a commit to morgo/tidb that referenced this pull request Aug 1, 2022

config, sysvar: add config instance.enable_ddl and sysvar `tidb_ena…

607cde3

…ble_ddl` (pingcap#35425) ref pingcap#34960

morgo mentioned this pull request Aug 2, 2022

Add test to ensure instance scoped variables always have matching sysvars #36795

Closed

morgo mentioned this pull request Aug 17, 2022

run-ddl should be marked as removed #37185

Closed

CbcWestwolf mentioned this pull request Aug 18, 2022

config, sysvar: update descriptions about instance sysvars pingcap/docs-cn#10978

Merged

12 tasks

tangenta mentioned this pull request Mar 22, 2023

system-variables.md: add version tag for tidb_enable_ddl pingcap/docs-cn#13509

Merged

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config, sysvar: add config `instance.enable_ddl` and sysvar `tidb_enable_ddl` #35425

config, sysvar: add config `instance.enable_ddl` and sysvar `tidb_enable_ddl` #35425

CbcWestwolf commented Jun 16, 2022 •

edited

Loading

ti-chi-bot commented Jun 16, 2022 •

edited

Loading

CbcWestwolf commented Jun 16, 2022

sre-bot commented Jun 16, 2022 •

edited

Loading

CbcWestwolf commented Jun 16, 2022

xiongjiwei Jun 16, 2022

morgo Jun 16, 2022

xhebox Jun 28, 2022 •

edited

Loading

CbcWestwolf commented Aug 1, 2022

ti-chi-bot commented Aug 1, 2022

tangenta commented Aug 1, 2022

ti-chi-bot commented Aug 1, 2022

xiongjiwei commented Aug 1, 2022

ti-chi-bot commented Aug 1, 2022

sre-bot commented Aug 1, 2022

		// EnableDDL indicates whether the tidb-server runs DDL statements,
		EnableDDL = "enable_ddl"

config, sysvar: add config instance.enable_ddl and sysvar tidb_enable_ddl #35425

config, sysvar: add config instance.enable_ddl and sysvar tidb_enable_ddl #35425

Conversation

CbcWestwolf commented Jun 16, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Manual Test Steps

Test 1

Test 2

Release note

ti-chi-bot commented Jun 16, 2022 • edited Loading

CbcWestwolf commented Jun 16, 2022

sre-bot commented Jun 16, 2022 • edited Loading

CbcWestwolf commented Jun 16, 2022

xiongjiwei Jun 16, 2022

Choose a reason for hiding this comment

morgo Jun 16, 2022

Choose a reason for hiding this comment

xhebox Jun 28, 2022 • edited Loading

Choose a reason for hiding this comment

CbcWestwolf commented Aug 1, 2022

ti-chi-bot commented Aug 1, 2022

tangenta commented Aug 1, 2022

ti-chi-bot commented Aug 1, 2022

xiongjiwei commented Aug 1, 2022

ti-chi-bot commented Aug 1, 2022

sre-bot commented Aug 1, 2022

TiDB MergeCI notify

config, sysvar: add config `instance.enable_ddl` and sysvar `tidb_enable_ddl` #35425

config, sysvar: add config `instance.enable_ddl` and sysvar `tidb_enable_ddl` #35425

CbcWestwolf commented Jun 16, 2022 •

edited

Loading

ti-chi-bot commented Jun 16, 2022 •

edited

Loading

sre-bot commented Jun 16, 2022 •

edited

Loading

xhebox Jun 28, 2022 •

edited

Loading