dm/optimistic: make errors which isn't sync error handled by 'binlog skip' command #4883

lichunzhu · 2022-03-14T07:04:24Z

What problem does this PR solve?

Issue Number: close #3789

What is changed and how it works?

Add ConflictStage ConflictError and returns an error directly when dm-worker meets this stage.

Check List

Tests

Unit test
Integration test

Code changes

Has persistent data change

Side effects

Breaking backward compatibility

Related changes

Need to update the documentation

Release note

None

ti-chi-bot · 2022-03-14T07:04:25Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

Ehco1996
GMHDBJD

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.

GMHDBJD

rest LGTM

GMHDBJD · 2022-03-14T07:11:48Z

dm/tests/shardddl3_1/run.sh

-		"\"result\": true" 1
+		"binlog skip test" \
+		"\"result\": true" 2 \
+		"\"source 'mysql-replica-02' has no error\"" 1


How about check the task status after skip? Maybe insert some new rows.

GMHDBJD · 2022-03-14T07:14:13Z

chaos test https://github.com/pingcap/tiflow/actions/runs/1979146851

…into fixAddDroppedCol

lichunzhu · 2022-03-14T07:31:58Z

/run-all-tests

lichunzhu · 2022-03-14T12:14:13Z

/run-all-tests

Ehco1996 · 2022-03-15T01:31:30Z

will review after codefreeze 😂

lichunzhu · 2022-03-15T08:27:02Z

/run-all-tests

…into fixAddDroppedCol

lichunzhu · 2022-03-16T07:23:02Z

/run-all-tests

lichunzhu · 2022-03-16T09:15:30Z

/run-verify-ci

lichunzhu · 2022-03-16T09:15:41Z

/run-dm-integration-test

codecov-commenter · 2022-03-16T09:41:13Z

Codecov Report

Merging #4883 (a2a2158) into master (687e248) will decrease coverage by 0.2351%.
The diff coverage is 49.4444%.

Flag	Coverage Δ
cdc	`61.0616% <50.3067%> (+0.4630%)`	⬆️
dm	`51.7699% <49.1434%> (-0.6997%)`	⬇️

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

@@               Coverage Diff                @@
##             master      #4883        +/-   ##
================================================
- Coverage   56.1240%   55.8888%   -0.2352%     
================================================
  Files           522        524         +2     
  Lines         65325      66677      +1352     
================================================
+ Hits          36663      37265       +602     
- Misses        25094      25767       +673     
- Partials       3568       3645        +77

lichunzhu · 2022-03-17T02:42:07Z

@Ehco1996 @GMHDBJD PTAL

GMHDBJD

LGTM

Ehco1996

LGTM

Ehco1996 · 2022-03-17T03:09:15Z

dm/syncer/optimist.go

@@ -179,6 +179,9 @@ func (s *Syncer) handleQueryEventOptimistic(qec *queryEventContext) error {
 		}
 	}

