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

redo(ticdc): make implementation in manager asynchronously #5683

Merged
merged 15 commits into from
Jul 2, 2022

Conversation

CharlesCheung96
Copy link
Contributor

@CharlesCheung96 CharlesCheung96 commented May 31, 2022

What problem does this PR solve?

Issue Number: close #6011, ref #5920

What is changed and how it works?

  1. Make implementation in manager asynchronously.
  2. Use single goroutine bgUpdateLog to write and flush log to backend storage serially.
  3. Read redoTs directly in table_actor to decouple sinkNode's resolvedTS from redoTs.
  4. Increase the load of redo integration tests to cover large transaction scenarios.

Check List

Tests

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

image

image

Questions

Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?

Release note

`None`.

@CharlesCheung96 CharlesCheung96 added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 31, 2022
@ti-chi-bot
Copy link
Member

ti-chi-bot commented May 31, 2022

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • amyangfei
  • zhaoxinyu

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 do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 31, 2022
@CharlesCheung96 CharlesCheung96 force-pushed the refactor_redo_interface branch from 4c04494 to fcc5d5d Compare May 31, 2022 17:08
@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 31, 2022
@CharlesCheung96 CharlesCheung96 changed the title [DNM] redo(ticdc): make implmentatioin in manager asynchronously [DNM] redo(ticdc): make implementatioin in manager asynchronously May 31, 2022
@ti-chi-bot ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 31, 2022
@CharlesCheung96 CharlesCheung96 force-pushed the refactor_redo_interface branch 2 times, most recently from 02ed825 to 3c53e6a Compare June 6, 2022 10:58
@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2022

Codecov Report

Merging #5683 (76e110c) into master (792faf7) will decrease coverage by 0.9077%.
The diff coverage is 49.6487%.

Flag Coverage Δ
cdc 63.1553% <52.6530%> (-2.0516%) ⬇️
dm 51.9212% <43.1034%> (-0.0026%) ⬇️
engine 62.6395% <100.0000%> (+0.0083%) ⬆️

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

@@               Coverage Diff                @@
##             master      #5683        +/-   ##
================================================
- Coverage   58.6479%   57.7402%   -0.9078%     
================================================
  Files           711        701        -10     
  Lines         83933      82866      -1067     
================================================
- Hits          49225      47847      -1378     
- Misses        30258      30657       +399     
+ Partials       4450       4362        -88     

cdc/redo/manager.go Outdated Show resolved Hide resolved
@CharlesCheung96 CharlesCheung96 changed the title [DNM] redo(ticdc): make implementatioin in manager asynchronously [DNM] redo(ticdc): make implementation in manager asynchronously Jun 15, 2022
@CharlesCheung96 CharlesCheung96 force-pushed the refactor_redo_interface branch 3 times, most recently from e36e1e9 to b516897 Compare June 19, 2022 07:46
@ti-chi-bot ti-chi-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 Jun 19, 2022
@CharlesCheung96 CharlesCheung96 force-pushed the refactor_redo_interface branch 2 times, most recently from 5d9a9da to e5178fc Compare June 20, 2022 04:51
@ti-chi-bot ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jun 20, 2022
@CharlesCheung96 CharlesCheung96 force-pushed the refactor_redo_interface branch 3 times, most recently from d07c5a6 to 7cf1624 Compare June 21, 2022 07:05
@CharlesCheung96 CharlesCheung96 force-pushed the refactor_redo_interface branch from 76e110c to 374d695 Compare July 2, 2022 08:29
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 2, 2022
@CharlesCheung96
Copy link
Contributor Author

/run-all-tests

@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 Jul 2, 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: 374d695

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 2, 2022
@ti-chi-bot ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jul 2, 2022
@CharlesCheung96
Copy link
Contributor Author

/run-all-tests

@amyangfei
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 515377f

1 similar comment
@ti-chi-bot
Copy link
Member

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

Commit hash: 515377f

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 2, 2022
@ti-chi-bot ti-chi-bot merged commit f952bf0 into pingcap:master Jul 2, 2022
@CharlesCheung96
Copy link
Contributor Author

/label needs-cherry-pick-release-6.1

@ti-chi-bot ti-chi-bot added the needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. label Jul 6, 2022
@CharlesCheung96
Copy link
Contributor Author

/run-cherry-picker

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Jul 6, 2022
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

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

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. needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ 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. status/ptal Could you please take a look?
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make implementation in manager asynchronously
6 participants