Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ticdc: Support create table ddl only appear in tidb_ddl_history instead of tidb_ddl_job table #10907

Merged
merged 18 commits into from
May 7, 2024
Merged

ticdc: Support create table ddl only appear in tidb_ddl_history instead of tidb_ddl_job table #10907

merged 18 commits into from
May 7, 2024

Conversation

hongyunyan
Copy link
Collaborator

@hongyunyan hongyunyan commented Apr 16, 2024 •
edited
Loading

What problem does this PR solve?

Issue Number: close #10908

What is changed and how it works?

  1. Support to listen to tidb_ddl_history table in ddlPuller
  2. filter other ddls in tidb_ddl_history, and only choose create table ddl as the real ddl job
  3. remove useless implementations, like listen to DDLJobList etc.

Check List

Tests

  • Manual test (add detailed scripts or steps below)
  1. test mannually with set global tidb_enable_fast_create_table =on and set global tidb_enable_fast_create_table =off. To make sure TiCDC can work well in both old ddl version and new ddl versions.
  2. I compared the time it took for TiCDC to process 100 create table and add columns before and after the modification. Both took 5 mins. Thus, the modification will not affect TiCDC performance.

Questions

Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?

Release note 

None

@ti-chi-bot ti-chi-bot bot added do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. do-not-merge/needs-triage-completed and removed do-not-merge/needs-linked-issue labels Apr 16, 2024
@codecov Codecov
Copy link

codecov bot commented Apr 16, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 1.42857% with 69 lines in your changes are missing coverage. Please review.

Project coverage is 57.4968%. Comparing base (936b4d4) to head (7da4ec5).
Report is 3 commits behind head on master.

Additional details and impacted files
Components Coverage Δ
cdc 61.8129% <1.4285%> (+0.1429%) ⬆️
dm 51.2679% <ø> (+0.0525%) ⬆️
engine 63.4232% <ø> (+0.0423%) ⬆️
Flag Coverage Δ
unit 57.4968% <1.4285%> (+0.1072%) ⬆️

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

@@               Coverage Diff                @@
##             master     #10907        +/-   ##
================================================
+ Coverage   57.3895%   57.4968%   +0.1072%     
================================================
  Files           851        851                
  Lines        125467     125840       +373     
================================================
+ Hits          72005      72354       +349     
- Misses        48044      48060        +16     
- Partials       5418       5426         +8

@ti-chi-bot ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Apr 16, 2024
@hongyunyan
Copy link
Collaborator Author

/test verify

@hongyunyan
Copy link
Collaborator Author

/test cdc-integration-mysql-test

@hongyunyan
Copy link
Collaborator Author

/test cdc-integration-pulsar-test

@hongyunyan
Copy link
Collaborator Author

/test dm-integration-test

@hongyunyan
Copy link
Collaborator Author

/test dm-compatibility-test

@hongyunyan
Copy link
Collaborator Author

/test cdc-integration-kafka-test

@hongyunyan
Copy link
Collaborator Author

/test cdc-integration-pulsar-test

@hongyunyan
Copy link
Collaborator Author

/test verify

@CharlesCheung96
Copy link
Contributor

Please add some integration tests to cover the tidb_enable_fast_create_table=on case.

hicqu
hicqu reviewed Apr 17, 2024
View reviewed changes
pkg/version/check.go Outdated
@@ -37,13 +37,13 @@ import (

var (
// minPDVersion is the version of the minimal compatible PD.
minPDVersion = semver.New("5.1.0-alpha")
minPDVersion = semver.New("7.1.0-alpha")
Copy link
Contributor

Choose a reason for hiding this comment

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

Please comment why 7.1 is required.

@hicqu
Copy link
Contributor

hicqu commented Apr 17, 2024

Rest LGTM.

@ti-chi-bot ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 22, 2024
@ti-chi-bot ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 22, 2024
@hongyunyan
Copy link
Collaborator Author

/retest

@hongyunyan
Copy link
Collaborator Author

/hold

@ti-chi-bot 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 Apr 24, 2024
@hongyunyan
Copy link
Collaborator Author

/test dm-integration-test

@ti-chi-bot ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented May 7, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CharlesCheung96, hicqu

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 [CharlesCheung96,hicqu]

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 ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels May 7, 2024
@ti-chi-bot ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented May 7, 2024

[LGTM Timeline notifier]

Timeline:

  • 2024-04-17 06:59:31.246928433 +0000 UTC m=+407790.357974881: ☑️ agreed by hicqu.
  • 2024-05-07 03:52:28.188678489 +0000 UTC m=+934101.945814060: ☑️ agreed by CharlesCheung96.

@hongyunyan
Copy link
Collaborator Author

/unhold

@ti-chi-bot 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 May 7, 2024
@hongyunyan
Copy link
Collaborator Author

/retest

@hongyunyan
Copy link
Collaborator Author

/test verify

@hongyunyan
Copy link
Collaborator Author

/test cdc-integration-mysql-test

@ti-chi-bot ti-chi-bot bot merged commit 8d6a9e1 into pingcap:master May 7, 2024
28 checks passed
3AceShowHand added a commit to 3AceShowHand/tiflow that referenced this pull request Aug 23, 2024
@3AceShowHand
Revert "ticdc: Support create table ddl only appear in tidb_ddl_his…
fef5255 
…tory instead of tidb_ddl_job table (pingcap#10907)"

This reverts commit 8d6a9e1.
@wlwilliamx wlwilliamx mentioned this pull request Dec 3, 2024
Closed
wlwilliamx added a commit to wlwilliamx/tiflow that referenced this pull request Dec 5, 2024
@wlwilliamx
Revert "ticdc: Support create table ddl only appear in tidb_ddl_his…
55bca14 
…tory instead of tidb_ddl_job table (pingcap#10907)"

This reverts commit 8d6a9e1.
ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Dec 20, 2024
@wlwilliamx @ti-chi-bot
Revert "ticdc: Support create table ddl only appear in tidb_ddl_his…
4057d10 
…tory instead of tidb_ddl_job table (pingcap#10907)"

This reverts commit 8d6a9e1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hicqu hicqu hicqu approved these changes

@CharlesCheung96 CharlesCheung96 CharlesCheung96 approved these changes

@lidezhu lidezhu Awaiting requested review from lidezhu

Assignees
No one assigned
Labels
approved lgtm 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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TiCDC support create table accelerate in TiDB
4 participants
@hongyunyan @CharlesCheung96 @hicqu @lidezhu