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

Reply via email to