> 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 > >