checker(dm): add downstream CDC/PiTR check for lightning #7839

lance6716 · 2022-12-07T08:05:09Z

Signed-off-by: lance6716 lance6716@gmail.com

What problem does this PR solve?

Issue Number: ref #3510

What is changed and how it works?

Also I remove a local disk checking in this PR, because precheck is performed at DM-master and the lightning is run at DM-worker.

Check List

Tests

Manual test (add detailed scripts or steps below)

                       {
                               "id": 28,
                               "name": "lightning_downstream_cdc_pitr",
                               "desc": "check whether the downstream has tasks incompatible with Physical import mode",
                               "state": "fail",
                               "errors": [
                                       {
                                               "severity": "fail",
                                               "short_error": "found CDC capture(s): clusterID: default captureID(s): [12e1ad9b-b461-48ba-83d8-9dc4d226001b b5def693-5849-4a54-974a-52fa34869273],\nlocal backend is not compatible with them. Please switch to tidb backend then try again."
                                       }
                               ],
                               "instruction": "you can switch to logical import mode which has no requirements on this"
                       }

Questions

Will it cause performance regression or break compatibility?

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

Release note

 `None`.

Signed-off-by: lance6716 <lance6716@gmail.com>

ti-chi-bot · 2022-12-07T08:05:11Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

D3Hunter
buchuitoudegou

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.

Signed-off-by: lance6716 <lance6716@gmail.com>

lance6716 · 2022-12-07T08:48:42Z

also check the effects of sorting-dir-physical

Signed-off-by: lance6716 <lance6716@gmail.com>

lance6716 · 2022-12-08T03:58:41Z

/cc @GMHDBJD

Signed-off-by: lance6716 <lance6716@gmail.com>

D3Hunter · 2022-12-09T09:13:06Z

dm/pkg/checker/lightning.go

