*(dm): move downstream table info logic into sqlmodel #4885

lance6716 · 2022-03-14T10:47:25Z

What problem does this PR solve?

Issue Number: close #4287

What is changed and how it works?

mainly move the logic of DM's different source table info and target table info.
check index state to fit CDC usage
rename a variable

Check List

Tests

Unit test

Code changes

Has exported function/method change
Has exported variable/fields change

Release note


 `None`.

ti-chi-bot · 2022-03-14T10:47:26Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

D3Hunter
Ehco1996

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

lance6716 · 2022-03-14T10:49:14Z

/cc @WizardXiao @GMHDBJD @Ehco1996

Signed-off-by: lance6716 <[email protected]>

Ehco1996

no big problems, nice job to megre pkg of cdc and dm 🤙🤙🤙

LGTM

Ehco1996 · 2022-03-15T01:28:36Z

dm/pkg/schema/tracker.go

-	AbsoluteUKIndexInfo  *model.IndexInfo   // absolute uk index is a pk/uk(not null)
-	AvailableUKIndexList []*model.IndexInfo // index list which is all uks
+	TableInfo        *model.TableInfo // tableInfo which comes from parse create statement syntaxtree
+	WhereHandleCache *sqlmodel.WhereHandle


why call this cache ?

it's computed once for many row changes

the whole struct is cached in tr.dsTracker.tableInfos which should be called xxxCache.

prefer remove this suffix here.

pkg/sqlmodel/where_handle.go

Ehco1996 · 2022-03-15T01:38:47Z

dm/pkg/schema/tracker.go

