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.

Sign up for GitHub

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

Jump to bottom

pipeline: let sorter node report resolved ts #3029

Merged
merged 8 commits into from
Oct 27, 2021
Merged

pipeline: let sorter node report resolved ts #3029

merged 8 commits into from
Oct 27, 2021

Conversation

overvenus
Copy link
Member

What problem does this PR solve?

Decouples reporting resolved ts from flow control (sorter -> sink).
It makes flow control flexible, and makes possiable to implement a
global flow control (sorter -> sink) mechanism.

Cc #2561

master this PR
image image
image image
image image

Check List

Tests

  • Unit test
  • Integration test

Release note 

None

@overvenus
pipeline: let sorter node report resolved ts
afab963 
Decouples reporting resolved ts from flow control (sorter -> sink).
It makes flow control flexible, and makes possiable to implement a
global flow control (sorter -> sink) mechanism.

Signed-off-by: Neil Shen <overvenus@gmail.com>
@overvenus overvenus added component/sink Sink component. subject/performance Denotes an issue or pull request is related to replication performance. component/sorter Sorter component. labels Oct 13, 2021
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Oct 13, 2021 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • amyangfei
  • liuzix

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-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 13, 2021
@liuzix
Copy link
Contributor

liuzix commented Oct 13, 2021

/run-integration-tests

amyangfei
amyangfei reviewed Oct 15, 2021
View reviewed changes
cdc/processor/pipeline/sorter.go Show resolved Hide resolved
cdc/processor/pipeline/table.go Outdated Show resolved Hide resolved
pkg/pipeline/context.go Outdated Show resolved Hide resolved
@amyangfei
Copy link
Contributor

Will this PR be cherry-picked to release branches?

@overvenus
address comments
b32285a 
Signed-off-by: Neil Shen <overvenus@gmail.com>
@ti-chi-bot 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 Oct 18, 2021
@ti-chi-bot ti-chi-bot added status/LGT1 Indicates that a PR has LGTM 1. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 19, 2021
@overvenus
Merge branch 'master' into sorter-resolved-ts
fd1ba29 
Signed-off-by: Neil Shen <overvenus@gmail.com>
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 25, 2021
@overvenus
assert resolved ts regression
c085d67 
Signed-off-by: Neil Shen <overvenus@gmail.com>
@overvenus
Copy link
Member Author

@liuzix PTAL, thanks!

@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 Oct 27, 2021
@liuzix
Copy link
Contributor

liuzix commented Oct 27, 2021

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 5afff2a

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Oct 27, 2021
@liuzix
Copy link
Contributor

liuzix commented Oct 27, 2021

/merge

@codecov-commenter
Copy link

Codecov Report

Merging #3029 (5afff2a) into master (3c3b915) will decrease coverage by 0.5094%.
The diff coverage is 51.3831%.

@@               Coverage Diff                @@
##             master      #3029        +/-   ##
================================================
- Coverage   57.2251%   56.7156%   -0.5095%     
================================================
  Files           163        203        +40     
  Lines         19453      22306      +2853     
================================================
+ Hits          11132      12651      +1519     
- Misses         7261       8363      +1102     
- Partials       1060       1292       +232

@ti-chi-bot ti-chi-bot merged commit 97bfe4b into pingcap:master Oct 27, 2021
This was referenced Oct 27, 2021
Closed
Merged
Closed
This was referenced Nov 19, 2021
Merged
Closed
This was referenced Nov 22, 2021
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liuzix liuzix liuzix approved these changes

@amyangfei amyangfei amyangfei approved these changes

@ben1009 ben1009 Awaiting requested review from ben1009

Assignees
No one assigned
Labels
component/sink Sink component. component/sorter Sorter component. release-note-none Denotes a PR that doesn't merit a release note. 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. subject/performance Denotes an issue or pull request is related to replication performance.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@overvenus @ti-chi-bot @liuzix @amyangfei @codecov-commenter