> On April 17, 2015, 10:44 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/Log.scala, lines 775-777 > > <https://reviews.apache.org/r/33168/diff/2/?file=929633#file929633line775> > > > > It's not clear what "this operation" is. It seems to refer to > > replaceSegments(), bit replaceSegments() doesn't create new suffix > > .cleaned. So, it probably should refer to the cleaning process.
Replaced "this" with clean-and-swap, which is the operation being aborted. > On April 17, 2015, 10:44 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/Log.scala, lines 806-807 > > <https://reviews.apache.org/r/33168/diff/2/?file=929633#file929633line806> > > > > What happens if the broker crashes just before the swapping in line > > 805. Then, when calling replaceSegments() during broker restart, you will > > have old segments already with suffix .delete. In asyncDeleteSegment(), we > > will then try to rename the file by adding another .delete suffix. The > > deletion will probaby still work, but this will be a bit wierd. oldSegments during recovery only contains segments which were not renamed before the crash. So this scenario would not occur. .deleted files are deleted before replaceSegments is invoked during recovery. Have updated comment to reflect this. - Rajini ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33168/#review80543 ----------------------------------------------------------- On April 19, 2015, 7:03 p.m., Rajini Sivaram wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33168/ > ----------------------------------------------------------- > > (Updated April 19, 2015, 7:03 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2118 > https://issues.apache.org/jira/browse/KAFKA-2118 > > > Repository: kafka > > > Description > ------- > > Patch for KAFKA-2118: Fix recovery of cleaned segments after broker crash > Improve comment > > > Diffs > ----- > > core/src/main/scala/kafka/log/Log.scala > 5563f2de8113a0ece8929bec9c75dbf892abbb66 > core/src/test/scala/unit/kafka/log/CleanerTest.scala > 9792ed689033dbd4ad99809a4e566136d2b9fadf > > Diff: https://reviews.apache.org/r/33168/diff/ > > > Testing > ------- > > > Thanks, > > Rajini Sivaram > >
