----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29755/#review69529 -----------------------------------------------------------
core/src/main/scala/kafka/log/Log.scala <https://reviews.apache.org/r/29755/#comment114225> Seems like in the case of the exception we actually forcefully delete, but then after deleting schedule another one. Should the scheduler action be inside the try block so it only occurs if the rename succeeds? - Jay Kreps On Jan. 24, 2015, 5:18 a.m., Jaikiran Pai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29755/ > ----------------------------------------------------------- > > (Updated Jan. 24, 2015, 5:18 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1853 > https://issues.apache.org/jira/browse/KAFKA-1853 > > > Repository: kafka > > > Description > ------- > > KAFKA-1853 Prevent leaking open file resources when renaming of the > LogSegment fails > > > Diffs > ----- > > core/src/main/scala/kafka/log/FileMessageSet.scala > b2652ddbe2f857028d5980e29a008b2c614694a3 > core/src/main/scala/kafka/log/Log.scala > 846023bb98d0fa0603016466360c97071ac935ea > core/src/main/scala/kafka/log/OffsetIndex.scala > 1c4c7bd89e19ea942cf1d01eafe502129e97f535 > > Diff: https://reviews.apache.org/r/29755/diff/ > > > Testing > ------- > > Have run the existing tests in LogManagerTest (which includes a test for > cleaning of expired LogSegments) and those have passed with this change. I > did give a thought of trying to replicate a failed rename scenario and then > to ensure that we don't leak resources anymore, but that's not > straightforward to do in the tests, so haven't added any new tests. > > > Thanks, > > Jaikiran Pai > >
