ddl: run add-index subtasks at system keyspace tidb #62045

tangenta · 2025-06-27T06:53:58Z

What problem does this PR solve?

Issue Number: ref #61702

Problem Summary:

This PR supports adding index at system keyspace TiDB.

What changed and how does it work?

Check List

Tests

Unit test
Integration test

Manual test (add detailed scripts or steps below)

-- system keyspace tidb
mysql> create table t (a int);
Query OK, 0 rows affected (0.14 sec)

mysql> insert into t values (1);
Query OK, 1 row affected (0.04 sec)

mysql> alter table t add index idx(a);
Query OK, 0 rows affected (6.41 sec)

mysql> admin check table t;
Query OK, 0 rows affected (0.02 sec)

-- user keyspace tidb
mysql> create table t (a int);
Query OK, 0 rows affected (0.12 sec)

mysql> insert into t values (1);
Query OK, 1 row affected (0.04 sec)

mysql> alter table t add index idx(a);
Query OK, 0 rows affected (2.44 sec)

mysql> admin check table t;
Query OK, 0 rows affected (0.00 sec)

mysql> select id,state,step,task_key from mysql.tidb_global_task union select id,state,step,task_key from mysql.tidb_global_task_history;
+-------+---------+------+-------------------------+
| id    | state   | step | task_key                |
+-------+---------+------+-------------------------+
|     1 | succeed |   -2 | SYSTEM/ddl/backfill/116 |
| 30001 | succeed |   -2 | ks1/ddl/backfill/116    |
+-------+---------+------+-------------------------+
2 rows in set (0.00 sec)

No need to test
- I checked and no code files have been changed.

Side effects

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

Documentation

Release note

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

None

Signed-off-by: tangenta <[email protected]>

tiprow · 2025-06-27T06:54:52Z

Hi @tangenta. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

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 kubernetes-sigs/prow repository.

codecov · 2025-06-27T07:12:06Z

Codecov Report

Attention: Patch coverage is 50.00000% with 25 lines in your changes missing coverage. Please review.

Project coverage is 74.8263%. Comparing base (1962f22) to head (e986f89).
Report is 22 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #62045        +/-   ##
================================================
+ Coverage   72.9246%   74.8263%   +1.9016%     
================================================
  Files          1739       1786        +47     
  Lines        482820     494656     +11836     
================================================
+ Hits         352095     370133     +18038     
+ Misses       109169     101418      -7751     
- Partials      21556      23105      +1549

