relay(dm): send one heartbeat for successive skipped GTID #5070

lance6716 · 2022-03-30T07:40:01Z

Signed-off-by: lance6716 [email protected]

What problem does this PR solve?

Issue Number: ref #5063

What is changed and how it works?

see #5063 (comment)

Check List

Tests

Integration test
Unit tests

Related changes

Need to cherry-pick to the release branch

Release note

send one heartbeat for successive skipped GTID when enable relay log

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

ti-chi-bot · 2022-03-30T07:40:03Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

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

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

…tbeat

lance6716 · 2022-03-30T08:12:44Z

/run-dm-integration-test

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

codecov-commenter · 2022-03-30T09:16:49Z

Codecov Report

Merging #5070 (9669072) into master (687e248) will decrease coverage by 0.0540%.
The diff coverage is 55.5390%.

Flag	Coverage Δ
cdc	`61.3824% <48.5972%> (+0.7837%)`	⬆️
dm	`51.8658% <87.5000%> (-0.6038%)`	⬇️

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

@@               Coverage Diff                @@
##             master      #5070        +/-   ##
================================================
- Coverage   56.1240%   56.0699%   -0.0541%     
================================================
  Files           522        526         +4     
  Lines         65325      67398      +2073     
================================================
+ Hits          36663      37790      +1127     
- Misses        25094      25891       +797     
- Partials       3568       3717       +149

lance6716 · 2022-03-30T09:20:49Z

/run-dm-integration-test

D3Hunter · 2022-03-30T10:01:25Z

dm/relay/local_reader.go

