pkg/filter(ticdc): ignore ddl in ddl_manager #10518

CharlesCheung96 · 2024-01-22T03:20:48Z

What problem does this PR solve?

Issue Number: close #10524

What is changed and how it works?

After this pr, the behavior of TiCDC in ignoring create/delete/truncate/alter DDL scenarios is as follows:

Case 1: After a table or partition is created, TiCDC synchronizes the new physical table.

[filter]
rules = ['test.*']

[[filter.event-filters]]
matcher = ["test.t1"] # matcher is an allow list, which means this rule only applies to the worker table in the test database.
ignore-event = ["create table, add table partition"]

Case 2: After a table or partition is dropped, TiCDC does not synchronize the deleted physical table.

[filter]
rules = ['test.*']

[[filter.event-filters]]
matcher = ["test.t1"] # matcher is an allow list, which means this rule only applies to the worker table in the test database.
ignore-event = ["drop table, drop table partition"]

Case 3: After a table or partition is truncated, TiCDC does not synchronize the deleted physical table, and synchronizes the new physical table.

[filter]
rules = ['test.*']

[[filter.event-filters]]
matcher = ["test.t1"] # matcher is an allow list, which means this rule only applies to the worker table in the test database.
ignore-event = ["truncate table, truncate table partition"]

Case 4: After the Alter table or partition ddl, CDC uses the schma of the new table to parse data.

[filter]
rules = ['test.*']

[[filter.event-filters]]
matcher = ["test.t1"] # matcher is an allow list, which means this rule only applies to the worker table in the test database.
ignore-event = ["alter table, alter table partition"]

Check List

Tests

Unit test
Integration test

Questions

Will it cause performance regression or break compatibility?

Do you need to update user documentation, design documentation or monitoring documentation?

Release note

`Fix the issue of cdc does not synchronize related tables after ddl was filtered`.

ti-chi-bot · 2024-01-22T03:20:51Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

pkg/filter/filter.go

sre-bot · 2024-01-22T07:45:52Z

All committers have signed the CLA.

CharlesCheung96 · 2024-01-22T08:09:55Z

/test all

CharlesCheung96 · 2024-01-22T09:44:48Z

/test all

hongyunyan · 2024-01-22T14:25:45Z

cdc/owner/ddl_manager.go

-		m.justSentDDL = m.executingDDL
-		m.executingDDL = nil
-		m.cleanCache()
+	skip, cleanMsg, err := m.shouldSkipDDL(m.executingDDL)


Why do we decide whether to skip this ddl here, but not in where we append ddls into pendingDDLs in "ddlManager.tick"?

Because all ddls must be cached in pengingDDLs until m.checkpointTs == nextDDL.CommitTs. Otherwise, the changefeed's checkpoint cannot be guaranteed to be correct.

pkg/filter/filter.go

tests/integration_tests/event_filter/data/test_truncate.sql

tests/integration_tests/event_filter/data/test_alter.sql

tests/integration_tests/event_filter/data/test.sql

CharlesCheung96 · 2024-01-23T02:36:13Z

/test all pd=7.6.0

3AceShowHand · 2024-01-23T02:55:00Z

cdc/owner/changefeed.go

@@ -597,13 +597,13 @@ LOOP2:
 	}
 	c.barriers.Update(finishBarrier, c.latestInfo.GetTargetTs())

-	f, err := filter.NewFilter(c.latestInfo.Config, "")
+	filter, err := filter.NewFilter(c.latestInfo.Config, "")


filter is also the package name, so I renamed it to f. filter looks good to me, but report warning by the IDE

Use the package alias pfilter, PTAL

3AceShowHand · 2024-01-23T02:56:21Z

cdc/owner/ddl_manager.go

