Skip to content

*: add read-first DDL framework notes#66094

Open
wjhuang2016 wants to merge 4 commits intopingcap:masterfrom
wjhuang2016:docs/ddl-read-first
Open

*: add read-first DDL framework notes#66094
wjhuang2016 wants to merge 4 commits intopingcap:masterfrom
wjhuang2016:docs/ddl-read-first

Conversation

@wjhuang2016
Copy link
Member

@wjhuang2016 wjhuang2016 commented Feb 5, 2026

What problem does this PR solve?

Issue Number: close #66093

Problem Summary:

  • DDL framework knowledge is scattered across code and design docs, and it’s easy to implement DDL behavior ad-hoc in pkg/executor/ without understanding the job-based, owner-driven execution pipeline.
  • That increases correctness risk and review overhead, because changes can bypass intended invariants (schema state machine, schema sync, resumability, reorg/backfill).

What changed and how does it work?

  • Added a “read-first” DDL framework note set under docs/note/ddl/ with an index + reading order, covering execution flow, job lifecycle, reorg/backfill pointers, a dev checklist, and a file map.
  • Strengthened agent guidance in AGENTS.md with DDL module-only rules:
    • read the DDL notes before DDL work,
    • treat notes as hypotheses in debugging (validate via code/tests),
    • update notes immediately when doc/code diverge.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • 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

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

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

None

@ti-chi-bot ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-triage-completed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 5, 2026
@pantheon-ai
Copy link

pantheon-ai bot commented Feb 5, 2026

Request Failed

The system couldn’t send or save your message because it’s missing the information that identifies which user it belongs to. Please try again after signing in or making sure your account is selected.

Please try again or contact support if the issue persists.

Open in Web
Learn more about Pantheon AI

@ti-chi-bot
Copy link

ti-chi-bot bot commented Feb 5, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign zhaoxinyu for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

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

Details Needs approval from an approver in each of these files:

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

Copy link
Contributor

@lance6716 lance6716 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very helpful! (will help to comment soon)


- End the previous transaction and start a new internal DDL transaction context.
- Perform statement-level branching (AST switch) and call `ddl.Executor` methods.
- Convert “schema outdated” errors into `ErrInfoSchemaChanged` so the caller can retry safely (`(*DDLExec).toErr`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about removing this line? Error path is less important than other main logic

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't verify the whole context for now; it still needs further verification

Entry: `pkg/executor/ddl.go` (`type DDLExec`, `(*DDLExec).Next`)

Responsibilities:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if my impression helps 😂

Acts as the SQL abstraction layer that wraps internal distributed DDL module for end-user interaction. Runs on the TiDB node that user connected.

- Persisting schema changes directly (should go through DDL jobs).
- Any logic that must survive owner transfer / restart (belongs to job execution).

## 2) DDL executor (DDL module): statement → job(s) → wait
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entry point for distributed DDL tasks. Acts as a framework client responsible for verification, serialization, and blocking wait. Runs on the TiDB node that user connected.

@ti-chi-bot ti-chi-bot bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs/ddl: Add read-first DDL execution framework notes

2 participants