(redo)ticdc: fix the event orderliness in redo log #11117

hongyunyan · 2024-05-16T05:19:04Z

What problem does this PR solve?

Issue Number: close #11096

What is changed and how it works?

Fix the less function of logHeap to make dml event of redo log in the same txn sorted as delete/update/insert.
use startTs instead of commitTs in redo apply to distinguish different txns.

Check List

Tests

Unit test
Manual test (add detailed scripts or steps below)

create mysql sink changefeed with redo log on
run gotpc workload
After 30m, pause changefeed and run redo apply

Questions

Will it cause performance regression or break compatibility?

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

Release note

None

codecov · 2024-05-16T06:32:17Z

Codecov Report

Attention: Patch coverage is 95.23810% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 57.6426%. Comparing base (aed7768) to head (2d51f8e).
Report is 3 commits behind head on master.

Additional details and impacted files

Components	Coverage Δ
cdc	`62.1232% <95.2381%> (+0.1692%)`	⬆️
dm	`51.2358% <ø> (-0.0092%)`	⬇️
engine	`63.4161% <ø> (+0.0070%)`	⬆️

Flag	Coverage Δ
unit	`57.6426% <95.2381%> (+0.0959%)`	⬆️

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

@@               Coverage Diff                @@
##             master     #11117        +/-   ##
================================================
+ Coverage   57.5466%   57.6426%   +0.0959%     
================================================
  Files           853        854         +1     
  Lines        125679     126266       +587     
================================================
+ Hits          72324      72783       +459     
- Misses        47986      48102       +116     
- Partials       5369       5381        +12

lidezhu

LGTM

CharlesCheung96 · 2024-05-16T09:06:50Z

cdc/redo/reader/reader.go

