You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Adrian Preston <pr...@uk.ibm.com> on 2015/03/18 12:12:35 UTC
Review Request 32199: Patch for KAFKA-1858
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32199/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-1858
https://issues.apache.org/jira/browse/KAFKA-1858
Repository: kafka
Description
-------
KAFKA-1858. Better checking for threads being cleaned up
Diffs
-----
core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala b46daa436231d5aa5c1e2992fd5c2d9a73a30c80
Diff: https://reviews.apache.org/r/32199/diff/
Testing
-------
Thanks,
Adrian Preston
Re: Review Request 32199: Patch for KAFKA-1858
Posted by Mayuresh Gharat <gh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32199/#review77060
-----------------------------------------------------------
LGTM
- Mayuresh Gharat
On March 18, 2015, 11:12 a.m., Adrian Preston wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32199/
> -----------------------------------------------------------
>
> (Updated March 18, 2015, 11:12 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1858
> https://issues.apache.org/jira/browse/KAFKA-1858
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1858. Better checking for threads being cleaned up
>
>
> Diffs
> -----
>
> core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala b46daa436231d5aa5c1e2992fd5c2d9a73a30c80
>
> Diff: https://reviews.apache.org/r/32199/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Adrian Preston
>
>
Re: Review Request 32199: Patch for KAFKA-1858
Posted by Adrian Preston <pr...@uk.ibm.com>.
> On March 18, 2015, 3:17 p.m., Jiangjie Qin wrote:
> > This will definitely make the test more stable. I am just thinking does it make sense to add a non deamon thread check in each test? So we can make sure there is no deamon thread left from some tests.
Good question. I think this depends on how the KafkaServer class is intended to be used. I'd been working from the assumption that there would only be one instance of KafkaServer in a JVM - and that the lifetime of KafkaServer was pretty much the lifetime of the JVM. Thus the checking done for non-daemon threads done in ServerShutdownTest is to ensure that Kafka brokers shut down cleanly.
- Adrian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32199/#review76896
-----------------------------------------------------------
On March 18, 2015, 11:12 a.m., Adrian Preston wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32199/
> -----------------------------------------------------------
>
> (Updated March 18, 2015, 11:12 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1858
> https://issues.apache.org/jira/browse/KAFKA-1858
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1858. Better checking for threads being cleaned up
>
>
> Diffs
> -----
>
> core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala b46daa436231d5aa5c1e2992fd5c2d9a73a30c80
>
> Diff: https://reviews.apache.org/r/32199/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Adrian Preston
>
>
Re: Review Request 32199: Patch for KAFKA-1858
Posted by Jiangjie Qin <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32199/#review76896
-----------------------------------------------------------
This will definitely make the test more stable. I am just thinking does it make sense to add a non deamon thread check in each test? So we can make sure there is no deamon thread left from some tests.
- Jiangjie Qin
On March 18, 2015, 11:12 a.m., Adrian Preston wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32199/
> -----------------------------------------------------------
>
> (Updated March 18, 2015, 11:12 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1858
> https://issues.apache.org/jira/browse/KAFKA-1858
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1858. Better checking for threads being cleaned up
>
>
> Diffs
> -----
>
> core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala b46daa436231d5aa5c1e2992fd5c2d9a73a30c80
>
> Diff: https://reviews.apache.org/r/32199/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Adrian Preston
>
>