-		if state.replaceWithHeartbeat {
+		// ref https://github.com/pingcap/tiflow/issues/5063#issuecomment-1082678211
+		// heartbeat period is implemented in LocalStreamer.GetEvent
+		if state.skipGTID {
 			switch e.Event.(type) {
 			// Only replace transaction event
 			// Other events such as FormatDescriptionEvent, RotateEvent, etc. should be the same as before
 			case *replication.RowsEvent, *replication.QueryEvent, *replication.GTIDEvent, *replication.XIDEvent, *replication.TableMapEvent:


seems we missed *replication.MariadbGTIDEvent

D3Hunter · 2022-03-30T10:04:24Z

dm/relay/local_reader.go

+		} else if lastSkipGTID && state.lastSkipGTIDHeader != nil {
+			// skipGTID is turned off after this event
+			select {
+			case s.ch <- event.GenHeartbeatEvent(state.lastSkipGTIDHeader):
+			case <-ctx.Done():


prefer remove this branch, since we don't have strong dependency on heartbeat event and LocalStreamer.GetEvent has simulate normal heartbeat.

mainly to reduce the behaving difference with MySQL, the original purpose is stated in pingcap/dm#1390

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

Ehco1996

lgtm

dm/relay/local_reader.go

lance6716 · 2022-03-31T02:07:06Z

PTAL @GMHDBJD

GMHDBJD

LGTM

lance6716 · 2022-03-31T09:35:00Z

/merge

ti-chi-bot · 2022-03-31T09:35:03Z

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

Commit hash: 9669072

lance6716 · 2022-04-01T02:53:44Z

/run-dm-integration-test

lance6716 · 2022-04-01T03:18:38Z

/run-dm-integration-test

ti-chi-bot · 2022-04-01T05:01:22Z

@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

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.

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

lance6716 · 2022-04-01T05:25:20Z

/merge

ti-chi-bot · 2022-04-01T05:25:22Z

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

Commit hash: 69c565a

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

ti-chi-bot · 2022-04-01T05:53:07Z

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

ti-chi-bot · 2022-04-01T05:53:44Z

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

ti-chi-bot · 2022-04-01T05:54:21Z

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

) ref #5063

relay(dm): send one heartbeat for successive skipped GTID

6c05916

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

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. needs-cherry-pick-release-6.0 Should cherry pick this PR to release-6.0 branch. labels Mar 30, 2022

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

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

lance6716 added needs-cherry-pick-release-5.3 Should cherry pick this PR to release-5.3 branch. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 30, 2022

lance6716 added 2 commits March 30, 2022 16:11

fix CI

29ae8fc

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

Merge branch 'master' of github.com:pingcap/ticdc into fix-relay-hear…

8dcde68

…tbeat

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

lance6716 requested review from D3Hunter and GMHDBJD and removed request for D3Hunter March 30, 2022 08:12

lance6716 requested a review from D3Hunter March 30, 2022 08:24

fix again

9357dae

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

D3Hunter reviewed Mar 30, 2022

View reviewed changes

address comment

9669072

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

D3Hunter approved these changes Mar 30, 2022

View reviewed changes

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

Ehco1996 reviewed Mar 31, 2022

View reviewed changes

dm/relay/local_reader.go Show resolved Hide resolved

GMHDBJD approved these changes Mar 31, 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 31, 2022

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

Merge branch 'master' into fix-relay-heartbeat

cab5856

ti-chi-bot added 2 commits April 1, 2022 12:01

Merge branch 'master' into fix-relay-heartbeat

46d0de9

Merge branch 'master' into fix-relay-heartbeat

b46e622

fix println

69c565a

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

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

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

ti-chi-bot merged commit 012ee38 into pingcap:master Apr 1, 2022

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

This is an automated cherry-pick of pingcap#5070

1fa1e4e

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

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

relay(dm): send one heartbeat for successive skipped GTID (#5070) #5092

Merged

This was referenced Apr 1, 2022

relay(dm): send one heartbeat for successive skipped GTID (#5070) #5093

Merged

relay(dm): send one heartbeat for successive skipped GTID (#5070) #5094

Closed

ti-chi-bot added a commit that referenced this pull request Apr 7, 2022

relay(dm): send one heartbeat for successive skipped GTID (#5070) (#5093

2e4f0b4

) ref #5063

ti-chi-bot added a commit that referenced this pull request Apr 7, 2022

relay(dm): send one heartbeat for successive skipped GTID (#5070) (#5092

2605a84

) ref #5063

lance6716 deleted the fix-relay-heartbeat branch October 13, 2022 08:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

relay(dm): send one heartbeat for successive skipped GTID #5070

relay(dm): send one heartbeat for successive skipped GTID #5070

lance6716 commented Mar 30, 2022 •

edited

Loading

ti-chi-bot commented Mar 30, 2022 •

edited

Loading

lance6716 commented Mar 30, 2022

codecov-commenter commented Mar 30, 2022 •

edited

Loading

lance6716 commented Mar 30, 2022

D3Hunter Mar 30, 2022

D3Hunter Mar 30, 2022

lance6716 Mar 30, 2022

Ehco1996 left a comment

lance6716 commented Mar 31, 2022

GMHDBJD left a comment

lance6716 commented Mar 31, 2022

ti-chi-bot commented Mar 31, 2022

lance6716 commented Apr 1, 2022

lance6716 commented Apr 1, 2022

ti-chi-bot commented Apr 1, 2022

lance6716 commented Apr 1, 2022

ti-chi-bot commented Apr 1, 2022

ti-chi-bot commented Apr 1, 2022

ti-chi-bot commented Apr 1, 2022

ti-chi-bot commented Apr 1, 2022

relay(dm): send one heartbeat for successive skipped GTID #5070

relay(dm): send one heartbeat for successive skipped GTID #5070

Conversation

lance6716 commented Mar 30, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

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

lance6716 commented Mar 30, 2022

codecov-commenter commented Mar 30, 2022 • edited Loading

Codecov Report

lance6716 commented Mar 30, 2022

D3Hunter Mar 30, 2022

Choose a reason for hiding this comment

D3Hunter Mar 30, 2022

Choose a reason for hiding this comment

lance6716 Mar 30, 2022

Choose a reason for hiding this comment

Ehco1996 left a comment

Choose a reason for hiding this comment

lance6716 commented Mar 31, 2022

GMHDBJD left a comment

Choose a reason for hiding this comment

lance6716 commented Mar 31, 2022

ti-chi-bot commented Mar 31, 2022

lance6716 commented Apr 1, 2022

lance6716 commented Apr 1, 2022

ti-chi-bot commented Apr 1, 2022

lance6716 commented Apr 1, 2022

ti-chi-bot commented Apr 1, 2022

ti-chi-bot commented Apr 1, 2022

ti-chi-bot commented Apr 1, 2022

ti-chi-bot commented Apr 1, 2022

lance6716 commented Mar 30, 2022 •

edited

Loading

ti-chi-bot commented Mar 30, 2022 •

edited

Loading

codecov-commenter commented Mar 30, 2022 •

edited

Loading