dm: support FK causality for one-to-one routes by OliverS929 · Pull Request #12675 · pingcap/tiflow

OliverS929 · 2026-06-05T01:20:39Z

What problem does this PR solve?

Issue Number: ref #12350

What is changed and how it works?

Support DM FK causality when foreign_key_checks=1 and syncer.worker-count > 1 with static one-to-one route rules.

This PR:

builds FK causality relations with source table identity after route validation
makes causality key generation follow the task case-sensitive setting, including routed FK causality keys
respects the task case-sensitive setting when matching FK metadata and checking route topology
rejects route rules that map multiple source tables in the task to the same target table
rejects compact and multiple-rows DML boundary options when foreign_key_checks=1
caches successful route topology checks and invalidates the cache after schema/config state changes
adds unit and integration test coverage for routed multi-worker FK causality

A follow-up PR will explicitly reject hot updates that can change route/filter/case-sensitive/FK causality mode.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Questions

Will it cause performance regression or break compatibility?

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

Release note

Support foreign key causality for static one-to-one routing in DM multi-worker sync.

gemini-code-assist

Code Review

This pull request introduces support for foreign key causality checks under multi-worker syncer configurations with routing rules, specifically handling case-insensitive table routing and rejecting DML boundary-changing options (like compact and multiple-rows) when foreign_key_checks is enabled. The feedback identifies three potential nil pointer dereference panics due to missing nil checks on filter.Table parameters in sameForeignKeyTableIdentity, foreignKeyRouteTopologyTableID, and causalityKeySourceTableNameForRowChange.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

OliverS929 · 2026-06-05T01:27:30Z

/test pull-syncdiff-integration-test

OliverS929 · 2026-06-05T03:55:52Z

cc @joechenrh PTAL.

D3Hunter · 2026-06-09T04:07:49Z

make the file name the same as the inner used db name fk_route_src

I used db1.prepare.sql to follow the existing DM integration naming convention for source-1 SQL files. The schema name itself is still fk_route_src in the SQL and task config.

i guess existing are wrong, please remove the db1 prefix or rename using real schema name or source1

Changed in 996dcf1.

D3Hunter · 2026-06-09T04:49:50Z

 	return dmls, nil
 }