+	// In a BDR mode cluster, TiCDC can receive DDLs from all roles of TiDB.
+	// However, CDC only executes the DDLs from the TiDB that has BDRRolePrimary role.
+	if m.BDRMode && ddl.BDRRole != string(ast.BDRRolePrimary) {
+		return true, "changefeed is in BDRMode and the DDL is not executed by Primary Cluster, skip it", nil


the second returned value looks used to print log, can we just print the log here, instead of return it ?

If redo is enabled, shouldSkipDDL will be called twice, so we use the return value to avoid printing duplicate logs.

ti-chi-bot · 2024-01-23T13:56:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 3AceShowHand, hongyunyan, sdojjy

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~~ [3AceShowHand,sdojjy]

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 · 2024-01-23T13:56:26Z

[LGTM Timeline notifier]

Timeline:

2024-01-23 02:57:22.064043991 +0000 UTC m=+843083.628341693: ☑️ agreed by 3AceShowHand.
2024-01-23 13:56:24.822430829 +0000 UTC m=+882626.386728534: ☑️ agreed by sdojjy.

CharlesCheung96 · 2024-01-23T17:44:15Z

/retest

CharlesCheung96 · 2024-01-23T17:45:00Z

/test all

CharlesCheung96 · 2024-01-24T01:50:55Z

/retest

CharlesCheung96 · 2024-01-24T02:40:57Z

/retest

codecov · 2024-01-24T03:02:01Z

Codecov Report

Merging #10518 (96c7b2a) into master (ea976f4) will decrease coverage by 0.0423%.
Report is 3 commits behind head on master.
The diff coverage is 66.2162%.

Additional details and impacted files

Components	Coverage Δ
cdc	`61.6866% <66.2162%> (-0.1201%)`	⬇️
dm	`51.2944% <ø> (+0.0587%)`	⬆️
engine	`63.4423% <ø> (+0.0212%)`	⬆️

Flag	Coverage Δ
unit	`57.4569% <66.2162%> (-0.0423%)`	⬇️

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

@@               Coverage Diff                @@
##             master     #10518        +/-   ##
================================================
- Coverage   57.4992%   57.4569%   -0.0423%     
================================================
  Files           849        848         -1     
  Lines        126073     125795       -278     
================================================
- Hits          72491      72278       -213     
+ Misses        48174      48116        -58     
+ Partials       5408       5401         -7

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-01-24T04:55:00Z

In response to a cherrypick label: new pull request created to branch release-6.5: #10539.

ti-chi-bot · 2024-01-24T04:55:38Z

In response to a cherrypick label: new pull request created to branch release-7.1: #10540.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-01-24T04:56:17Z

In response to a cherrypick label: new pull request created to branch release-7.5: #10541.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

close pingcap#10524

close #10524

close pingcap#10524

close #10524

asddongmen reviewed Jan 22, 2024

View reviewed changes

pkg/filter/filter.go Outdated Show resolved Hide resolved

ti-chi-bot bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 22, 2024

CharlesCheung96 force-pushed the fix-ddl-filter branch from fc9f75d to 7bc6895 Compare January 22, 2024 08:09

CharlesCheung96 force-pushed the fix-ddl-filter branch from 7bc6895 to 05666a5 Compare January 22, 2024 09:27

ti-chi-bot bot added do-not-merge/needs-triage-completed and removed do-not-merge/needs-linked-issue labels Jan 22, 2024

CharlesCheung96 marked this pull request as ready for review January 22, 2024 10:24

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 22, 2024

CharlesCheung96 requested review from asddongmen, 3AceShowHand and sdojjy January 22, 2024 10:24

hongyunyan reviewed Jan 22, 2024

View reviewed changes

CharlesCheung96 requested a review from hongyunyan January 22, 2024 15:39

ti-chi-bot bot removed the do-not-merge/needs-triage-completed label Jan 22, 2024

CharlesCheung96 commented Jan 23, 2024

View reviewed changes

tests/integration_tests/event_filter/data/test_truncate.sql Outdated Show resolved Hide resolved

CharlesCheung96 commented Jan 23, 2024

View reviewed changes

tests/integration_tests/event_filter/data/test_alter.sql Outdated Show resolved Hide resolved

CharlesCheung96 commented Jan 23, 2024

View reviewed changes

tests/integration_tests/event_filter/data/test.sql Outdated Show resolved Hide resolved

3AceShowHand reviewed Jan 23, 2024

View reviewed changes

3AceShowHand approved these changes Jan 23, 2024

View reviewed changes

fix ut

2661127

CharlesCheung96 force-pushed the fix-ddl-filter branch from e68d852 to 2661127 Compare January 23, 2024 13:53

sdojjy approved these changes Jan 23, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 23, 2024

fix ut

96c7b2a

ti-chi-bot bot merged commit 8d3e31b into pingcap:master Jan 24, 2024
27 of 28 checks passed

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Jan 24, 2024

This is an automated cherry-pick of pingcap#10518

369aa72

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Jan 24, 2024

pkg/filter(ticdc): ignore ddl in ddl_manager (#10518) #10539

Merged

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Jan 24, 2024

This is an automated cherry-pick of pingcap#10518

8894558

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Jan 24, 2024

pkg/filter(ticdc): ignore ddl in ddl_manager (#10518) #10540

Merged

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Jan 24, 2024

This is an automated cherry-pick of pingcap#10518

d7b3829

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Jan 24, 2024

pkg/filter(ticdc): ignore ddl in ddl_manager (#10518) #10541

Merged

CharlesCheung96 added a commit to ti-chi-bot/tiflow that referenced this pull request Jan 24, 2024

pkg/filter(ticdc): ignore ddl in ddl_manager (pingcap#10518)

f80fefb

close pingcap#10524

CharlesCheung96 added a commit to ti-chi-bot/tiflow that referenced this pull request Jan 24, 2024

pkg/filter(ticdc): ignore ddl in ddl_manager (pingcap#10518)

d273307

close pingcap#10524

CharlesCheung96 added a commit to ti-chi-bot/tiflow that referenced this pull request Jan 24, 2024

pkg/filter(ticdc): ignore ddl in ddl_manager (pingcap#10518)

d8b86e8

close pingcap#10524

ti-chi-bot bot pushed a commit that referenced this pull request Jan 24, 2024

pkg/filter(ticdc): ignore ddl in ddl_manager (#10518) (#10539)

300ab7e

close #10524

CharlesCheung96 added a commit to ti-chi-bot/tiflow that referenced this pull request Jan 24, 2024

pkg/filter(ticdc): ignore ddl in ddl_manager (pingcap#10518)

efe8d0e

close pingcap#10524

ti-chi-bot bot pushed a commit that referenced this pull request Jan 25, 2024

pkg/filter(ticdc): ignore ddl in ddl_manager (#10518) (#10541)

12bae00

close #10524

ti-chi-bot bot pushed a commit that referenced this pull request Jan 25, 2024

pkg/filter(ticdc): ignore ddl in ddl_manager (#10518) (#10540)

84c2df7

close #10524

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pkg/filter(ticdc): ignore ddl in ddl_manager #10518

pkg/filter(ticdc): ignore ddl in ddl_manager #10518

CharlesCheung96 commented Jan 22, 2024 •

edited

Loading

ti-chi-bot bot commented Jan 22, 2024

sre-bot commented Jan 22, 2024 •

edited

Loading

CharlesCheung96 commented Jan 22, 2024

CharlesCheung96 commented Jan 22, 2024

hongyunyan Jan 22, 2024

CharlesCheung96 Jan 22, 2024

CharlesCheung96 commented Jan 23, 2024

3AceShowHand Jan 23, 2024

CharlesCheung96 Jan 23, 2024

3AceShowHand Jan 23, 2024

CharlesCheung96 Jan 23, 2024

ti-chi-bot bot commented Jan 23, 2024

ti-chi-bot bot commented Jan 23, 2024

CharlesCheung96 commented Jan 23, 2024

CharlesCheung96 commented Jan 23, 2024

CharlesCheung96 commented Jan 24, 2024

CharlesCheung96 commented Jan 24, 2024

codecov bot commented Jan 24, 2024 •

edited

Loading

ti-chi-bot commented Jan 24, 2024

ti-chi-bot commented Jan 24, 2024

ti-chi-bot commented Jan 24, 2024

pkg/filter(ticdc): ignore ddl in ddl_manager #10518

pkg/filter(ticdc): ignore ddl in ddl_manager #10518

Conversation

CharlesCheung96 commented Jan 22, 2024 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Tests

Questions

Will it cause performance regression or break compatibility?

Do you need to update user documentation, design documentation or monitoring documentation?

Release note

ti-chi-bot bot commented Jan 22, 2024

sre-bot commented Jan 22, 2024 • edited Loading

CharlesCheung96 commented Jan 22, 2024

CharlesCheung96 commented Jan 22, 2024

hongyunyan Jan 22, 2024

Choose a reason for hiding this comment

CharlesCheung96 Jan 22, 2024

Choose a reason for hiding this comment

CharlesCheung96 commented Jan 23, 2024

3AceShowHand Jan 23, 2024

Choose a reason for hiding this comment

CharlesCheung96 Jan 23, 2024

Choose a reason for hiding this comment

3AceShowHand Jan 23, 2024

Choose a reason for hiding this comment

CharlesCheung96 Jan 23, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Jan 23, 2024

ti-chi-bot bot commented Jan 23, 2024

[LGTM Timeline notifier]

CharlesCheung96 commented Jan 23, 2024

CharlesCheung96 commented Jan 23, 2024

CharlesCheung96 commented Jan 24, 2024

CharlesCheung96 commented Jan 24, 2024

codecov bot commented Jan 24, 2024 • edited Loading

Codecov Report

ti-chi-bot commented Jan 24, 2024

ti-chi-bot commented Jan 24, 2024

ti-chi-bot commented Jan 24, 2024

CharlesCheung96 commented Jan 22, 2024 •

edited

Loading

sre-bot commented Jan 22, 2024 •

edited

Loading

codecov bot commented Jan 24, 2024 •

edited

Loading