syncer(dm): fix log error caused by "COMMIT" in QueryEvent #7726

liumengya94 · 2022-11-28T06:45:47Z

What problem does this PR solve?

Issue Number: ref #7525

What is changed and how it works?

fix log error and save checkpoint for COMMIT QueryEvent

Check List

Tests

Integration test

Questions

Will it cause performance regression or break compatibility?

No

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

No

Release note

None

ti-chi-bot · 2022-11-28T06:45:49Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

D3Hunter
lance6716

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.

codecov-commenter · 2022-11-28T07:38:31Z

Codecov Report

Merging #7726 (fc7999b) into master (b0f5a0b) will increase coverage by 4.9960%.
The diff coverage is 50.6480%.

Additional details and impacted files

Flag	Coverage Δ
cdc	`66.2097% <45.4326%> (?)`
dm	`52.3405% <76.0233%> (+0.1935%)`	⬆️
engine	`64.3319% <ø> (+0.0474%)`	⬆️

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

@@               Coverage Diff                @@
##             master      #7726        +/-   ##
================================================
+ Coverage   55.0067%   60.0027%   +4.9960%     
================================================
  Files           415        813       +398     
  Lines         52739      94049     +41310     
================================================
+ Hits          29010      56432     +27422     
- Misses        20957      32723     +11766     
- Partials       2772       4894      +2122

liumengya94 · 2022-11-28T08:08:18Z

/run-dm-integration-test

liumengya94 · 2022-11-29T03:28:50Z

/cc @lance6716

dm/syncer/syncer.go

lance6716 · 2022-11-29T03:48:36Z

dm/syncer/syncer.go

@@ -2272,7 +2272,18 @@ func (s *Syncer) Run(ctx context.Context) (err error) {
 			}
 		case *replication.QueryEvent:
 			originSQL = strings.TrimSpace(string(ev.Query))
-			err2 = s.ddlWorker.HandleQueryEvent(ev, ec, originSQL)
+			if originSQL == "COMMIT" {


maybe use a fallthrough to run into the case *replication.XIDEvent: branch

lance6716 · 2022-11-29T08:57:35Z

dm/syncer/syncer.go

 				}
 			}

 			s.tctx.L().Debug("", zap.String("event", "XID"), zap.Stringer("last location", lastTxnEndLocation), log.WrapStringerField("location", endLocation))

 			job := newXIDJob(endLocation, startLocation, endLocation)
-			_, err2 = s.handleJobFunc(job)
+			_, err = s.handleJobFunc(job)
+			return needContinue, err


Suggested change

return needContinue, err

return false, err

use explicit value for better understanding

lance6716 · 2022-11-29T08:58:16Z

dm/syncer/syncer.go

