-
Notifications
You must be signed in to change notification settings - Fork 288
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
owner(cdc): asynchronously close the ddl sink #4249
owner(cdc): asynchronously close the ddl sink #4249
Conversation
[REVIEW NOTIFICATION] This pull request has not been approved. To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. @@ Coverage Diff @@
## master #4249 +/- ##
================================================
- Coverage 55.8077% 55.3270% -0.4807%
================================================
Files 495 495
Lines 61246 61422 +176
================================================
- Hits 34180 33983 -197
- Misses 23614 24038 +424
+ Partials 3452 3401 -51 |
4c54774
to
f8ee1dd
Compare
/run-all-tests |
/run-all-tests |
2 similar comments
/run-all-tests |
/run-all-tests |
8283ed5
to
1e05755
Compare
@3AceShowHand: PR needs rebase. 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 kubernetes/test-infra repository. |
It looks like we don't care if the close is successful, so can we just add a close timeout? That way the impact on the owner and processor is minimal? |
Such a good idea, I will do this in another new PR. #5186 |
closed by #5186 |
What problem does this PR solve?
Issue Number: close #4241, #4384
when closing the
ddlSink
with Kafka as the sink, it would block theowner
orprocessorManager
for a period of time, which had a negative impact on all running changefeeds.What is changed and how it works?
At the owner side:
ddlSink
asynchronslychangeFeedRunningStatus
, at the moment forchangeFeedClosed
,changeFeedRunning
,changeFeedClosing
.changeFeedClose
->changeFeedRunning
if initialize the changefeed.changeFeedRunning
->changefeedClosing
if close the changefeedchangeFeedClosing
->changeFeedClosed
if the downstream sink fully closed.without introducing
changeFeedClosing
, a closing changefeed may be reinitializing at the same time, also may be closing multiple times.At the
processorManager
side:sinkManager
in an asynchronous way.processorRunningStatus
, at the moment forprocessorClosed
,processorInitializing
,processorRunning
,processorClosing
.processorClosed
->processorInitializing
, if try to initialize the sinkprocessorInitializing
->processorRunning
, if the sinkManager is successfully initialized.processorRunning
->processorClosing
, if try to close the processor.processorClosing
->processorClosed
if the sinkManager is fully closed, not matter success or not.processorRunning
.Check List
Tests
Code changes
Side effects
Related changes
Release note