+	if op.ConflictStage == optimism.ConflictError {


maybe use switch to handle different op, ok to change in next pr

lichunzhu · 2022-03-17T03:44:49Z

/merge

ti-chi-bot · 2022-03-17T03:44:51Z

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

Commit hash: a2a2158

…into fixAddDroppedCol

lichunzhu · 2022-03-17T03:49:02Z

/run-all-tests

lichunzhu · 2022-03-17T05:34:52Z

/run-leak-test

lichunzhu · 2022-03-17T05:35:00Z

/run-dm-integration-test

lichunzhu · 2022-03-17T05:36:22Z

/merge

ti-chi-bot · 2022-03-17T05:36:25Z

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

Commit hash: 389c472

lichunzhu · 2022-03-17T06:39:17Z

/run-dm-integration-test

lichunzhu added 3 commits March 14, 2022 11:32

tmp

7e5f981

tmp

b3ffc32

add ConflictError and let binlog skip to handle those cases

1af8595

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

Merge branch 'master' into fixAddDroppedCol

584f92f

ti-chi-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 14, 2022

lichunzhu requested review from GMHDBJD, WizardXiao, lance6716 and Ehco1996 and removed request for GMHDBJD and WizardXiao March 14, 2022 07:04

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

GMHDBJD reviewed Mar 14, 2022

View reviewed changes

lichunzhu added 2 commits March 14, 2022 15:31

fix and address comments

02270f7

Merge branch 'fixAddDroppedCol' of https://github.com/lichunzhu/ticdc …

b0a2ceb

…into fixAddDroppedCol

lichunzhu added 2 commits March 14, 2022 06:51

Merge branch 'master' into fixAddDroppedCol

f507aef

fix

e33f253

lichunzhu added 2 commits March 15, 2022 16:26

fix tests

7410338

Merge branch 'master' into fixAddDroppedCol

c15a892

lichunzhu added 3 commits March 15, 2022 18:54

fix tests

d2e0620

Merge branch 'fixAddDroppedCol' of https://github.com/lichunzhu/ticdc …

242b952

…into fixAddDroppedCol

Merge branch 'master' into fixAddDroppedCol

9034c8b

GMHDBJD approved these changes Mar 17, 2022

View reviewed changes

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

Ehco1996 approved these changes Mar 17, 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 17, 2022

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

ti-chi-bot and others added 5 commits March 17, 2022 11:44

Merge branch 'master' into fixAddDroppedCol

a458bf7

set skip and redirect error message for now

3345f53

Merge branch 'master' into fixAddDroppedCol

dad0fa0

Merge branch 'master' into fixAddDroppedCol

f1c3712

Merge branch 'fixAddDroppedCol' of https://github.com/lichunzhu/ticdc …

389c472

…into fixAddDroppedCol

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

pingcap deleted a comment from ti-chi-bot Mar 17, 2022

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

Merge branch 'master' into fixAddDroppedCol

d0c77a7

ti-chi-bot merged commit b0e6b2a into pingcap:master Mar 17, 2022

lichunzhu deleted the fixAddDroppedCol branch March 17, 2022 07:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dm/optimistic: make errors which isn't sync error handled by 'binlog skip' command #4883

dm/optimistic: make errors which isn't sync error handled by 'binlog skip' command #4883

lichunzhu commented Mar 14, 2022

ti-chi-bot commented Mar 14, 2022 •

edited

Loading

GMHDBJD left a comment

GMHDBJD Mar 14, 2022

lichunzhu Mar 14, 2022

GMHDBJD commented Mar 14, 2022

lichunzhu commented Mar 14, 2022

lichunzhu commented Mar 14, 2022

Ehco1996 commented Mar 15, 2022

lichunzhu commented Mar 15, 2022

lichunzhu commented Mar 16, 2022

lichunzhu commented Mar 16, 2022

lichunzhu commented Mar 16, 2022

codecov-commenter commented Mar 16, 2022 •

edited

Loading

lichunzhu commented Mar 17, 2022

GMHDBJD left a comment

Ehco1996 left a comment

Ehco1996 Mar 17, 2022

lichunzhu commented Mar 17, 2022

ti-chi-bot commented Mar 17, 2022

lichunzhu commented Mar 17, 2022

lichunzhu commented Mar 17, 2022

lichunzhu commented Mar 17, 2022

lichunzhu commented Mar 17, 2022

ti-chi-bot commented Mar 17, 2022

lichunzhu commented Mar 17, 2022

dm/optimistic: make errors which isn't sync error handled by 'binlog skip' command #4883

dm/optimistic: make errors which isn't sync error handled by 'binlog skip' command #4883

Conversation

lichunzhu 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

GMHDBJD left a comment

Choose a reason for hiding this comment

GMHDBJD Mar 14, 2022

Choose a reason for hiding this comment

lichunzhu Mar 14, 2022

Choose a reason for hiding this comment

GMHDBJD commented Mar 14, 2022

lichunzhu commented Mar 14, 2022

lichunzhu commented Mar 14, 2022

Ehco1996 commented Mar 15, 2022

lichunzhu commented Mar 15, 2022

lichunzhu commented Mar 16, 2022

lichunzhu commented Mar 16, 2022

lichunzhu commented Mar 16, 2022

codecov-commenter commented Mar 16, 2022 • edited Loading

Codecov Report

lichunzhu commented Mar 17, 2022

GMHDBJD left a comment

Choose a reason for hiding this comment

Ehco1996 left a comment

Choose a reason for hiding this comment

Ehco1996 Mar 17, 2022

Choose a reason for hiding this comment

lichunzhu commented Mar 17, 2022

ti-chi-bot commented Mar 17, 2022

lichunzhu commented Mar 17, 2022

lichunzhu commented Mar 17, 2022

lichunzhu commented Mar 17, 2022

lichunzhu commented Mar 17, 2022

ti-chi-bot commented Mar 17, 2022

lichunzhu commented Mar 17, 2022

ti-chi-bot commented Mar 14, 2022 •

edited

Loading

codecov-commenter commented Mar 16, 2022 •

edited

Loading