+		case *replication.QueryEvent:
+			originSQL = strings.TrimSpace(string(ev.Query))
+			if originSQL == "COMMIT" {
+				needContinue, _ := funCommit()


don't call funCommit twice

lance6716 · 2022-11-29T09:00:08Z

dm/tests/start_task/run.sh

+	cleanup_data start_task
+	prepare_data_MyISAM
+
+	cp $cur/conf/source1.yaml $WORK_DIR/source1.yaml


no need to copy them if you won't change the file content

liumengya94 · 2022-11-29T13:55:12Z

/run-all-tests

liumengya94 · 2022-11-29T14:32:53Z

/cc @D3Hunter

dm/syncer/syncer.go

Co-authored-by: lance6716 <[email protected]>

D3Hunter

rest lgtm

dm/syncer/syncer.go

D3Hunter · 2022-11-30T06:19:00Z

dm/tests/start_task/run.sh

+	echo "check data"
+	check_sync_diff $WORK_DIR $cur/conf/diff_config.toml
+
+	check_log_not_contains $WORK_DIR/worker1/log/dm-worker.log "originSQL: COMMIT"


where does this log printed previously?

in trackOriginDDL, "COMMIT" QueryEvent is identified as ddl, and print log in parseOneStmt

tiflow/dm/syncer/syncer.go

Line 2797 in 3f4a8a7

stmt, err := parseOneStmt(qec)

tiflow/dm/syncer/ddl.go

Line 934 in 3f4a8a7

qec.tctx.L().Info("parse ddl", zap.String("event", "query"), zap.Stringer("query event context", qec))

[2022/11/28 14:48:30.512 +08:00] [INFO] [ddl.go:934] ["parse ddl"] [task=test] [unit="binlog replication"] [event=query] ["query event context"="{schema: , originSQL: COMMIT, startLocation: position: (mysql-bin|000001.000001, 4407), gtid-set: e266a895-6e51-11ed-b110-0242ac110002:1-16, endLocation: position: (mysql-bin|000001.000001, 4476), gtid-set: e266a895-6e51-11ed-b110-0242ac110002:1-16, lastLocation: position: (mysql-bin|000001.000001, 4476), gtid-set: e266a895-6e51-11ed-b110-0242ac110002:1-16, re-sync: , needHandleDDLs: , trackInfos: }"]

liumengya94 · 2022-11-30T08:00:03Z

/run-all-tests

liumengya94 · 2022-11-30T08:37:02Z

/run-dm-integration-test

liumengya94 · 2022-11-30T09:01:30Z

/run-dm-integration-test

liumengya94 · 2022-11-30T16:16:51Z

/run-all-tests

lance6716 · 2022-12-01T01:45:39Z

/run-dm-integration-test

lance6716 · 2022-12-01T01:46:39Z

/merge

ti-chi-bot · 2022-12-01T01:46:43Z

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

Commit hash: 030d73c

lance6716 · 2022-12-01T01:47:43Z

/run-all-tests

purelind · 2022-12-01T02:26:16Z

/run-all-tests

lance6716 · 2022-12-01T04:40:19Z

/run-dm-integration-test
/run-verify

lance6716 · 2022-12-01T05:15:07Z

/run-dm-integration-test

ti-chi-bot · 2022-12-01T13:08:12Z

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

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.

lance6716 · 2022-12-01T13:10:15Z

/run-all-tests

ti-chi-bot · 2022-12-01T13:42:40Z

In response to a cherrypick label: new pull request created: #7773.

Signed-off-by: ti-chi-bot <[email protected]>

…7773)

liumengya94 added 2 commits November 28, 2022 14:30

add test for COMMIT in QueryEvent

eeb011f

add test for COMMIT in QueryEvent

0a2e079

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 28, 2022

liumengya94 and others added 2 commits November 28, 2022 14:47

Merge branch 'master' into issue-7525

ec7bc24

add test for COMMIT in QueryEvent

2b37d88

save checkpoint for COMMIT in QueryEvent

14d4062

ti-chi-bot requested a review from lance6716 November 29, 2022 03:28

lance6716 reviewed Nov 29, 2022

View reviewed changes

save checkpoint for COMMIT in QueryEvent

f25cec6

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 29, 2022

liumengya94 requested a review from lance6716 November 29, 2022 08:48

lance6716 reviewed Nov 29, 2022

View reviewed changes

liumengya94 and others added 2 commits November 29, 2022 17:46

save checkpoint for COMMIT in QueryEvent

526b2f8

Merge branch 'master' into issue-7525

2b9f844

lance6716 changed the title ~~fix log error caused by "COMMIT" in QueryEvent~~ syncer(dm): fix log error caused by "COMMIT" in QueryEvent Nov 29, 2022

fix lint error

17baa88

ti-chi-bot requested a review from D3Hunter November 29, 2022 14:32

lance6716 approved these changes Nov 30, 2022

View reviewed changes

dm/syncer/syncer.go Outdated Show resolved Hide resolved

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

Update dm/syncer/syncer.go

5de629a

Co-authored-by: lance6716 <[email protected]>

D3Hunter reviewed Nov 30, 2022

View reviewed changes

Merge branch 'master' into issue-7525

6e37b68

D3Hunter approved these changes Nov 30, 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 Nov 30, 2022

liumengya94 and others added 2 commits December 1, 2022 00:12

fix test error

e4ae452

Merge branch 'master' into issue-7525

030d73c

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

ti-chi-bot added 5 commits December 1, 2022 17:02