-// NewLightningSortingSpaceChecker creates a new LightningSortingSpaceChecker.
-func NewLightningSortingSpaceChecker(lightningChecker restore.PrecheckItem) RealChecker {
-	return &LightningSortingSpaceChecker{inner: lightningChecker}
+// NewLightningCDCPiTRChecker creates a new LightningClusterVersionChecker.


Suggested change

// NewLightningCDCPiTRChecker creates a new LightningClusterVersionChecker.

// NewLightningCDCPiTRChecker creates a new LightningCDCPiTRChecker.

D3Hunter · 2022-12-09T09:13:53Z

dm/pkg/checker/lightning.go

+func NewLightningCDCPiTRChecker(lightningChecker restore.PrecheckItem) RealChecker {
+	c, ok := lightningChecker.(*restore.CDCPITRCheckItem)
+	if ok {
+		c.Instruction = "physical import mode is not compatible with them. Please switch to logical import mode then try again."


Suggested change

c.Instruction = "physical import mode is not compatible with them. Please switch to logical import mode then try again."

c.Instruction = "there are running CDC or PiTR, physical import mode is not compatible with them. Please switch to logical import mode then try again."

The detailed message (CDC or PiTR) is printed at lightning checker's side, like

"found PiTR log streaming task(s): [br_name],\n"+ "found CDC capture(s): clusterID: <nil> captureID(s): [f14cb04d-5ba1-410e-a59b-ccd796920e9d],\n"+ "local backend is not compatible with them. Please switch to tidb backend then try again."

no need to repeat again

D3Hunter · 2022-12-09T09:14:48Z

dm/config/checking_item.go

@@ -42,7 +42,7 @@ const (
 	LightningRegionDistributionChecking = "region_distribution"
 	LightningDownstreamVersionChecking  = "downstream_version"
 	LightningFreeSpaceChecking          = "free_space"
-	LightningSortingSpaceChecking       = "enough_sorting_space"


not needed?

seem duplicate with free_space

"sorting space" is a property of DM worker, but precheck is done at DM master. I forget this limit

buchuitoudegou

rest lgtm

buchuitoudegou · 2022-12-12T06:29:47Z

dm/config/checking_item.go

@@ -42,7 +42,7 @@ const (
 	LightningRegionDistributionChecking = "region_distribution"
 	LightningDownstreamVersionChecking  = "downstream_version"
 	LightningFreeSpaceChecking          = "free_space"
-	LightningSortingSpaceChecking       = "enough_sorting_space"


seem duplicate with free_space

dm/pkg/checker/lightning.go

Signed-off-by: lance6716 <lance6716@gmail.com>

lance6716 · 2022-12-14T02:47:18Z

/cc @gozssky @dsdashun
ptal

lance6716 · 2022-12-15T02:19:43Z

ptal @D3Hunter

lance6716 · 2022-12-16T02:12:56Z

ptal @D3Hunter

lance6716 · 2022-12-16T02:19:19Z

/merge

ti-chi-bot · 2022-12-16T02:19:22Z

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

Commit hash: 479429c

ti-chi-bot · 2022-12-16T02:19:35Z

@lance6716: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

trigger some heavy tests which will not run always when PR updated.

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

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 ti-community-infra/tichi repository.

checker(dm): add downstream CDC/PiTR check for lightning

f7757ab

Signed-off-by: lance6716 <lance6716@gmail.com>

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 7, 2022

change message

216ac26

Signed-off-by: lance6716 <lance6716@gmail.com>

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

lance6716 changed the title ~~checker(dm): add downstream CDC/PiTR check for lightning~~ [WIP]checker(dm): add downstream CDC/PiTR check for lightning Dec 7, 2022

ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 7, 2022

finish

83223a1

Signed-off-by: lance6716 <lance6716@gmail.com>

lance6716 changed the title ~~[WIP]checker(dm): add downstream CDC/PiTR check for lightning~~ checker(dm): add downstream CDC/PiTR check for lightning Dec 8, 2022

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

lance6716 requested review from buchuitoudegou and D3Hunter December 8, 2022 03:58

ti-chi-bot requested a review from GMHDBJD December 8, 2022 03:58

lance6716 added 3 commits December 8, 2022 14:37

fix lint

d3c8c02

Signed-off-by: lance6716 <lance6716@gmail.com>

change config name

964ed5d

Signed-off-by: lance6716 <lance6716@gmail.com>

Merge branch 'master' of github.com:pingcap/ticdc into add-more-check

7ab436e

D3Hunter reviewed Dec 9, 2022

View reviewed changes

buchuitoudegou approved these changes Dec 12, 2022

View reviewed changes

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

address comment

479429c

Signed-off-by: lance6716 <lance6716@gmail.com>

ti-chi-bot requested review from dsdashun and sleepymole December 14, 2022 02:47

D3Hunter approved these changes Dec 16, 2022

View reviewed changes

ti-chi-bot removed the status/LGT1 Indicates that a PR has LGTM 1. label Dec 16, 2022

ti-chi-bot added the status/LGT2 Indicates that a PR has LGTM 2. label Dec 16, 2022

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

Merge branch 'master' into add-more-check

098e1fd

ti-chi-bot merged commit f2424c7 into pingcap:master Dec 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

checker(dm): add downstream CDC/PiTR check for lightning #7839

checker(dm): add downstream CDC/PiTR check for lightning #7839

lance6716 commented Dec 7, 2022 •

edited

Loading

ti-chi-bot commented Dec 7, 2022 •

edited

Loading

lance6716 commented Dec 7, 2022

lance6716 commented Dec 8, 2022

D3Hunter Dec 9, 2022

D3Hunter Dec 9, 2022

lance6716 Dec 12, 2022

D3Hunter Dec 9, 2022

buchuitoudegou Dec 12, 2022

lance6716 Dec 12, 2022

buchuitoudegou left a comment

buchuitoudegou Dec 12, 2022

lance6716 commented Dec 14, 2022

lance6716 commented Dec 15, 2022

lance6716 commented Dec 16, 2022

lance6716 commented Dec 16, 2022

ti-chi-bot commented Dec 16, 2022

ti-chi-bot commented Dec 16, 2022

	// NewLightningCDCPiTRChecker creates a new LightningClusterVersionChecker.
	// NewLightningCDCPiTRChecker creates a new LightningCDCPiTRChecker.

	c.Instruction = "physical import mode is not compatible with them. Please switch to logical import mode then try again."
	c.Instruction = "there are running CDC or PiTR, physical import mode is not compatible with them. Please switch to logical import mode then try again."

checker(dm): add downstream CDC/PiTR check for lightning #7839

checker(dm): add downstream CDC/PiTR check for lightning #7839

Conversation

lance6716 commented Dec 7, 2022 • 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 commented Dec 7, 2022 • edited Loading

lance6716 commented Dec 7, 2022

lance6716 commented Dec 8, 2022

D3Hunter Dec 9, 2022

Choose a reason for hiding this comment

D3Hunter Dec 9, 2022

Choose a reason for hiding this comment

lance6716 Dec 12, 2022

Choose a reason for hiding this comment

D3Hunter Dec 9, 2022

Choose a reason for hiding this comment

buchuitoudegou Dec 12, 2022

Choose a reason for hiding this comment

lance6716 Dec 12, 2022

Choose a reason for hiding this comment

buchuitoudegou left a comment

Choose a reason for hiding this comment

buchuitoudegou Dec 12, 2022

Choose a reason for hiding this comment

lance6716 commented Dec 14, 2022

lance6716 commented Dec 15, 2022

lance6716 commented Dec 16, 2022

lance6716 commented Dec 16, 2022

ti-chi-bot commented Dec 16, 2022

ti-chi-bot commented Dec 16, 2022

lance6716 commented Dec 7, 2022 •

edited

Loading

ti-chi-bot commented Dec 7, 2022 •

edited

Loading