pingcap / pingcap/tidb

ddl code refactor/optimize

Open
#54,436 0 comments 1 reaction 0 assignees View on GitHub
type/enhancement
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Enhancement
We have listed a lot of items that we want to refactor, to make the structure more clear and code easier to maintain and extend, and some optimize tasks to make general DDL run faster. And in the last 8.2 sprint, we optimized for general DDL, and done some refactor for job scheduling and have left some TODOs during the refactor, https://github.com/pingcap/tidb/issues/53246.
In 8.3 sprint, we have the same goal as in 8.2 and we also want to enhance testibility of DDL component, by decoupling different part of DDL(TODO), and replacing ddl.Hook with simplier failpoint.

### Decouple components of DDL
Currently, ddl/ddlCtx is quite large and has a lot of responsibility, including:
- handle job submission which come from sql executor, this is the biggest part
- This part shares the same interface as schema-tracker, maybe extract this interface as something called DDLExecutor.
- Handle local ddl job execution for fast creating tables. It runs on every node
- owner election, start job scheduler
- Enable/disable some functions, such as whether this node can attend owner election, and fast-create
- It contains a lot of methods related to job execution, such as writePhysicalTableRecord
- Manages fields which are shared with other components, such as below. They only use part of their function, and there are no intersections. Maybe we can create those fields on each side or separate the function.
- SchemaSyncer, or should be called schema-version-syncer, shared with domain
- StateSyncer, shared with SyncUpgrade api handling.
- Manages fields that should be part of job scheduler: reorg-ctx/job-ctx/ddlSeqNum/waitSchemaSyncedController
- Some fields are used for unit tests: hook/interceptor inside ddlCtx. We can replace them with failpoints.
- Some util methods which should be moved out: GetTableMaxHandle

### Tasks
- [ ] Optimize job execution
- [x] remove this markJobProcessing completely, we have move job dependency calculation in memory, and as long as the job resides in the table, we should try run it, so this fields is useless now. see https://github.com/pingcap/tidb/pull/54438
- [x] merge loadDDLJobAndRun by job type into one(we have do it in this #54419), and remove the sub-query in getJobSQL, and handle this case https://github.com/pingcap/tidb/pull/54110#discussion_r1647002871 . And schedule as many jobs as possible in one round. see https://github.com/pingcap/tidb/pull/54438
- [x] https://github.com/pingcap/tidb/pull/54581
- [x] Make job id allocation and insert job run in one transaction, and test QPS of it when run in parallel. If it outperforms job execution, we can re-add the reverted optimization that query from the min job id.
- [x] https://github.com/pingcap/tidb/pull/54547
- [x] https://github.com/pingcap/tidb/pull/54669
- [x] https://github.com/pingcap/tidb/pull/54693
- [x] https://github.com/pingcap/tidb/pull/55025
- [x] https://github.com/pingcap/tidb/pull/55116
- [x] [**later**]Use RPC to notify job done when DDL job is submitted on non-owner node.
- [ ] optimize global schema version allocation
- [ ] Decouple components of DDL
- [ ] Move waitSchemaSyncedController / reorgCtx / jobCtx / ddlSeqNum / schemaVersionManager to job scheduler from ddlCtx
- [x] https://github.com/pingcap/tidb/pull/54774
- [x] https://github.com/pingcap/tidb/pull/55411
- [x] https://github.com/pingcap/tidb/pull/55461
- [x] https://github.com/pingcap/tidb/pull/56376
- [x] https://github.com/pingcap/tidb/pull/56399
- [x] https://github.com/pingcap/tidb/pull/56542
- [x] Separate DDLExecutor interface out of DDL interface
- [x] https://github.com/pingcap/tidb/pull/54858
- [x] https://github.com/pingcap/tidb/pull/55140
- [x] ~separate local job execution out~: we have integrate fast-create into general DDL, see https://github.com/pingcap/tidb/pull/55025
- [x] Moving methods related to job execution out https://github.com/pingcap/tidb/pull/54858
- [x] ~Avoid exposing SchemaSyncer and StateSyncer from DDL~
- [x] Replace fields only used for test with failpoint: such as hook/interceptor inside ddlCtx
- [x] https://github.com/pingcap/tidb/pull/54882
- [x] https://github.com/pingcap/tidb/pull/55091
- [x] https://github.com/pingcap/tidb/pull/55120
- [x] https://github.com/pingcap/tidb/pull/55215
- [x] Move GetTableMaxHandle out https://github.com/pingcap/tidb/pull/55411
- [x] https://github.com/pingcap/tidb/pull/55312
- [x] https://github.com/pingcap/tidb/pull/55368
- [x] https://github.com/pingcap/tidb/pull/55552
- [ ] Refactor for reorg jobs (TODO, only list a few)
- [ ] make all global variables like ingest.LitBackCtxMgr local to job scheduler
- [ ] ReorgCtx is stored at ddlCtx, and we are waiting for the reorg routine repeatedly by entering/exit the ddl worker. If after the check due to waitTimeout, owner changes, we might have no chance to clean it up. We better wait for the routine inside runReorgJob
- [x] Remove dependency on lightning config, we should use local.BackendConfig directly. https://github.com/pingcap/tidb/pull/55433
- [x] https://github.com/pingcap/tidb/pull/54149
- [x] https://github.com/pingcap/tidb/pull/54292
- [x] https://github.com/pingcap/tidb/pull/54598
- [x] https://github.com/pingcap/tidb/pull/54747
- [x] https://github.com/pingcap/tidb/pull/55302
- [x] https://github.com/pingcap/tidb/issues/53930
- [ ] pass CtxVars through job args
- [x] https://github.com/pingcap/tidb/pull/56586
- [x] https://github.com/pingcap/tidb/pull/56632
- [x] https://github.com/pingcap/tidb/pull/56656
- [ ] https://github.com/pingcap/tidb/issues/56398
- [ ] others
- [x] https://github.com/pingcap/tidb/pull/54685
- [x] https://github.com/pingcap/tidb/pull/54967
- [x] https://github.com/pingcap/tidb/pull/55692
- [x] https://github.com/pingcap/tidb/pull/55841

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the remaining unchecked tasks and the ddl/ddlCtx responsibilities described in the issue, especially global schema version allocation, reorg jobs, and issue #56398. Trace the relevant DDL scheduler code and existing tests before selecting one narrowly scoped task. Done means the selected refactor or optimization is implemented, its behavior is covered by tests, and the checklist item can be marked complete.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, databases, distributed-systems
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
15/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.