Merge branch 'master' into issue-7525

025e0f1

Merge branch 'master' into issue-7525

000ab9a

Merge branch 'master' into issue-7525

f2a89e0

Merge branch 'master' into issue-7525

fc7999b

Merge branch 'master' into issue-7525

76bfc4e

ti-chi-bot merged commit bc176c3 into pingcap:master Dec 1, 2022

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Dec 1, 2022

This is an automated cherry-pick of pingcap#7726

de95445

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request Dec 1, 2022

syncer(dm): fix log error caused by "COMMIT" in QueryEvent (#7726) #7773

Merged

GMHDBJD pushed a commit that referenced this pull request Jan 16, 2023

syncer(dm): fix log error caused by "COMMIT" in QueryEvent (#7726) (#…

7dfb18a

…7773)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

syncer(dm): fix log error caused by "COMMIT" in QueryEvent #7726

syncer(dm): fix log error caused by "COMMIT" in QueryEvent #7726

liumengya94 commented Nov 28, 2022 •

edited

Loading

ti-chi-bot commented Nov 28, 2022 •

edited

Loading

codecov-commenter commented Nov 28, 2022 •

edited

Loading

liumengya94 commented Nov 28, 2022

liumengya94 commented Nov 29, 2022

lance6716 Nov 29, 2022

lance6716 Nov 29, 2022

lance6716 Nov 29, 2022

lance6716 Nov 29, 2022

liumengya94 commented Nov 29, 2022

liumengya94 commented Nov 29, 2022

D3Hunter left a comment

D3Hunter Nov 30, 2022

liumengya94 Nov 30, 2022

liumengya94 commented Nov 30, 2022

liumengya94 commented Nov 30, 2022

liumengya94 commented Nov 30, 2022

liumengya94 commented Nov 30, 2022

lance6716 commented Dec 1, 2022

lance6716 commented Dec 1, 2022

ti-chi-bot commented Dec 1, 2022

lance6716 commented Dec 1, 2022

purelind commented Dec 1, 2022

lance6716 commented Dec 1, 2022

lance6716 commented Dec 1, 2022

ti-chi-bot commented Dec 1, 2022

lance6716 commented Dec 1, 2022

ti-chi-bot commented Dec 1, 2022

syncer(dm): fix log error caused by "COMMIT" in QueryEvent #7726

syncer(dm): fix log error caused by "COMMIT" in QueryEvent #7726

Conversation

liumengya94 commented Nov 28, 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 Nov 28, 2022 • edited Loading

codecov-commenter commented Nov 28, 2022 • edited Loading

Codecov Report

liumengya94 commented Nov 28, 2022

liumengya94 commented Nov 29, 2022

lance6716 Nov 29, 2022

Choose a reason for hiding this comment

lance6716 Nov 29, 2022

Choose a reason for hiding this comment

lance6716 Nov 29, 2022

Choose a reason for hiding this comment

lance6716 Nov 29, 2022

Choose a reason for hiding this comment

liumengya94 commented Nov 29, 2022

liumengya94 commented Nov 29, 2022

D3Hunter left a comment

Choose a reason for hiding this comment

D3Hunter Nov 30, 2022

Choose a reason for hiding this comment

liumengya94 Nov 30, 2022

Choose a reason for hiding this comment

liumengya94 commented Nov 30, 2022

liumengya94 commented Nov 30, 2022

liumengya94 commented Nov 30, 2022

liumengya94 commented Nov 30, 2022

lance6716 commented Dec 1, 2022

lance6716 commented Dec 1, 2022

ti-chi-bot commented Dec 1, 2022

lance6716 commented Dec 1, 2022

purelind commented Dec 1, 2022

lance6716 commented Dec 1, 2022

lance6716 commented Dec 1, 2022

ti-chi-bot commented Dec 1, 2022

lance6716 commented Dec 1, 2022

ti-chi-bot commented Dec 1, 2022

liumengya94 commented Nov 28, 2022 •

edited

Loading

ti-chi-bot commented Nov 28, 2022 •

edited

Loading

codecov-commenter commented Nov 28, 2022 •

edited

Loading