> On Dec. 19, 2014, 1:23 a.m., Neha Narkhede wrote: > > core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala, line 231 > > <https://reviews.apache.org/r/29210/diff/1/?file=796108#file796108line231> > > > > Since the bug is about entries related to deleted topics, it will be > > good to add that verification step to all tests in DeleteTopicTest. > > Gwen Shapira wrote: > I added the "delete.topic.enable" to createTestTopicAndCluster(), which > is used by all tests in DeleteTopicTest. This exercises the part of the > code-path where we abort and checkpoint the cleaner. > > However, this does not provide any verification that the cleaner > checkpoint file was correctly updated. I wanted to add that, but it looks > like getting the content of the checkpoint file from the information > available at the delete topic tests will require quite intrusive modification > to unrelated parts of the code (the cleaner being private to the log, for > example). So I left this part out, and did some manual testing instead.
Gwen, the logic for reading/writing an offset checkpoint is encapsulated in OffsetCheckpoint.scala. I was thinking we should add a helper method to read cleaner checkpoints from there and see if the deleted topic exists or not. I do think that no matter what other testing we do, we should definitely add that as it is meant to be a sanity check to verify that all state that is associated with the deleted topic is truly gone. - Neha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29210/#review65578 ----------------------------------------------------------- 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 > >