----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15674/#review29783 -----------------------------------------------------------
core/src/main/scala/kafka/log/LogCleaner.scala <https://reviews.apache.org/r/15674/#comment57268> Are we going to have other checkDone implementations for this case? If not we could just save passing it as a parameter? core/src/main/scala/kafka/log/LogCleaner.scala <https://reviews.apache.org/r/15674/#comment57267> Where should this exception be captured? core/src/main/scala/kafka/log/LogCleaner.scala <https://reviews.apache.org/r/15674/#comment57266> Do we have to keep two buildOffsetMap functions here? core/src/main/scala/kafka/log/LogManager.scala <https://reviews.apache.org/r/15674/#comment57269> Is there still a risk that after waitUntilInProgressFlusherIsDone passed another flushing procedure starts? - Guozhang Wang On Dec. 4, 2013, 6:15 p.m., Jun Rao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15674/ > ----------------------------------------------------------- > > (Updated Dec. 4, 2013, 6:15 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1074 > https://issues.apache.org/jira/browse/KAFKA-1074 > > > Repository: kafka > > > Description > ------- > > Summary of the new patch: > LogCleaner: > *added the logic to cancel an in-progress cleaning task > *made the cleaner thread a shutdownable, but not interruptible thread. > Interrupting the cleaner may cause the file channel to be closed, which will > fail other operations like log flushing during shutdown. > > LogManager: > * added the logic to wait until an in-progress flushing process complete > * to delete a log do > (1) take the log to be deleted from log lists so that log cleaner and log > flusher won't see it in the future > (2) cancel any in-progress cleaning task > (3) wait until any in-progress flushing process to complete > (4) at this moment, we know the log won't be touched by the cleaner or the > flusher any more and we can delete the whole directory synchronously. > > Todos: > * If the overall logic looks good, we can get rid of > OptimisticLockFailureException in LogCleaner too by canceling any cleaner > task before truncating the log. This can be done in a follow up jira. > > > Diffs > ----- > > core/src/main/scala/kafka/cluster/Partition.scala > 02ccc17c79b6d44c75f9bb6ca7cda8c51ae6f6fb > core/src/main/scala/kafka/common/AllDoneException.scala PRE-CREATION > core/src/main/scala/kafka/common/CancelledCleaningException.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/ReplicaManager.scala > f9c7c29bb96a63d6452269f46a12751daca15b62 > core/src/test/scala/unit/kafka/admin/AdminTest.scala > c30069e837e54fb91bf1d5b75b133282a28dedf8 > core/src/test/scala/unit/kafka/log/CleanerTest.scala > 5a312bf0803c1df4636e2e64ba83036a4e8e92dd > > Diff: https://reviews.apache.org/r/15674/diff/ > > > Testing > ------- > > > Thanks, > > Jun Rao > >