-		dti = GetDownStreamTI(downstreamTI, originTi)
+		dti = &DownstreamTableInfo{
+			TableInfo:        downstreamTI,
+			WhereHandleCache: sqlmodel.GetWhereHandle(originTi, downstreamTI),


maybe can rename downstreamTI to downstreamTi 😂

TI = TableInfo

but another one called originTi 😂

Ehco1996 · 2022-03-15T01:42:18Z

pkg/sqlmodel/where_handle.go

+// - an UNIQUE index and the data is all NOT NULL.
+// For the last case, last used index is swapped to front.
+func (h *WhereHandle) getWhereIdxByData(data []interface{}) *model.IndexInfo {
+	if h == nil {


seems h never equal to nil?

Just to improve robustness because WhereHandle is public

maybe log something, should be a bug in most usage.

Signed-off-by: lance6716 <[email protected]>

…eam-ti

lance6716 · 2022-03-15T03:06:36Z

[](https://ci.pingcap.net/blue/organizations/jenkins/ut-check/detail/ut-check/5428/pipeline/#step-49-log-111)[](https://ci.pingcap.net/blue/organizations/jenkins/ut-check/detail/ut-check/5428/pipeline/#step-49-log-112)[](https://ci.pingcap.net/blue/organizations/jenkins/ut-check/detail/ut-check/5428/pipeline/#step-49-log-113)[](https://ci.pingcap.net/blue/organizations/jenkins/ut-check/detail/ut-check/5428/pipeline/#step-49-log-114)[](https://ci.pingcap.net/blue/organizations/jenkins/ut-check/detail/ut-check/5428/pipeline/#step-49-log-115)[](https://ci.pingcap.net/blue/organizations/jenkins/ut-check/detail/ut-check/5428/pipeline/#step-49-log-116)[](https://ci.pingcap.net/blue/organizations/jenkins/ut-check/detail/ut-check/5428/pipeline/#step-49-log-117)[2022-03-15T02:46:22.488Z] FAIL: optimist_test.go:224: testOptimist.TestOptimist

[2022-03-15T02:46:22.488Z] 

[2022-03-15T02:46:22.488Z] optimist_test.go:225:

[2022-03-15T02:46:22.488Z]     t.testOptimist(c, t.etcdTestCli, noRestart)

[2022-03-15T02:46:22.488Z] optimist_test.go:406:

[2022-03-15T02:46:22.488Z]     c.Assert(opm, HasLen, 0)

[2022-03-15T02:46:22.488Z] ... obtained map[string]map[string]map[string]map[string]optimism.Operation = map[string]map[string]map[string]map[string]optimism.Operation{"task-lock-with-init-schema":map[string]map[string]map[string]optimism.Operation{"mysql-replica-1":map[string]map[string]optimism.Operation{"foo":map[string]optimism.Operation{"bar-1":optimism.Operation{ID:"task-lock-with-init-schema-`db`.`tbl`", Task:"task-lock-with-init-schema", Source:"mysql-replica-1", UpSchema:"foo", UpTable:"bar-1", DDLs:[]string{}, ConflictStage:"none", ConflictMsg:"", Done:false, Cols:[]string{"c"}, Revision:14}}}}}

[2022-03-15T02:46:22.488Z] ... n int = 0

https://ci.pingcap.net/blue/organizations/jenkins/ut-check/detail/ut-check/5428/pipeline/

@GMHDBJD seems didn't fix the unstable case

lance6716 · 2022-03-22T06:08:05Z

ping @GMHDBJD

D3Hunter · 2022-03-22T11:01:15Z

pkg/sqlmodel/where_handle.go

+		}
+		// use downstream columns to check NOT NULL constraint
+		if ret.UniqueNotNullIdx == nil && allColsNotNull(idx, target.Columns) {
+			// TODO: considering cardinality


what does this todo mean?

D3Hunter · 2022-03-22T11:06:11Z

pkg/sqlmodel/where_handle.go

+// - an UNIQUE index and the data is all NOT NULL.
+// For the last case, last used index is swapped to front.
+func (h *WhereHandle) getWhereIdxByData(data []interface{}) *model.IndexInfo {
+	if h == nil {


maybe log something, should be a bug in most usage.

D3Hunter · 2022-03-22T11:33:28Z

dm/pkg/schema/tracker.go

-	AbsoluteUKIndexInfo  *model.IndexInfo   // absolute uk index is a pk/uk(not null)
-	AvailableUKIndexList []*model.IndexInfo // index list which is all uks
+	TableInfo        *model.TableInfo // tableInfo which comes from parse create statement syntaxtree
+	WhereHandleCache *sqlmodel.WhereHandle


the whole struct is cached in tr.dsTracker.tableInfos which should be called xxxCache.

prefer remove this suffix here.

Signed-off-by: lance6716 <[email protected]>

lance6716 · 2022-03-23T09:43:07Z

/merge

ti-chi-bot · 2022-03-23T09:43:09Z

This pull request has been accepted and is ready to merge.

Commit hash: 3a7a66e

lance6716 · 2022-03-23T11:06:29Z

/run-leak-test

*(dm): move downstream table info logic into sqlmodel

06dd525

ti-chi-bot added the release-note-none Denotes a PR that doesn't merit a release note. label Mar 14, 2022

lance6716 added the area/dm Issues or PRs related to DM. label Mar 14, 2022

ti-chi-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 14, 2022

ti-chi-bot requested review from Ehco1996, GMHDBJD and WizardXiao March 14, 2022 10:49

fix ut

1d1f306

Signed-off-by: lance6716 <[email protected]>

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 14, 2022

Ehco1996 approved these changes Mar 15, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 15, 2022

lance6716 added 2 commits March 15, 2022 10:28

address comment

530a79a

Signed-off-by: lance6716 <[email protected]>

Merge branch 'master' of github.com:pingcap/ticdc into refine-downstr…

186aad5

…eam-ti

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 15, 2022

D3Hunter reviewed Mar 22, 2022

View reviewed changes

address comment

3a7a66e

Signed-off-by: lance6716 <[email protected]>

D3Hunter approved these changes Mar 23, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 23, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 23, 2022

ti-chi-bot added 2 commits March 23, 2022 17:43

Merge branch 'master' into refine-downstream-ti

d8a39f5

Merge branch 'master' into refine-downstream-ti

11d38ed

ti-chi-bot merged commit 14d01a8 into pingcap:master Mar 23, 2022

lance6716 deleted the refine-downstream-ti branch October 13, 2022 08:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*(dm): move downstream table info logic into sqlmodel #4885

*(dm): move downstream table info logic into sqlmodel #4885

lance6716 commented Mar 14, 2022

ti-chi-bot commented Mar 14, 2022 •

edited

Loading

lance6716 commented Mar 14, 2022

Ehco1996 left a comment

Ehco1996 Mar 15, 2022

lance6716 Mar 15, 2022

D3Hunter Mar 22, 2022

Ehco1996 Mar 15, 2022

lance6716 Mar 15, 2022

Ehco1996 Mar 15, 2022

Ehco1996 Mar 15, 2022

lance6716 Mar 18, 2022

D3Hunter Mar 22, 2022

lance6716 commented Mar 15, 2022

lance6716 commented Mar 22, 2022

D3Hunter Mar 22, 2022

D3Hunter Mar 22, 2022

D3Hunter Mar 22, 2022

lance6716 commented Mar 23, 2022

ti-chi-bot commented Mar 23, 2022

lance6716 commented Mar 23, 2022

*(dm): move downstream table info logic into sqlmodel #4885

*(dm): move downstream table info logic into sqlmodel #4885

Conversation

lance6716 commented Mar 14, 2022

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Mar 14, 2022 • edited Loading

lance6716 commented Mar 14, 2022

Ehco1996 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lance6716 commented Mar 15, 2022

lance6716 commented Mar 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lance6716 commented Mar 23, 2022

ti-chi-bot commented Mar 23, 2022

lance6716 commented Mar 23, 2022

ti-chi-bot commented Mar 14, 2022 •

edited

Loading