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: refine close in kafka sink #586

Merged
merged 1 commit into from
May 20, 2020
Merged

sink: refine close in kafka sink #586

merged 1 commit into from
May 20, 2020

Conversation

amyangfei
Copy link
Contributor

What problem does this PR solve?

Fix #576, also close sarama.SyncProducer and sarama.AsyncProducer before Kafka sink is destroyed.

What is changed and how it works?

  • make Close of kafkaSaramaProducer reentrant
  • use Close to release resource

Check List

Tests

  • Unit test
  • Integration test

Release note

  • No release note

@amyangfei amyangfei added the component/sink Sink component. label May 19, 2020
@amyangfei
Copy link
Contributor Author

/run-integration-tests

@amyangfei
Copy link
Contributor Author

/run-kafka-tests

@codecov-commenter
Copy link

Codecov Report

Merging #586 into master will increase coverage by 0.0414%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             master       #586        +/-   ##
================================================
+ Coverage   31.6167%   31.6581%   +0.0414%     
================================================
  Files            72         72                
  Lines          7243       7243                
================================================
+ Hits           2290       2293         +3     
+ Misses         4777       4774         -3     
  Partials        176        176

@amyangfei amyangfei added the status/ptal Could you please take a look? label May 19, 2020
zier-one
zier-one approved these changes May 19, 2020
View reviewed changes
Copy link
Contributor

@zier-one zier-one left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zier-one zier-one added LGT1 and removed status/ptal Could you please take a look? labels May 19, 2020
overvenus
overvenus approved these changes May 20, 2020
View reviewed changes
Copy link
Member

@overvenus overvenus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@overvenus overvenus merged commit 13ccbe6 into pingcap:master May 20, 2020
@amyangfei amyangfei deleted the refine-mq-sink-close branch May 20, 2020 04:59
5kbpers pushed a commit to 5kbpers/ticdc that referenced this pull request Aug 24, 2020
@amyangfei
sink: refine close in kafka sink (pingcap#586)
827fe5e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@overvenus overvenus overvenus approved these changes

@zier-one zier-one zier-one approved these changes

Assignees
No one assigned
Labels
component/sink Sink component.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

panic close of closed in kafka sink
4 participants
@amyangfei @codecov-commenter @overvenus @zier-one