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

(sink/cdc): fix some bugs introduced by #8949 #9010

Merged
merged 7 commits into from
May 22, 2023
Merged

(sink/cdc): fix some bugs introduced by #8949 #9010

merged 7 commits into from
May 22, 2023

Conversation

hicqu
Copy link
Contributor

@hicqu hicqu commented May 21, 2023 •
edited
Loading

What problem does this PR solve?

Issue Number: ref #8657
Issue Number: close #9008

What is changed and how it works?

Fix some bugs introduced by #8949 .

Check List

Tests

  • Unit test

Questions

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

Release note 

None

hicqu added 2 commits May 21, 2023 16:32
@hicqu
sink or redo errors should cancel SinkManager correctly
d5026d1 
Signed-off-by: qupeng <qupeng@pingcap.com>
@hicqu
don't call EventSink.Close and EventSink.WriteEvents concurrently
0398ed2 
Signed-off-by: qupeng <qupeng@pingcap.com>
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented May 21, 2023 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • asddongmen
  • nongfushanquan

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 bot added release-note-none Denotes a PR that doesn't merit a release note. 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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 21, 2023
@hicqu
a little fix
d6cc452 
Signed-off-by: qupeng <qupeng@pingcap.com>
@hicqu hicqu force-pushed the fix-sink-manager-concurrent branch from 79cccd6 to d6cc452 Compare May 21, 2023 09:56
@hicqu hicqu requested review from sdojjy and Rustin170506 May 21, 2023 10:44
@ti-chi-bot ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label May 21, 2023
@ti-chi-bot ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels May 21, 2023
@hicqu
Copy link
Contributor Author

hicqu commented May 21, 2023

/merge

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented May 21, 2023

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

Commit hash: d6cc452

@ti-chi-bot ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label May 21, 2023
@nongfushanquan
Copy link
Contributor

/retest

@hicqu
Copy link
Contributor Author

hicqu commented May 21, 2023

/retest-required

1 similar comment
@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/retest-required

@hicqu
set sorter.max-memory-percentage default 10
57dc991 
Signed-off-by: qupeng <qupeng@pingcap.com>
@ti-chi-bot ti-chi-bot bot removed the status/can-merge Indicates a PR has been approved by a committer. label May 22, 2023
@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/merge

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented May 22, 2023

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

Commit hash: 57dc991

@ti-chi-bot ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label May 22, 2023
@CharlesCheung96
Copy link
Contributor

/retest

@ti-chi-bot ti-chi-bot bot removed status/can-merge Indicates a PR has been approved by a committer. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 22, 2023
@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/retest-required

1 similar comment
@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/retest-required

@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/test dm-integration-test

@hicqu
fix
9b1497c 
Signed-off-by: qupeng <qupeng@pingcap.com>
@ti-chi-bot ti-chi-bot bot removed the status/can-merge Indicates a PR has been approved by a committer. label May 22, 2023
@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/merge

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented May 22, 2023

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

Commit hash: 9b1497c

@ti-chi-bot ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label May 22, 2023
@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/retest-required

@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/verify

2 similar comments
@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/verify

@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/verify

@purelind
Copy link
Collaborator

/retest

@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/verify

1 similar comment
@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/verify

@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/test verify

@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/test dm-integration-test

@hicqu
Copy link
Contributor Author

hicqu commented May 22, 2023

/test cdc-integration-kafka-test

@ti-chi-bot ti-chi-bot bot merged commit fbb363a into pingcap:master May 22, 2023
@ti-chi-bot ti-chi-bot mentioned this pull request May 22, 2023
Merged
@ti-chi-bot
Copy link
Member

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

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request May 22, 2023
@hicqu @ti-chi-bot
This is an automated cherry-pick of pingcap#9010
7bf6743 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot ti-chi-bot mentioned this pull request May 22, 2023
Closed
@ti-chi-bot
Copy link
Member

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

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request May 22, 2023
@hicqu @ti-chi-bot
This is an automated cherry-pick of pingcap#9010
c55e764 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@hicqu hicqu deleted the fix-sink-manager-concurrent branch May 22, 2023 14:35
ti-chi-bot bot pushed a commit that referenced this pull request May 23, 2023
@ti-chi-bot
(sink/cdc): fix some bugs introduced by #8949 (#9010) (#9015)
5bf7c8f 
ref #8657
This was referenced May 23, 2023
Closed
Closed
hicqu added a commit to ti-chi-bot/tiflow that referenced this pull request May 30, 2023
@hicqu
(sink/cdc): fix some bugs introduced by pingcap#8949 (pingcap#9010)
a8d643b 
ref pingcap#8657

Signed-off-by: qupeng <qupeng@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nongfushanquan nongfushanquan nongfushanquan approved these changes

@asddongmen asddongmen asddongmen approved these changes

@sdojjy sdojjy Awaiting requested review from sdojjy

@Rustin170506 Rustin170506 Awaiting requested review from Rustin170506

Assignees
No one assigned
Labels
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. release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cdc panic after updating changefeed and enabling row checksum
6 participants
@hicqu @nongfushanquan @CharlesCheung96 @purelind @ti-chi-bot @asddongmen