You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "poorbarcode (via GitHub)" <gi...@apache.org> on 2023/06/30 13:39:21 UTC
[GitHub] [pulsar] poorbarcode opened a new pull request, #20698: [fix][io] Close the kafka source connector close stuck
poorbarcode opened a new pull request, #20698:
URL: https://github.com/apache/pulsar/pull/20698
### Motivation
https://github.com/apache/pulsar/discussions/19880#discussioncomment-6026807 When Kafka connector is closing, it waits for the `runnerThread` to stop, the task-close is running at the same thread, so it will be stuck.
### Modifications
run `close` in another thread.
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
### Matching PR in forked repository
PR in forked repository: x
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode merged pull request #20698: [fix][io] Close the kafka source connector got stuck
Posted by "poorbarcode (via GitHub)" <gi...@apache.org>.
poorbarcode merged PR #20698:
URL: https://github.com/apache/pulsar/pull/20698
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on pull request #20698: [fix][io] Close the kafka source connector got stuck
Posted by "mattisonchao (via GitHub)" <gi...@apache.org>.
mattisonchao commented on PR #20698:
URL: https://github.com/apache/pulsar/pull/20698#issuecomment-1614695690
We only use this thread once. Maybe we can new a thread and then run the close task?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a diff in pull request #20698: [fix][io] Close the kafka source connector close stuck
Posted by "codelipenghui (via GitHub)" <gi...@apache.org>.
codelipenghui commented on code in PR #20698:
URL: https://github.com/apache/pulsar/pull/20698#discussion_r1247888015
##########
pulsar-io/kafka/src/main/java/org/apache/pulsar/io/kafka/KafkaAbstractSource.java:
##########
@@ -64,6 +66,7 @@ public abstract class KafkaAbstractSource<V> extends PushSource<V> {
private volatile boolean running = false;
private KafkaSourceConfig kafkaSourceConfig;
private Thread runnerThread;
+ private final Executor executor = Executors.newSingleThreadExecutor();
Review Comment:
And when the executor will be closed?
##########
pulsar-io/kafka/src/main/java/org/apache/pulsar/io/kafka/KafkaAbstractSource.java:
##########
@@ -64,6 +66,7 @@ public abstract class KafkaAbstractSource<V> extends PushSource<V> {
private volatile boolean running = false;
private KafkaSourceConfig kafkaSourceConfig;
private Thread runnerThread;
+ private final Executor executor = Executors.newSingleThreadExecutor();
Review Comment:
It's better to set a thread name for debugging purpose.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #20698: [fix][io] Close the kafka source connector got stuck
Posted by "poorbarcode (via GitHub)" <gi...@apache.org>.
poorbarcode commented on code in PR #20698:
URL: https://github.com/apache/pulsar/pull/20698#discussion_r1247897743
##########
pulsar-io/kafka/src/main/java/org/apache/pulsar/io/kafka/KafkaAbstractSource.java:
##########
@@ -64,6 +66,7 @@ public abstract class KafkaAbstractSource<V> extends PushSource<V> {
private volatile boolean running = false;
private KafkaSourceConfig kafkaSourceConfig;
private Thread runnerThread;
+ private final Executor executor = Executors.newSingleThreadExecutor();
Review Comment:
changed it to a static variable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #20698: [fix][io] Close the kafka source connector got stuck
Posted by "poorbarcode (via GitHub)" <gi...@apache.org>.
poorbarcode commented on PR #20698:
URL: https://github.com/apache/pulsar/pull/20698#issuecomment-1615028495
The failed [OWASP dependency check](https://github.com/apache/pulsar/actions/runs/5424405010/jobs/9865506786?pr=20698) will be fixed by https://github.com/apache/pulsar/pull/20699
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codecov-commenter commented on pull request #20698: [fix][io] Close the kafka source connector got stuck
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #20698:
URL: https://github.com/apache/pulsar/pull/20698#issuecomment-1614961907
## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#20698](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (e64e835) into [master](https://app.codecov.io/gh/apache/pulsar/commit/0e603405c2a6faaa50a47881916a2c1268de9830?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (0e60340) will **increase** coverage by `0.56%`.
> The diff coverage is `80.00%`.
[](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #20698 +/- ##
============================================
+ Coverage 72.60% 73.17% +0.56%
- Complexity 32018 32105 +87
============================================
Files 1855 1871 +16
Lines 138569 138964 +395
Branches 15250 15283 +33
============================================
+ Hits 100605 101682 +1077
+ Misses 29945 29239 -706
- Partials 8019 8043 +24
```
| Flag | Coverage Δ | |
|---|---|---|
| inttests | `24.14% <34.81%> (+0.03%)` | :arrow_up: |
| systests | `25.10% <19.25%> (?)` | |
| unittests | `72.42% <80.00%> (+0.06%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...ava/org/apache/pulsar/admin/cli/CmdNamespaces.java](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNsaWVudC10b29scy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2FkbWluL2NsaS9DbWROYW1lc3BhY2VzLmphdmE=) | `76.17% <0.00%> (-0.08%)` | :arrow_down: |
| [.../service/SystemTopicBasedTopicPoliciesService.java](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1N5c3RlbVRvcGljQmFzZWRUb3BpY1BvbGljaWVzU2VydmljZS5qYXZh) | `76.48% <50.00%> (-1.19%)` | :arrow_down: |
| [...ar/common/naming/TopicBundleAssignmentFactory.java](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi9uYW1pbmcvVG9waWNCdW5kbGVBc3NpZ25tZW50RmFjdG9yeS5qYXZh) | `66.66% <66.66%> (ø)` | |
| [...pulsar/broker/admin/impl/PersistentTopicsBase.java](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi9pbXBsL1BlcnNpc3RlbnRUb3BpY3NCYXNlLmphdmE=) | `64.33% <68.18%> (+0.20%)` | :arrow_up: |
| [...xtensions/channel/ServiceUnitStateChannelImpl.java](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9sb2FkYmFsYW5jZS9leHRlbnNpb25zL2NoYW5uZWwvU2VydmljZVVuaXRTdGF0ZUNoYW5uZWxJbXBsLmphdmE=) | `84.22% <71.42%> (-0.09%)` | :arrow_down: |
| [...n/java/org/apache/pulsar/admin/cli/CliCommand.java](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNsaWVudC10b29scy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2FkbWluL2NsaS9DbGlDb21tYW5kLmphdmE=) | `57.95% <71.42%> (-1.60%)` | :arrow_down: |
| [...rg/apache/pulsar/io/kafka/KafkaAbstractSource.java](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWlvL2thZmthL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9wdWxzYXIvaW8va2Fma2EvS2Fma2FBYnN0cmFjdFNvdXJjZS5qYXZh) | `69.71% <71.42%> (+0.65%)` | :arrow_up: |
| [.../metadata/bookkeeper/PulsarRegistrationClient.java](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLW1ldGFkYXRhL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9wdWxzYXIvbWV0YWRhdGEvYm9va2tlZXBlci9QdWxzYXJSZWdpc3RyYXRpb25DbGllbnQuamF2YQ==) | `80.85% <81.11%> (-4.11%)` | :arrow_down: |
| [.../pulsar/common/topics/TopicCompactionStrategy.java](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi90b3BpY3MvVG9waWNDb21wYWN0aW9uU3RyYXRlZ3kuamF2YQ==) | `90.90% <83.33%> (-9.10%)` | :arrow_down: |
| [...he/pulsar/broker/admin/v2/NonPersistentTopics.java](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92Mi9Ob25QZXJzaXN0ZW50VG9waWNzLmphdmE=) | `62.55% <86.95%> (+0.63%)` | :arrow_up: |
| ... and [14 more](https://app.codecov.io/gh/apache/pulsar/pull/20698?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [164 files with indirect coverage changes](https://app.codecov.io/gh/apache/pulsar/pull/20698/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #20698: [fix][io] Close the kafka source connector got stuck
Posted by "poorbarcode (via GitHub)" <gi...@apache.org>.
poorbarcode commented on code in PR #20698:
URL: https://github.com/apache/pulsar/pull/20698#discussion_r1247902825
##########
pulsar-io/kafka/src/main/java/org/apache/pulsar/io/kafka/KafkaAbstractSource.java:
##########
@@ -64,6 +66,7 @@ public abstract class KafkaAbstractSource<V> extends PushSource<V> {
private volatile boolean running = false;
private KafkaSourceConfig kafkaSourceConfig;
private Thread runnerThread;
+ private final Executor executor = Executors.newSingleThreadExecutor();
Review Comment:
already remove the executor, and just new one thread if needed
##########
pulsar-io/kafka/src/main/java/org/apache/pulsar/io/kafka/KafkaAbstractSource.java:
##########
@@ -64,6 +66,7 @@ public abstract class KafkaAbstractSource<V> extends PushSource<V> {
private volatile boolean running = false;
private KafkaSourceConfig kafkaSourceConfig;
private Thread runnerThread;
+ private final Executor executor = Executors.newSingleThreadExecutor();
Review Comment:
already remove the executor, and just new one thread if needed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #20698: [fix][io] Close the kafka source connector got stuck
Posted by "poorbarcode (via GitHub)" <gi...@apache.org>.
poorbarcode commented on code in PR #20698:
URL: https://github.com/apache/pulsar/pull/20698#discussion_r1247898062
##########
pulsar-io/kafka/src/main/java/org/apache/pulsar/io/kafka/KafkaAbstractSource.java:
##########
@@ -64,6 +66,7 @@ public abstract class KafkaAbstractSource<V> extends PushSource<V> {
private volatile boolean running = false;
private KafkaSourceConfig kafkaSourceConfig;
private Thread runnerThread;
+ private final Executor executor = Executors.newSingleThreadExecutor();
Review Comment:
> It's better to set a thread name for debugging purpose.
fixed
> And when the executor will be closed?
changed it to a static variable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on a diff in pull request #20698: [fix][io] Close the kafka source connector got stuck
Posted by "mattisonchao (via GitHub)" <gi...@apache.org>.
mattisonchao commented on code in PR #20698:
URL: https://github.com/apache/pulsar/pull/20698#discussion_r1247888893
##########
pulsar-io/kafka/src/main/java/org/apache/pulsar/io/kafka/KafkaAbstractSource.java:
##########
@@ -64,6 +66,7 @@ public abstract class KafkaAbstractSource<V> extends PushSource<V> {
private volatile boolean running = false;
private KafkaSourceConfig kafkaSourceConfig;
private Thread runnerThread;
+ private final Executor executor = Executors.newSingleThreadExecutor();
Review Comment:
please close this executor. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org