> On Dec. 5, 2013, 1:54 a.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/log/LogCleaner.scala, line 205
> > <https://reviews.apache.org/r/15674/diff/2/?file=393911#file393911line205>
> >
> >     Are we going to have other checkDone implementations for this case? If 
> > not we could just save passing it as a parameter?

CheckDone() has to be defined in CleanerThread. Since Cleaner is not an inner 
class of CleanerThread, it won't have access to CheckDone unless it's passed in.


> On Dec. 5, 2013, 1:54 a.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/log/LogCleaner.scala, line 209
> > <https://reviews.apache.org/r/15674/diff/2/?file=393911#file393911line209>
> >
> >     Where should this exception be captured?

We don't need to catch this exception explicitly. Any exception will kill a 
ShutdownableThread. In ShutdownableThread.run(), we eat any exception if the 
thread is shut down normally.


> On Dec. 5, 2013, 1:54 a.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/log/LogCleaner.scala, line 524
> > <https://reviews.apache.org/r/15674/diff/2/?file=393911#file393911line524>
> >
> >     Do we have to keep two buildOffsetMap functions here?

The inner buildOffsetMap() is per segment. So, we can probably rename it to 
buildOffsetMapForSegment().


> On Dec. 5, 2013, 1:54 a.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/log/LogManager.scala, line 278
> > <https://reviews.apache.org/r/15674/diff/2/?file=393912#file393912line278>
> >
> >     Is there still a risk that after waitUntilInProgressFlusherIsDone 
> > passed another flushing procedure starts?

No, because the next flusher will not see the deleted log since it's already 
removed from the log list.


- Jun


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15674/#review29783
-----------------------------------------------------------


On Dec. 16, 2013, 5:41 p.m., Jun Rao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15674/
> -----------------------------------------------------------
> 
> (Updated Dec. 16, 2013, 5:41 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1074
>     https://issues.apache.org/jira/browse/KAFKA-1074
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> minor fix 2
> 
> 
> minor fix
> 
> 
> remove unused var and exception
> 
> 
> support pause/resume log clean and remove OptimisticLockFailureException
> 
> 
> kafka-1074; minor fix3
> 
> 
> kafka-1074; minor fix2
> 
> 
> kafka-1074; minor fix
> 
> 
> kafka-1074; better synchronization with log cleaner
> 
> 
> kafka-1074; fix 4
> 
> 
> kafka-1074; fix 3
> 
> 
> kafka-1074; fix 2
> 
> 
> kafka-1074
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 5c9307d71641ccc6c09a54b69d5aa2b4bc2a4cde 
>   core/src/main/scala/kafka/common/OptimisticLockFailureException.scala 
> 0e69110e71667ad75a460534f4422a7b6ec1cdc6 
>   core/src/main/scala/kafka/common/PausingCleaningException.scala 
> PRE-CREATION 
>   core/src/main/scala/kafka/log/Log.scala 
> beda421b558544196379bd9ab7855cea7614e8e3 
>   core/src/main/scala/kafka/log/LogCleaner.scala 
> ccde2abd99d2204775e4d3e9836aca34eb6747a3 
>   core/src/main/scala/kafka/log/LogManager.scala 
> 81be88aa618ed5614703d45a0556b77c97290085 
>   core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 
> 715845b167c44268bd7e4b76dfb69199bfb2fad4 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 242c18d47828b7c5e6b1fc219a0f1199fb1f9512 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala 
> 52d35a3ccc86a66acb57c1138d4f2fea8f4ca4b0 
>   core/src/test/scala/unit/kafka/log/CleanerTest.scala 
> 5a312bf0803c1df4636e2e64ba83036a4e8e92dd 
> 
> Diff: https://reviews.apache.org/r/15674/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jun Rao
> 
>

Reply via email to