You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "Phuc-Hong-Tran (via GitHub)" <gi...@apache.org> on 2024/02/03 13:44:42 UTC
[PR] KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread? [kafka]
Phuc-Hong-Tran opened a new pull request, #15312:
URL: https://github.com/apache/kafka/pull/15312
Changes:
1. Added backgroundEventHandler to ConsumerNetworkThread to handle unexpected exception.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread? [kafka]
Posted by "kirktrue (via GitHub)" <gi...@apache.org>.
kirktrue commented on PR #15312:
URL: https://github.com/apache/kafka/pull/15312#issuecomment-2533414719
Looks like there are some conflicts to resolve as well. Thanks!
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread? [kafka]
Posted by "jeffkbkim (via GitHub)" <gi...@apache.org>.
jeffkbkim commented on code in PR #15312:
URL: https://github.com/apache/kafka/pull/15312#discussion_r1539875541
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThread.java:
##########
@@ -87,8 +92,8 @@ public void run() {
try {
runOnce();
} catch (final Throwable e) {
- // Swallow the exception and continue
Review Comment:
I understand, but I am wondering why we added
```
// Swallow the exception and continue
```
in the first place.
The jira also asks:
> may you give some insights on why we don't want to propagate the error to the caller?
@kirktrue can you answer this?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread? [kafka]
Posted by "Phuc-Hong-Tran (via GitHub)" <gi...@apache.org>.
Phuc-Hong-Tran commented on code in PR #15312:
URL: https://github.com/apache/kafka/pull/15312#discussion_r1504404988
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThread.java:
##########
@@ -87,8 +92,8 @@ public void run() {
try {
runOnce();
} catch (final Throwable e) {
- // Swallow the exception and continue
Review Comment:
@jeffkbkim I have the answer for that question already, which is yes - we should tell the user what had gone wrong in the background thread via propagating the exception back to polling thread
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread? [kafka]
Posted by "Phuc-Hong-Tran (via GitHub)" <gi...@apache.org>.
Phuc-Hong-Tran commented on PR #15312:
URL: https://github.com/apache/kafka/pull/15312#issuecomment-2021512097
@kirktrue, I will
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread? [kafka]
Posted by "kirktrue (via GitHub)" <gi...@apache.org>.
kirktrue commented on code in PR #15312:
URL: https://github.com/apache/kafka/pull/15312#discussion_r1879175893
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThread.java:
##########
@@ -87,8 +92,8 @@ public void run() {
try {
runOnce();
} catch (final Throwable e) {
- // Swallow the exception and continue
Review Comment:
Yes, we should communicate these errors to the application thread.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread? [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #15312: KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread?
URL: https://github.com/apache/kafka/pull/15312
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread? [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15312:
URL: https://github.com/apache/kafka/pull/15312#issuecomment-2712494730
This PR is being marked as stale since it has not had any activity in 90 days. If you
would like to keep this PR alive, please leave a comment asking for a review. If the PR has
merge conflicts, update it with the latest from the base branch.
<p>
If you are having difficulty finding a reviewer, please reach out on the
[mailing list](https://kafka.apache.org/contact).
<p>
If this PR is no longer valid or desired, please feel free to close it. If no activity
occurs in the next 30 days, it will be automatically closed.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread? [kafka]
Posted by "Phuc-Hong-Tran (via GitHub)" <gi...@apache.org>.
Phuc-Hong-Tran commented on PR #15312:
URL: https://github.com/apache/kafka/pull/15312#issuecomment-1925989408
@philipnee, PTAL. Thanks in advance
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread? [kafka]
Posted by "Phuc-Hong-Tran (via GitHub)" <gi...@apache.org>.
Phuc-Hong-Tran closed pull request #15312: KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread?
URL: https://github.com/apache/kafka/pull/15312
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread? [kafka]
Posted by "jeffkbkim (via GitHub)" <gi...@apache.org>.
jeffkbkim commented on code in PR #15312:
URL: https://github.com/apache/kafka/pull/15312#discussion_r1498498011
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThread.java:
##########
@@ -87,8 +92,8 @@ public void run() {
try {
runOnce();
} catch (final Throwable e) {
- // Swallow the exception and continue
Review Comment:
This seems related to the jira comment in https://issues.apache.org/jira/projects/KAFKA/issues/KAFKA-15867.
@philipnee @kirktrue do you folks have more context?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15867: Should ConsumerNetworkThread wrap the exception and notify the polling thread? [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15312:
URL: https://github.com/apache/kafka/pull/15312#issuecomment-2791469443
This PR has been closed since it has not had any activity in 120 days. If you feel like this
was a mistake, or you would like to continue working on it, please feel free to re-open the
PR and ask for a review.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org