-		h[i].data.RedoRow.Row.StartTs < h[j].data.RedoRow.Row.StartTs {
-		return true
+	if h[i].data.RedoRow.Row.CommitTs == h[j].data.RedoRow.Row.CommitTs {
+		if h[i].data.RedoRow.Row.StartTs < h[j].data.RedoRow.Row.StartTs {


remove useless else branch:

Suggested change

if h[i].data.RedoRow.Row.StartTs < h[j].data.RedoRow.Row.StartTs {

if h[i].data.RedoRow.Row.StartTs != h[j].data.RedoRow.Row.StartTs {

return h[i].data.RedoRow.Row.StartTs < h[j].data.RedoRow.Row.StartTs

}

...

Good Point!

ti-chi-bot · 2024-05-16T13:11:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CharlesCheung96, lidezhu

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [CharlesCheung96,lidezhu]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2024-05-16T13:11:23Z

[LGTM Timeline notifier]

Timeline:

2024-05-16 08:55:35.498848527 +0000 UTC m=+1729889.255984101: ☑️ agreed by lidezhu.
2024-05-16 13:11:22.424236664 +0000 UTC m=+1745236.181372265: ☑️ agreed by CharlesCheung96.

CharlesCheung96 · 2024-05-16T13:11:31Z

/retest

hongyunyan · 2024-05-16T13:39:11Z

/retest

hongyunyan · 2024-05-17T02:32:49Z

/retest

hongyunyan · 2024-05-17T03:00:30Z

/retest

hongyunyan · 2024-05-17T03:02:07Z

/cherry-pick release-8.1

ti-chi-bot · 2024-05-17T03:02:10Z

@hongyunyan: once the present PR merges, I will cherry-pick it on top of release-8.1 in the new PR and assign it to you.

In response to this:

/cherry-pick release-8.1

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.

hongyunyan · 2024-05-17T04:08:21Z

/test cdc-integration-kafka-test

3AceShowHand · 2024-05-17T04:23:26Z

cdc/redo/reader/reader.go

+			return h[i].data.RedoRow.Row.StartTs < h[j].data.RedoRow.Row.StartTs
+		}
+		// in the same txn, we need to sort by delete/update/insert order
+		if h[i].data.RedoRow.Row.ToRowChangedEvent().IsDelete() {


ToRowChangedEvent should only be called once, if it may cost some resource.

else is redundant I think.

3AceShowHand · 2024-05-17T04:25:11Z

cdc/redo/reader/reader_test.go

+		expect bool
+	}{
+		{
+			name: "Delete before Update",


These data is hard to read, use schema test helper to new events is preferred.

I would suggest that test for each case independently, instead of create a tests and loop over each.

ti-chi-bot · 2024-05-17T05:03:27Z

@hongyunyan: new pull request created to branch release-8.1: #11130.

In response to this:

/cherry-pick release-8.1

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.

ti-chi-bot · 2024-05-17T05:04:04Z

In response to a cherrypick label: new pull request could not be created: failed to create pull request against pingcap/tiflow#release-8.1 from head ti-chi-bot:cherry-pick-11117-to-release-8.1: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for ti-chi-bot:cherry-pick-11117-to-release-8.1."}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request"}

close #11096

lidezhu · 2024-05-21T10:00:52Z

/run-cherry-picker

ti-chi-bot · 2024-05-21T10:01:19Z

In response to a cherrypick label: new pull request created to branch release-7.5: #11155.

close #11096

lidezhu · 2024-05-27T13:15:05Z

/run-cherry-picker

ti-chi-bot · 2024-05-27T13:15:34Z

In response to a cherrypick label: new pull request created to branch release-6.5: #11180.

ti-chi-bot · 2024-05-27T13:15:35Z

In response to a cherrypick label: new pull request created to branch release-7.1: #11181.

close #11096

commit c092599 Author: Ti Chi Robot <ti-community-prow-bot@tidb.io> Date: Wed Jun 12 00:26:59 2024 +0800 pkg/config, sink(ticdc): support output raw change event for mq and cloud storage sink (pingcap#11226) (pingcap#11290) close pingcap#11211 commit 3426e46 Author: Ti Chi Robot <ti-community-prow-bot@tidb.io> Date: Tue Jun 11 19:40:29 2024 +0800 puller(ticdc): fix wrong update splitting behavior after table scheduling (pingcap#11269) (pingcap#11282) close pingcap#11219 commit 2a28078 Author: Ti Chi Robot <ti-community-prow-bot@tidb.io> Date: Tue Jun 11 16:40:37 2024 +0800 mysql(ticdc): remove error filter when check isTiDB in backend init (pingcap#11214) (pingcap#11261) close pingcap#11213 commit 2425d54 Author: Ti Chi Robot <ti-community-prow-bot@tidb.io> Date: Tue Jun 11 16:40:30 2024 +0800 log(ticdc): Add more error query information to the returned error to facilitate users to know the cause of the failure (pingcap#10945) (pingcap#11257) close pingcap#11254 commit 053cdaf Author: Ti Chi Robot <ti-community-prow-bot@tidb.io> Date: Tue Jun 11 15:34:30 2024 +0800 cdc: log slow conflict detect every 60s (pingcap#11251) (pingcap#11287) close pingcap#11271 commit 327ba7b Author: Ti Chi Robot <ti-community-prow-bot@tidb.io> Date: Tue Jun 11 11:42:00 2024 +0800 redo(ticdc): return internal error in redo writer (pingcap#11011) (pingcap#11091) close pingcap#10124 commit d82ae89 Author: Ti Chi Robot <ti-community-prow-bot@tidb.io> Date: Mon Jun 10 22:28:29 2024 +0800 ddl_puller (ticdc): handle dorp pk/uk ddl correctly (pingcap#10965) (pingcap#10981) close pingcap#10890 commit f15bec9 Author: Ti Chi Robot <ti-community-prow-bot@tidb.io> Date: Fri Jun 7 16:16:28 2024 +0800 redo(ticdc): enable pprof and set memory limit for redo applier (pingcap#10904) (pingcap#10996) close pingcap#10900 commit ba50a0e Author: Ti Chi Robot <ti-community-prow-bot@tidb.io> Date: Wed Jun 5 19:58:26 2024 +0800 test(ticdc): enable sequence test (pingcap#11023) (pingcap#11037) close pingcap#11015 commit 94b9897 Author: Ti Chi Robot <ti-community-prow-bot@tidb.io> Date: Wed Jun 5 17:08:56 2024 +0800 mounter(ticdc): timezone fill default value should also consider tz. (pingcap#10932) (pingcap#10946) close pingcap#10931 commit a912d33 Author: Ti Chi Robot <ti-community-prow-bot@tidb.io> Date: Wed Jun 5 10:49:25 2024 +0800 mysql (ticdc): Improve the performance of the mysql sink by refining the transaction event batching logic (pingcap#10466) (pingcap#11242) close pingcap#11241 commit 6277d9a Author: dongmen <20351731+asddongmen@users.noreply.github.com> Date: Wed May 29 20:13:22 2024 +0800 kvClient (ticdc): revert e5999e3 to remove useless metrics (pingcap#11184) close pingcap#11073 commit 54e93ed Author: dongmen <20351731+asddongmen@users.noreply.github.com> Date: Wed May 29 17:43:22 2024 +0800 syncpoint (ticdc): make syncpoint support base64 encoded password (pingcap#11162) close pingcap#10516 commit 0ba9329 Author: Ti Chi Robot <ti-community-prow-bot@tidb.io> Date: Wed May 29 09:07:21 2024 +0800 (redo)ticdc: fix the event orderliness in redo log (pingcap#11117) (pingcap#11180) close pingcap#11096 Signed-off-by: qupeng <qupeng@pingcap.com>

update

a260040

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 16, 2024

hongyunyan added 2 commits May 16, 2024 13:58

add test

875cbd9

update

cbc5aca

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 16, 2024

update

4e44fbc

ti-chi-bot bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 16, 2024

lidezhu approved these changes May 16, 2024

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels May 16, 2024

CharlesCheung96 reviewed May 16, 2024

View reviewed changes

update

cef3111

CharlesCheung96 approved these changes May 16, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels May 16, 2024

update

2d51f8e

3AceShowHand reviewed May 17, 2024

View reviewed changes

ti-chi-bot bot merged commit e75248d into pingcap:master May 17, 2024
28 checks passed

ti-chi-bot added the needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. label May 17, 2024

ti-chi-bot mentioned this pull request May 17, 2024

(redo)ticdc: fix the event orderliness in redo log (#11117) #11130

Merged

ti-chi-bot bot pushed a commit that referenced this pull request May 17, 2024

(redo)ticdc: fix the event orderliness in redo log (#11117) (#11130)

9a296b9

close #11096

lidezhu added the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label May 21, 2024

ti-chi-bot mentioned this pull request May 21, 2024

(redo)ticdc: fix the event orderliness in redo log (#11117) #11155

Merged

ti-chi-bot bot pushed a commit that referenced this pull request May 22, 2024

(redo)ticdc: fix the event orderliness in redo log (#11117) (#11155)

7806104

close #11096

lidezhu added needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. labels May 27, 2024

ti-chi-bot mentioned this pull request May 27, 2024

(redo)ticdc: fix the event orderliness in redo log (#11117) #11180

Merged

ti-chi-bot mentioned this pull request May 27, 2024

(redo)ticdc: fix the event orderliness in redo log (#11117) #11181

Merged

ti-chi-bot bot pushed a commit that referenced this pull request May 29, 2024

(redo)ticdc: fix the event orderliness in redo log (#11117) (#11181)

6db1dc2

close #11096

ti-chi-bot bot pushed a commit that referenced this pull request May 29, 2024

(redo)ticdc: fix the event orderliness in redo log (#11117) (#11180)

0ba9329

close #11096

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(redo)ticdc: fix the event orderliness in redo log #11117

(redo)ticdc: fix the event orderliness in redo log #11117

hongyunyan commented May 16, 2024 •

edited

Loading

codecov bot commented May 16, 2024 •

edited

Loading

lidezhu left a comment

CharlesCheung96 May 16, 2024

hongyunyan May 16, 2024

ti-chi-bot bot commented May 16, 2024

ti-chi-bot bot commented May 16, 2024

CharlesCheung96 commented May 16, 2024

hongyunyan commented May 16, 2024

hongyunyan commented May 17, 2024

hongyunyan commented May 17, 2024

hongyunyan commented May 17, 2024

ti-chi-bot commented May 17, 2024

hongyunyan commented May 17, 2024

3AceShowHand May 17, 2024

3AceShowHand May 17, 2024

ti-chi-bot commented May 17, 2024

ti-chi-bot commented May 17, 2024

lidezhu commented May 21, 2024

ti-chi-bot commented May 21, 2024

lidezhu commented May 27, 2024

ti-chi-bot commented May 27, 2024

ti-chi-bot commented May 27, 2024

(redo)ticdc: fix the event orderliness in redo log #11117

(redo)ticdc: fix the event orderliness in redo log #11117

Conversation

hongyunyan commented May 16, 2024 • 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

codecov bot commented May 16, 2024 • edited Loading

Codecov Report

lidezhu left a comment

Choose a reason for hiding this comment

CharlesCheung96 May 16, 2024

Choose a reason for hiding this comment

hongyunyan May 16, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented May 16, 2024

ti-chi-bot bot commented May 16, 2024

[LGTM Timeline notifier]

CharlesCheung96 commented May 16, 2024

hongyunyan commented May 16, 2024

hongyunyan commented May 17, 2024

hongyunyan commented May 17, 2024

hongyunyan commented May 17, 2024

ti-chi-bot commented May 17, 2024

hongyunyan commented May 17, 2024

3AceShowHand May 17, 2024

Choose a reason for hiding this comment

3AceShowHand May 17, 2024

Choose a reason for hiding this comment

ti-chi-bot commented May 17, 2024

ti-chi-bot commented May 17, 2024

lidezhu commented May 21, 2024

ti-chi-bot commented May 21, 2024

lidezhu commented May 27, 2024

ti-chi-bot commented May 27, 2024

ti-chi-bot commented May 27, 2024

hongyunyan commented May 16, 2024 •

edited

Loading

codecov bot commented May 16, 2024 •

edited

Loading