Flag	Coverage Δ
integration	`48.8415% <2.0000%> (?)`
unit	`72.1587% <50.0000%> (+0.0020%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`52.7804% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`62.1822% <ø> (+15.8463%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

pkg/keyspace/keyspace.go

D3Hunter · 2025-06-27T07:41:58Z

pkg/domain/domain.go

+	distTaskSessPool := do.sysSessionPool
+	if keyspace.IsRunningOnUser() {
+		sp, err := do.GetKSSessPool(keyspace.System)
+		if err != nil {
+			return err
+		}
+		distTaskSessPool = sp
+	}
+	taskManager := storage.NewTaskManager(distTaskSessPool)


this cannot be merged now, current task still runs on its owner keyspace, we can merge it when we are ready to switch to DXF service

What else do we need to do? I can run add-index job normally after this PR.

seems ok for add-index

if we enable it, user ks DXF will also try get task/subtasks from SYSTEM ks

we shouldn't change this for all places, and only change where add-index uses TaskManager

For a classical kernel, it should use advancenSessionPool

pkg/ddl/session/session_pool.go

Signed-off-by: tangenta <[email protected]>

pkg/ddl/backfilling_dist_executor.go

D3Hunter

please add a real-tikv test like this one to make sure we do submit and run on DXF service on SYSTEM KS https://github.com/pingcap/tidb/pull/62023/files#diff-4ca8746e7f966fd0f1bc3257edfb8208f38f354c3a33d91c3591eb2d1eaa7376R30

D3Hunter · 2025-06-27T09:21:19Z

failed to add index on SYSTEM keyspace

mysql> show config where name='keyspace-name';
+------+----------------------+---------------+--------+
| Type | Instance             | Name          | Value  |
+------+----------------------+---------------+--------+
| tidb | 192.168.206.185:5000 | keyspace-name | SYSTEM |
+------+----------------------+---------------+--------+
1 row in set (0.03 sec)

mysql>
mysql> create table sys(id int);
Query OK, 0 rows affected (0.10 sec)

mysql> insert into sys values(1);
Query OK, 1 row affected (0.02 sec)

mysql> alter table sys add index(id);
ERROR 1105 (HY000): cross keyspace session manager is not available in classic kernel or current keyspace
mysql>

Signed-off-by: tangenta <[email protected]>

D3Hunter · 2025-06-30T02:40:54Z

pkg/ddl/backfilling_dist_executor.go

+	store := ddlObj.store
+	sessPool := ddlObj.sessPool
+	taskKS := s.task.Keyspace
+	if keyspace.IsRunningOnSystem() && taskKS != config.GetGlobalKeyspaceName() {


Suggested change

if keyspace.IsRunningOnSystem() && taskKS != config.GetGlobalKeyspaceName() {

if taskKS != config.GetGlobalKeyspaceName() {

taskKS != config.GetGlobalKeyspaceName() implies it's running on SYSTEM

I think it is better to make it explicit to avoid unexpected executions. (Make kerneltype.NextGen() and SYSTEM explicit)

maybe add a comment about why we still state it explicitly even though taskKS != config.GetGlobalKeyspaceName() implies it's running on SYSTEM

If you want to avoid unexpected executions, it's better to use intest.Assert to let it panic in test environment

D3Hunter · 2025-06-30T02:44:08Z

please add your manually test results when you

add index on SYSTEM ks
add index on user ks

D3Hunter · 2025-06-30T02:50:18Z

pkg/ddl/session/session_pool.go

 }

 // NewSessionPool creates a new Session pool.
-func NewSessionPool(resPool *pools.ResourcePool) *Pool {
+func NewSessionPool(resPool util.SessionPool) *Pool {


in nextgen, as we are not registering this cross ks session into the SessionManager, so it will not block GC during scanning kv?

Yes, it is a risk. Not sure if the GC worker can execute across keyspaces.

they won't AFAIK

tangenta · 2025-06-30T08:49:21Z

/retest

tiprow · 2025-06-30T08:49:48Z

@tangenta: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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 kubernetes-sigs/prow repository.

D3Hunter · 2025-06-30T10:21:45Z

please add your manually test results when you

add index on SYSTEM ks

add index on user ks

in your manually test, please also give the result of select id,state,step,task_key from mysql.tidb_global_task union select id,state,step,task_key from mysql.tidb_global_task_history;, so we known we submit the task to the right keyspace

…x-system

Signed-off-by: tangenta <[email protected]>

D3Hunter

rest lgtm

pkg/ddl/backfilling_dist_executor.go

Signed-off-by: tangenta <[email protected]>

D3Hunter · 2025-07-01T03:48:06Z

/hold

i am running on SYSTEM, but got error on ks1

mysql> show config where name='keyspace-name';
+------+------------------+---------------+--------+
| Type | Instance         | Name          | Value  |
+------+------------------+---------------+--------+
| tidb | 192.168.2.4:5000 | keyspace-name | SYSTEM |
+------+------------------+---------------+--------+
1 row in set (0.03 sec)

mysql> create table t(a int);
Query OK, 0 rows affected (0.17 sec)

mysql> insert into t values(1);
Query OK, 1 row affected (0.01 sec)

mysql> alter table t add index(a);
ERROR 8028 (HY000): Information schema is changed during the execution of the statement(for example, table definition may be updated by other DDL ran in parallel). If you see this error often, try increasing `tidb_max_delta_schema_count`. [try again later]

[2025/07/01 11:41:54.454 +08:00] [INFO] [manager.go:328] ["task executor started"] [keyspaceName=ks1] [task-id=1] [type=backfill] [concurrency=4] [remaining-slots=6]
[2025/07/01 11:41:54.778 +08:00] [INFO] [task_executor.go:359] ["failed to get step executor"] [keyspaceName=ks1] [task-id=1] [task-type=backfill] [error="[schema:1146]Table '(Schema ID 2).(Table ID 114)' doesn't exist"] [errorVerbose="[schema:1146]Table '(Schema ID 2).(Table ID 114)' doesn't exist\ngithub.com/pingcap/errors.(*Error).GenWithStackByArgs\n\t/Users/jujiajia/go/pkg/mod/github.com/pingcap/[email protected]/normalize.go:177\ngithub.com/pingcap/tidb/pkg/ddl.getTableInfo\n\t/Users/jujiajia/code/pingcap/tidb/pkg/ddl/table.go:433\ngithub.com/pingcap/tidb/pkg/ddl.getTableByTxn.func1\n\t/Users/jujiajia/code/pingcap/tidb/pkg/ddl/job_scheduler.go:668\ngithub.com/pingcap/tidb/pkg/kv.RunInNewTxn\n\t/Users/jujiajia/code/pingcap/tidb/pkg/kv/txn.go:132\ngithub.com/pingcap/tidb/pkg/ddl.getTableByTxn\n\t/Users/jujiajia/code/pingcap/tidb/pkg/ddl/job_scheduler.go:661\ngithub.com/pingcap/tidb/pkg/ddl.(*backfillDistExecutor).newBackfillStepExecutor\n\t/Users/jujiajia/code/pingcap/tidb/pkg/ddl/backfilling_dist_executor.go:179\ngithub.com/pingcap/tidb/pkg/ddl.(*backfillDistExecutor).GetStepExecutor\n\t/Users/jujiajia/code/pingcap/tidb/pkg/ddl/backfilling_dist_executor.go:258\ngithub.com/pingcap/tidb/pkg/disttask/framework/taskexecutor.(*BaseTaskExecutor).createStepExecutor\n\t/Users/jujiajia/code/pingcap/tidb/pkg/disttask/framework/taskexecutor/task_executor.go:357\ngithub.com/pingcap/tidb/pkg/disttask/framework/taskexecutor.(*BaseTaskExecutor).Run\n\t/Users/jujiajia/code/pingcap/tidb/pkg/disttask/framework/taskexecutor/task_executor.go:328\ngithub.com/pingcap/tidb/pkg/disttask/framework/taskexecutor.(*Manager).startTaskExecutor.func2\n\t/Users/jujiajia/code/pingcap/tidb/pkg/disttask/framework/taskexecutor/manager.go:339\ngithub.com/pingcap/tidb/pkg/util.(*WaitGroupWrapper).RunWithLog.func1\n\t/Users/jujiajia/code/pingcap/tidb/pkg/util/wait_group_wrapper.go:181\nruntime.goexit\n\t/Users/jujiajia/go/pkg/mod/golang.org/[email protected]/src/runtime/asm_arm64.s:1223"]

Signed-off-by: tangenta <[email protected]>

tangenta · 2025-07-02T05:47:46Z

/retest

tiprow · 2025-07-02T05:48:08Z

@tangenta: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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 kubernetes-sigs/prow repository.

pkg/disttask/framework/handle/handle.go

…into next-gen-add-index-system

Signed-off-by: tangenta <[email protected]>

D3Hunter · 2025-07-02T08:01:34Z

/unhold

Signed-off-by: tangenta <[email protected]>

D3Hunter · 2025-07-02T09:01:19Z

/retest

tiprow · 2025-07-02T09:01:42Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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 kubernetes-sigs/prow repository.

D3Hunter · 2025-07-02T09:16:14Z

/retest

tiprow · 2025-07-02T09:16:37Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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 kubernetes-sigs/prow repository.

D3Hunter · 2025-07-02T09:49:30Z

/retest

tiprow · 2025-07-02T09:49:53Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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 kubernetes-sigs/prow repository.

ti-chi-bot · 2025-07-02T10:05:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: D3Hunter, wjhuang2016

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [D3Hunter,wjhuang2016]
~~pkg/ddl/OWNERS~~ [D3Hunter,wjhuang2016]
~~pkg/disttask/OWNERS~~ [D3Hunter,wjhuang2016]
~~pkg/domain/OWNERS~~ [D3Hunter,wjhuang2016]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2025-07-02T10:05:30Z

[LGTM Timeline notifier]

Timeline:

2025-07-01 03:12:16.901340431 +0000 UTC m=+1364589.624519414: ☑️ agreed by D3Hunter.
2025-07-02 10:05:29.149956161 +0000 UTC m=+1475781.873135143: ☑️ agreed by wjhuang2016.

ddl: run add-index subtasks at system keyspace tidb

5df877a

Signed-off-by: tangenta <[email protected]>

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 27, 2025

D3Hunter reviewed Jun 27, 2025

View reviewed changes

address comment

999588f

Signed-off-by: tangenta <[email protected]>

D3Hunter reviewed Jun 27, 2025

View reviewed changes

pkg/ddl/backfilling_dist_executor.go Outdated Show resolved Hide resolved

D3Hunter reviewed Jun 27, 2025

View reviewed changes

address comment

1a3c091

Signed-off-by: tangenta <[email protected]>

D3Hunter reviewed Jun 30, 2025

View reviewed changes

tangenta added 3 commits July 1, 2025 01:25

Merge remote-tracking branch 'upstream/master' into next-gen-add-inde…

e5b342f

…x-system

add comment

aea0822

Signed-off-by: tangenta <[email protected]>

Merge remote-tracking branch 'upstream/master' into HEAD

351e563

D3Hunter approved these changes Jul 1, 2025

View reviewed changes

pkg/ddl/backfilling_dist_executor.go Outdated Show resolved Hide resolved

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jul 1, 2025

fix typo

72a4001

Signed-off-by: tangenta <[email protected]>

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 1, 2025

address comment

c96b809

Signed-off-by: tangenta <[email protected]>

D3Hunter reviewed Jul 2, 2025

View reviewed changes

pkg/disttask/framework/handle/handle.go Outdated Show resolved Hide resolved

tangenta added 2 commits July 2, 2025 15:52

Merge branch 'next-gen-add-index-system' of github.com:tangenta/tidb …

64987ad

…into next-gen-add-index-system

revert GetTaskMgrToAccessDXFService changes

617b7e9

Signed-off-by: tangenta <[email protected]>

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 2, 2025

tangenta added 2 commits July 2, 2025 16:05

fix linter

ebc0ced

Signed-off-by: tangenta <[email protected]>

update bazel

e986f89

Signed-off-by: tangenta <[email protected]>

wjhuang2016 approved these changes Jul 2, 2025

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jul 2, 2025

ti-chi-bot bot merged commit c56481e into pingcap:master Jul 2, 2025
27 checks passed

D3Hunter mentioned this pull request Aug 12, 2025

DXF service on nextgen kernel #61702

Open

65 tasks

	if keyspace.IsRunningOnSystem() && taskKS != config.GetGlobalKeyspaceName() {
	if taskKS != config.GetGlobalKeyspaceName() {

ddl: run add-index subtasks at system keyspace tidb #62045

ddl: run add-index subtasks at system keyspace tidb #62045

Uh oh!

Conversation

tangenta commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

tiprow bot commented Jun 27, 2025

Uh oh!

codecov bot commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

D3Hunter left a comment

Choose a reason for hiding this comment

Uh oh!

D3Hunter commented Jun 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tangenta Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

D3Hunter commented Jun 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tangenta commented Jun 30, 2025

Uh oh!

tiprow bot commented Jun 30, 2025

Uh oh!

D3Hunter commented Jun 30, 2025

Uh oh!

D3Hunter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

D3Hunter commented Jul 1, 2025

Uh oh!

tangenta commented Jul 2, 2025

Uh oh!

tiprow bot commented Jul 2, 2025

Uh oh!

Uh oh!

D3Hunter commented Jul 2, 2025

Uh oh!

D3Hunter commented Jul 2, 2025

Uh oh!

tiprow bot commented Jul 2, 2025

Uh oh!

D3Hunter commented Jul 2, 2025

Uh oh!

tangenta commented Jun 27, 2025 •

edited

Loading

codecov bot commented Jun 27, 2025 •

edited

Loading

tangenta Jun 30, 2025 •

edited

Loading