+func (s *Syncer) causalityKeySourceTableNameForRowChange(sourceTable *filter.Table) *cdcmodel.TableName {


Suggested change

func (s *Syncer) causalityKeySourceTableNameForRowChange(sourceTable *filter.Table) *cdcmodel.TableName {

func (s *Syncer) getCausalityKeySourceTableName(sourceTable *filter.Table) *cdcmodel.TableName {

The current name is meant to tie this helper to the RowChange.SetCausalityKeySourceTable hook. It only changes the source table used by CausalityKeys(), not the row change source table itself. If the shorter name is not a must-change for you, I'd prefer to keep this one.

D3Hunter · 2026-06-09T05:26:23Z

+	})
+	p := parser.New()
+
+	execTrackedDDL(t, syncer.schemaTracker, p, "", "create database db")


maybe abstract those common pattern a bit, such as prepareSimpleParentChildSchema

My concern with a higher-level helper here is that the nearby cases vary in schema case, route rules, target table names, and downstream FK definitions. The shared lower-level pieces are already covered by execTrackedDDL and expectFetchAllDoTables.

just let AI try to do it without hurting readibility

Changed in ccd9638.

D3Hunter · 2026-06-09T05:34:26Z

 	sourceTable *cdcmodel.TableName
 	targetTable *cdcmodel.TableName
+	// Optional source table name used only by CausalityKeys.
+	causalityKeySourceTable *cdcmodel.TableName


can we set sourceTable to the right table directly, so no need to add a separate field

I think we should keep these two identities separate here. sourceTable is still the real row-change source table and is used by GetSourceTable(), DML job metadata, validator/checkpoint paths, and logs. This field only overrides the table name used by CausalityKeys(), so the FK causality key can use the normalized source-domain identity without changing the row change's source table semantics.

D3Hunter · 2026-06-09T05:43:05Z

+	return ok
+}
+
+func (s *Syncer) markForeignKeyRouteTopologyChecked(cacheKey string) {


the key is always the same, what's the purpose to make a map?
add comment to explain this, if for future extension, we can do it later

The map/key came from an earlier version that cached checks per routed target set for the current FK graph, instead of one task-level check. We moved back to task-level checking because scoped checks need more support to catch parent-only DML cases early enough. With the current task-level check, there is only one checked state, so I simplified it to a single checked flag in d8c4449.

If we further extend this to scoped FK-graph checks later, the keyed cache can come back with the needed parent-side detection.

joechenrh · 2026-06-05T07:22:42Z

 func isForeignKeyChecksEnabled(session map[string]string) bool {
-	for key, value := range session {
-		if strings.EqualFold(key, "foreign_key_checks") {
-			trimmed := strings.Trim(value, " '\"")
-			return variable.TiDBOptOn(trimmed)
-		}
-	}
-	return false
+	return config.IsForeignKeyChecksEnabled(session)
 }


Why not removing this function

I kept the wrapper initially to avoid moving the existing syncer-side coverage while moving the parsing helper into config. I’ve moved that coverage to the config package and removed the wrapper in d8c4449.

joechenrh · 2026-06-09T06:01:24Z

+	case job, ok := <-causalityCh:
+		require.True(t, ok)
+		return job
+	case <-time.After(3 * time.Second):


Will this make the test flaky?

I don't think this adds a flaky timing dependency. The timeout is meant as a fail-fast guard if the causality goroutine does not produce output. The test has already put both jobs into a buffered channel and closed it, so it is expected that the receive is waiting for local goroutine processing rather than an external timing condition. Without the timeout, a bug in the causality path could leave the test hanging.

joechenrh · 2026-06-09T06:05:15Z

+	parentDMLs, err := syncer.genAndFilterInsertDMLs(tcontext.Background(), &genDMLParam{
+		sourceTable:     parentSource,
+		targetTable:     parentTarget,
+		originalData:    [][]interface{}{{10}},


nit: replace interface{} with any

Changed in d8c4449.

joechenrh · 2026-06-09T06:29:58Z

-	downstreamTableInfo *schema.DownstreamTableInfo,
-) error {
-	if downstreamTableInfo == nil || downstreamTableInfo.TableInfo == nil {
+const foreignKeyRouteTopologyTaskCacheKey = "task"


Will we have other task keys in the future?

No, not in the current design. The map/key came from an earlier version that cached checks per routed target set for the current FK graph, instead of one task-level check. We moved back to task-level checking because scoped checks need more support to catch parent-only DML cases early enough. With the current task-level check, there is only one checked state, so I simplified it to a single checked flag in d8c4449. If we further extend this to scoped FK-graph checks later, we can bring back keyed cache behavior together with the needed parent-side detection.

ti-chi-bot · 2026-06-10T03:54:44Z

@joechenrh: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

In response to this:

Instructions 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.

D3Hunter

rest lgtm

D3Hunter · 2026-06-10T03:59:49Z

+	})
+	p := parser.New()
+
+	execTrackedDDL(t, syncer.schemaTracker, p, "", "create database db")


just let AI try to do it without hurting readibility

OliverS929 · 2026-06-10T04:25:49Z

/retest

OliverS929 · 2026-06-10T04:34:59Z

/test pull-dm-compatibility-test

Benjamin2037

LGTM

ti-chi-bot · 2026-06-10T05:06:02Z

[LGTM Timeline notifier]

Timeline:

2026-06-10 04:30:56.65570283 +0000 UTC m=+934357.726020240: ☑️ agreed by D3Hunter.
2026-06-10 05:06:01.25589413 +0000 UTC m=+936462.326211520: ☑️ agreed by Benjamin2037.

OliverS929 · 2026-06-10T05:58:47Z

/retest

OliverS929 · 2026-06-10T08:45:52Z

/retest

ti-chi-bot · 2026-06-10T10:45:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Benjamin2037, D3Hunter, joechenrh, yudongusa

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Benjamin2037,D3Hunter,joechenrh]
~~dm/OWNERS~~ [Benjamin2037,D3Hunter,joechenrh]
~~dm/config/OWNERS~~ [yudongusa]

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 · 2026-06-10T11:50:10Z

In response to a cherrypick label: new pull request created to branch release-8.5: #12707.
But this PR has conflicts, please resolve them!

ref #12350

Support routed FK causality

b370658

gemini-code-assist Bot reviewed Jun 5, 2026

View reviewed changes

Comment thread dm/pkg/schema/tracker.go

Comment thread dm/syncer/syncer.go

Comment thread dm/syncer/dml.go

OliverS929 added 2 commits June 5, 2026 01:38

Update error docs

7c69880

Clean up route table ID

f58f57d

OliverS929 requested review from D3Hunter, GMHDBJD and lance6716 June 5, 2026 03:50

D3Hunter reviewed Jun 9, 2026

View reviewed changes

joechenrh reviewed Jun 9, 2026

View reviewed changes

dm: address review comments

d8c4449

joechenrh approved these changes Jun 10, 2026

View reviewed changes

OliverS929 requested review from D3Hunter and joechenrh June 10, 2026 03:54

dm: rename source sql files

996dcf1

D3Hunter reviewed Jun 10, 2026

View reviewed changes

address comment

ccd9638

OliverS929 requested a review from D3Hunter June 10, 2026 04:20

D3Hunter approved these changes Jun 10, 2026

View reviewed changes

ti-chi-bot Bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jun 10, 2026

Benjamin2037 approved these changes Jun 10, 2026

View reviewed changes

ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jun 10, 2026

yudongusa approved these changes Jun 10, 2026

View reviewed changes

ti-chi-bot Bot added the approved label Jun 10, 2026

ti-chi-bot Bot merged commit 3450f5d into pingcap:master Jun 10, 2026
41 of 42 checks passed

ti-chi-bot mentioned this pull request Jun 10, 2026

dm: support FK causality for one-to-one routes (#12675) #12707

Merged

2 of 4 tasks

OliverS929 deleted the fk-route-causality branch June 10, 2026 12:26

ti-chi-bot Bot pushed a commit that referenced this pull request Jun 11, 2026

dm: support FK causality for one-to-one routes (#12675) (#12707)

6da78d9

ref #12350

This was referenced Jun 26, 2026

master: clarify DM foreign key support for one-to-one routing pingcap/docs#23153

Merged

add v8.5.7 release notes pingcap/docs#23061

Merged

lilin90 mentioned this pull request Jun 29, 2026

master: clarify DM foreign key support for one-to-one routing pingcap/docs-cn#21747

Merged

2 of 12 tasks

This was referenced Jul 1, 2026

v8.5.7: clarify DM foreign key support for one-to-one routing (#23153) pingcap/docs#23183

Merged

v8.5.7: clarify DM foreign key support for one-to-one routing (#21747) pingcap/docs-cn#21755

Merged

	func (s Syncer) causalityKeySourceTableNameForRowChange(sourceTable filter.Table) *cdcmodel.TableName {
	func (s Syncer) getCausalityKeySourceTableName(sourceTable filter.Table) *cdcmodel.TableName {

Conversation

OliverS929 commented Jun 5, 2026

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

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

OliverS929 commented Jun 5, 2026

Uh oh!

OliverS929 commented Jun 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OliverS929 Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot Bot commented Jun 10, 2026

Uh oh!

D3Hunter left a comment

OliverS929 Jun 9, 2026 •

edited

Loading