Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sink(ticdc): make mysql sink support split transactions #5281

Merged
merged 11 commits into from
May 17, 2022

Conversation

CharlesCheung96
Copy link
Contributor

@CharlesCheung96 CharlesCheung96 commented Apr 26, 2022

What problem does this PR solve?

Issue Number: close #5280

What is changed and how it works?

  1. Add a SplitTxn field to RowChangedEvent, which identifies whether a transaction is split from the current event.
  2. Split transactions in flow control module and record the size of transaction using txnSizeEntry.
  3. Make sink support split transactions so that a single large transaction can be written concurrently to downstream, which can significantly reduce synchronization latency.

Note: In this PR, FlowControl system still release memory based on checkPointTs. After sink module supports batch resolve mechanism, FlowControl should release memory based on txnSizeEntry.

Check List

Tests

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

total row of txn: 2 million
txn size: ~360MB
downstream: tidb cluster

image
image
image

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has interface methods change

Side effects

  • Increased code complexity

Related changes

  • Need to update the documentation
  • Need to update key monitor metrics in both TiCDC document and official document

Release note

`Reduce mysql sink synchronization latency`.

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Apr 26, 2022

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • amyangfei
  • asddongmen

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.

@ti-chi-bot ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/needs-triage-completed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed do-not-merge/needs-triage-completed labels Apr 26, 2022
@CharlesCheung96
Copy link
Contributor Author

/run-all-tests

@CharlesCheung96 CharlesCheung96 added the area/ticdc Issues or PRs related to TiCDC. label Apr 26, 2022
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 28, 2022
@CharlesCheung96 CharlesCheung96 changed the title sink(ticdc): add batch resolve mechanism to support splitting large transactions [WIP] sink(ticdc): add batch resolve mechanism to support splitting large transactions Apr 28, 2022
@ti-chi-bot 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 Apr 28, 2022
@CharlesCheung96 CharlesCheung96 force-pushed the mysql_support_split_txns branch from fea52c4 to 3ce5f99 Compare May 6, 2022 07:54
@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 6, 2022
@CharlesCheung96 CharlesCheung96 changed the title [WIP] sink(ticdc): add batch resolve mechanism to support splitting large transactions sink(ticdc): making mysql sink support split transactions May 6, 2022
@ti-chi-bot 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 May 6, 2022
@codecov-commenter
Copy link

codecov-commenter commented May 6, 2022

Codecov Report

Merging #5281 (9ffb23c) into master (687e248) will decrease coverage by 0.1393%.
The diff coverage is 51.7150%.

Flag Coverage Δ
cdc 61.0665% <51.7150%> (+0.4679%) ⬆️
dm 52.0276% <ø> (-0.4420%) ⬇️

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

@@               Coverage Diff                @@
##             master      #5281        +/-   ##
================================================
- Coverage   56.1240%   55.9846%   -0.1394%     
================================================
  Files           522        528         +6     
  Lines         65325      69519      +4194     
================================================
+ Hits          36663      38920      +2257     
- Misses        25094      26886      +1792     
- Partials       3568       3713       +145     

@CharlesCheung96 CharlesCheung96 changed the title sink(ticdc): making mysql sink support split transactions sink(ticdc): make mysql sink support split transactions May 7, 2022
@CharlesCheung96 CharlesCheung96 force-pushed the mysql_support_split_txns branch from 563e654 to fffe763 Compare May 12, 2022 07:57
@CharlesCheung96
Copy link
Contributor Author

/run-verify
/run-leak-test
/run-kafka-integration-test
/run-integration-test

@purelind
Copy link
Collaborator

/run-verify
/run-leak-test
/run-kafka-integration-test
/run-integration-test

@CharlesCheung96
Copy link
Contributor Author

/run-all-tests

@CharlesCheung96 CharlesCheung96 force-pushed the mysql_support_split_txns branch from 546305f to 574a651 Compare May 13, 2022 03:39
@ti-chi-bot ti-chi-bot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 13, 2022
@ti-chi-bot ti-chi-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 13, 2022
@CharlesCheung96 CharlesCheung96 force-pushed the mysql_support_split_txns branch from 574a651 to 5b748df Compare May 13, 2022 05:21
@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 13, 2022
@CharlesCheung96 CharlesCheung96 added the status/ptal Could you please take a look? label May 13, 2022
cdc/processor/pipeline/sorter.go Outdated Show resolved Hide resolved
cdc/sink/flowcontrol/flow_control.go Outdated Show resolved Hide resolved
@CharlesCheung96
Copy link
Contributor Author

/run-all-tests

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label May 16, 2022
@ti-chi-bot 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 May 17, 2022
@amyangfei
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 8c28ec6

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label May 17, 2022
@CharlesCheung96 CharlesCheung96 removed the status/ptal Could you please take a look? label May 17, 2022
@ti-chi-bot ti-chi-bot merged commit 5e858a4 into pingcap:master May 17, 2022
sdojjy pushed a commit to sdojjy/tiflow that referenced this pull request May 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ticdc Issues or PRs related to TiCDC. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make mysql sink support split transactions
7 participants