----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29210/#review65866 -----------------------------------------------------------
core/src/main/scala/kafka/log/LogCleaner.scala <https://reviews.apache.org/r/29210/#comment109115> * space after comma * For boolean or None arguments I personally prefer explicitly naming the argument - i.e., update = None * (Alternatively we can just use have the argument default to None) core/src/main/scala/kafka/log/LogCleanerManager.scala <https://reviews.apache.org/r/29210/#comment109118> We can probably drop the comment as it is understood throughout the class core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala <https://reviews.apache.org/r/29210/#comment109125> I think it would suffice to exercise this logic as part of LogCleanerIntegrationTest. We can just "simulate" deletion by removing the entry from the logs map. Although the access methods for the cleaner checkpoints are inaccessible there, we can directly read the checkpoint file. LogManagerTest.verifyCheckpointRecovery has a similar example. Let me know if that works or not. - Joel Koshy On Dec. 18, 2014, 6:59 p.m., Gwen Shapira wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29210/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2014, 6:59 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1819 > https://issues.apache.org/jira/browse/KAFKA-1819 > > > Repository: kafka > > > Description > ------- > > added locking > > > Diffs > ----- > > core/src/main/scala/kafka/log/LogCleaner.scala > f8fcb843c80eec3cf3c931df6bb472c019305253 > core/src/main/scala/kafka/log/LogCleanerManager.scala > bcfef77ed53f94017c06a884e4db14531774a0a2 > core/src/main/scala/kafka/log/LogManager.scala > 4d2924d04bc4bd62413edb0ee2d4aaf3c0052867 > core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala > 29cc01bcef9cacd8dec1f5d662644fc6fe4994bc > > Diff: https://reviews.apache.org/r/29210/diff/ > > > Testing > ------- > > > Thanks, > > Gwen Shapira > >
