----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30330/#review70466 -----------------------------------------------------------
This looks almost good to go. I posted one comment which is pretty minor. Can you also add a unit test to ensure that a checkpoint gets written when you call close? flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java <https://reviews.apache.org/r/30330/#comment115673> Since the lock is re-entrant, it is fine to acquire the exclusive lock at this point. I also think you should set open=false before writing the checkpoint to ensure that no write ops happen after (it is fine if it does, there would only be a small replay). Also not locking exclusively can cause a checkpoint to get written, followed by a scheduled checkpoint - which is pretty useless. - Hari Shreedharan On Jan. 27, 2015, 10 p.m., Roshan Naik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30330/ > ----------------------------------------------------------- > > (Updated Jan. 27, 2015, 10 p.m.) > > > Review request for Flume. > > > Bugs: FLUME-2595 > https://issues.apache.org/jira/browse/FLUME-2595 > > > Repository: flume-git > > > Description > ------- > > FC to auto checkpoint on shutdown. Added an option to disable it if needed. > updated docs. > > > Diffs > ----- > > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java > 61c353a > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannelConfiguration.java > f8c0378 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java > 0e9171e > flume-ng-doc/sphinx/FlumeUserGuide.rst bcadc2d > > Diff: https://reviews.apache.org/r/30330/diff/ > > > Testing > ------- > > verified manually with the option enabled & disabled > > > Thanks, > > Roshan Naik > >
