You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "philipnee (via GitHub)" <gi...@apache.org> on 2023/02/13 05:18:59 UTC
[GitHub] [kafka] philipnee opened a new pull request, #13238: KAFKA-14708: Use Java thread instead of kafka library for example purpose
philipnee opened a new pull request, #13238:
URL: https://github.com/apache/kafka/pull/13238
The purpose was mentioned in Jira:
Remove "kafka.examples.Consumer" dependency on ShutdownableThread. "examples" module should be dependent only on public APIs but not to be dependent upon server common/internal components.
### 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
[GitHub] [kafka] showuon commented on a diff in pull request #13238: KAFKA-14708: Use Java thread instead of kafka library for example purpose
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on code in PR #13238:
URL: https://github.com/apache/kafka/pull/13238#discussion_r1105357737
##########
examples/src/main/java/kafka/examples/Consumer.java:
##########
@@ -63,33 +63,36 @@ public Consumer(final String topic,
this.numMessageToConsume = numMessageToConsume;
this.messageRemaining = numMessageToConsume;
this.latch = latch;
+ this.isRunning = true;
}
KafkaConsumer<Integer, String> get() {
return consumer;
}
@Override
+ public void run() {
+ try {
+ do {
+ doWork();
+ } while (isRunning && messageRemaining > 0);
+ System.out.println(groupId + " finished reading " + numMessageToConsume + " messages");
+ } catch (Exception ignored) {
Review Comment:
Why do we ignore this exception? There might be something useful for troubleshooting, right?
##########
examples/src/main/java/kafka/examples/Consumer.java:
##########
@@ -63,33 +63,36 @@ public Consumer(final String topic,
this.numMessageToConsume = numMessageToConsume;
this.messageRemaining = numMessageToConsume;
this.latch = latch;
+ this.isRunning = true;
}
KafkaConsumer<Integer, String> get() {
return consumer;
}
@Override
+ public void run() {
+ try {
+ do {
+ doWork();
+ } while (isRunning && messageRemaining > 0);
Review Comment:
Why should we need `isRunning`? From the usage, we will wait for 5 mins for the `latch` completed, if not, `TimeoutException` will be thrown, so no other thread will call `shutdown` during it's running. Is my understanding correct?
--
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
[GitHub] [kafka] showuon commented on pull request #13238: KAFKA-14708: Use Java thread instead of kafka library for example purpose
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on PR #13238:
URL: https://github.com/apache/kafka/pull/13238#issuecomment-1431011866
@philipnee , there's spotbug error, could you help fix it?
--
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
[GitHub] [kafka] philipnee commented on a diff in pull request #13238: KAFKA-14708: Use Java thread instead of kafka library for example purpose
Posted by "philipnee (via GitHub)" <gi...@apache.org>.
philipnee commented on code in PR #13238:
URL: https://github.com/apache/kafka/pull/13238#discussion_r1106061755
##########
examples/src/main/java/kafka/examples/Consumer.java:
##########
@@ -63,33 +63,36 @@ public Consumer(final String topic,
this.numMessageToConsume = numMessageToConsume;
this.messageRemaining = numMessageToConsume;
this.latch = latch;
+ this.isRunning = true;
}
KafkaConsumer<Integer, String> get() {
return consumer;
}
@Override
+ public void run() {
+ try {
+ do {
+ doWork();
+ } while (isRunning && messageRemaining > 0);
Review Comment:
you are right, we don't need this flag there.
--
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
[GitHub] [kafka] philipnee commented on pull request #13238: KAFKA-14708: Use Java thread instead of kafka library for example purpose
Posted by "philipnee (via GitHub)" <gi...@apache.org>.
philipnee commented on PR #13238:
URL: https://github.com/apache/kafka/pull/13238#issuecomment-1432105559
The failing tests seem unrelated:
```
Build / JDK 11 and Scala 2.13 / [2] tlsProtocol=TLSv1.2, useInlinePem=true – org.apache.kafka.common.network.SslTransportLayerTest
15s
Build / JDK 11 and Scala 2.13 / testSingleNodeCluster() – org.apache.kafka.connect.mirror.integration.DedicatedMirrorIntegrationTest
1m 12s
Build / JDK 11 and Scala 2.13 / testMultiNodeCluster() – org.apache.kafka.connect.mirror.integration.DedicatedMirrorIntegrationTest
1m 55s
Build / JDK 11 and Scala 2.13 / testMultiNodeCluster() – org.apache.kafka.connect.mirror.integration.DedicatedMirrorIntegrationTest
1m 15s
```
--
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
[GitHub] [kafka] philipnee commented on pull request #13238: KAFKA-14708: Use Java thread instead of kafka library for example purpose
Posted by "philipnee (via GitHub)" <gi...@apache.org>.
philipnee commented on PR #13238:
URL: https://github.com/apache/kafka/pull/13238#issuecomment-1427361409
@satishd - randomly saw this so made some changes there. Do you want to have a look at it?
--
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
[GitHub] [kafka] showuon merged pull request #13238: KAFKA-14708: Use Java thread instead of kafka library for example purpose
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon merged PR #13238:
URL: https://github.com/apache/kafka/pull/13238
--
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
[GitHub] [kafka] philipnee commented on a diff in pull request #13238: KAFKA-14708: Use Java thread instead of kafka library for example purpose
Posted by "philipnee (via GitHub)" <gi...@apache.org>.
philipnee commented on code in PR #13238:
URL: https://github.com/apache/kafka/pull/13238#discussion_r1106052765
##########
examples/src/main/java/kafka/examples/Consumer.java:
##########
@@ -63,33 +63,36 @@ public Consumer(final String topic,
this.numMessageToConsume = numMessageToConsume;
this.messageRemaining = numMessageToConsume;
this.latch = latch;
+ this.isRunning = true;
}
KafkaConsumer<Integer, String> get() {
return consumer;
}
@Override
+ public void run() {
+ try {
+ do {
+ doWork();
+ } while (isRunning && messageRemaining > 0);
+ System.out.println(groupId + " finished reading " + numMessageToConsume + " messages");
+ } catch (Exception ignored) {
Review Comment:
Hmm good point, I'll add a log there.
--
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