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