logservice,event: stop capturing tidb_ddl_history (#4766)#4905
logservice,event: stop capturing tidb_ddl_history (#4766)#4905ti-chi-bot wants to merge 1 commit intopingcap:release-8.5from
Conversation
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
|
This cherry pick PR is for a release branch and has not yet been approved by triage owners. To merge this cherry pick:
DetailsInstructions 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. |
|
@wlwilliamx This PR has conflicts, I have hold it. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@ti-chi-bot: ## If you want to know how to resolve it, please read the guide in TiDB Dev Guide. DetailsInstructions 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. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request removes the logic for capturing and processing DDL jobs from the tidb_ddl_history table, simplifying the schema store to only subscribe to the tidb_ddl_job table. This change reflects TiDB's normalized DDL lifecycle in version 8.3+. Feedback highlights critical issues that must be addressed before merging: unresolved git merge conflict markers were found in ddl_job_fetcher.go and span_op.go, and a missing import in span_op.go will lead to compilation errors. Additionally, it is recommended to explicitly check for JobStateDone in the parseJob function to strictly adhere to the intended filtering logic.
| <<<<<<< HEAD | ||
| const ( | ||
| // JobTableID is the id of `tidb_ddl_job`. | ||
| JobTableID = ddl.JobTableID | ||
| // JobHistoryID is the id of `tidb_ddl_history` | ||
| JobHistoryID = ddl.HistoryTableID | ||
| ) | ||
|
|
||
| ======= | ||
| >>>>>>> 895bb89bc (logservice,event: stop capturing tidb_ddl_history (#4766)) |
| <<<<<<< HEAD | ||
| JobTableID = ddl.JobTableID | ||
| // JobHistoryID is the id of `tidb_ddl_history` | ||
| JobHistoryID = ddl.HistoryTableID | ||
| ======= | ||
| JobTableID = metadef.TiDBDDLJobTableID | ||
| >>>>>>> 895bb89bc (logservice,event: stop capturing tidb_ddl_history (#4766)) |
There was a problem hiding this comment.
The code contains git merge conflict markers. Additionally, metadef is not imported in this file, so the second branch of the conflict would cause a compilation error. Please resolve this by keeping only the JobTableID = ddl.JobTableID definition and removing JobHistoryID.
JobTableID = ddl.JobTableID| } | ||
| // We only replay DDL jobs that are already visible in the normalized Done | ||
| // state so schema store ordering stays aligned with the job table stream. | ||
| if !job.IsDone() { |
There was a problem hiding this comment.
The PR description and the added test case ignores synced create table compatibility jobs indicate that only JobStateDone should be accepted. job.IsDone() is too broad as it typically includes JobStateSynced (and other terminal states like Cancelled). To strictly ignore JobStateSynced as intended, you should check for model.JobStateDone explicitly.
| if !job.IsDone() { | |
| if job.State != model.JobStateDone { |
This is an automated cherry-pick of #4766
What problem does this PR solve?
Issue Number: close #2272
What is changed and how it works?
This PR removes TiCDC's remaining upstream capture path for
tidb_ddl_history.tidb_ddl_historyin the schema-store DDL fetchertidb_ddl_historymetadata for DDL parsingtidb_ddl_jobJobStateDonejobs fromtidb_ddl_joband drop the oldJobStateSyncedcompatibility pathThis aligns TiCDC with the TiDB v8.3+ / v8.5 GA behavior targeted by issue #2272, where
CREATE TABLEno longer requirestidb_ddl_historycapture.Check List
Tests
Questions
Will it cause performance regression or break compatibility?
No performance regression is expected.
This intentionally drops compatibility with older TiDB behavior where
CREATE TABLEcould be emitted only throughtidb_ddl_history. That is the behavior cleanup requested by issue #2272.Do you need to update user documentation, design documentation or monitoring documentation?
No.
Release note
Summary by CodeRabbit