----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6719/#review10590 -----------------------------------------------------------
Looks very good. One minor suggestion - more of a nit. flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java <https://reviews.apache.org/r/6719/#comment22675> The only reason I put this call in this method was because a checkpoint should not happen in between the log.put and queue.addWithoutCommit calls. Now that this is guaranteed to not happen, it might be a good idea to move this call to the transaction.put call, right? - Hari Shreedharan On Aug. 21, 2012, 4:11 p.m., Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6719/ > ----------------------------------------------------------- > > (Updated Aug. 21, 2012, 4:11 p.m.) > > > Review request for Flume and Hari Shreedharan. > > > Description > ------- > > The data structure we implement in FileChannel requires consistent > checkpoints, in that It's very similar to a LGSM. This change moves the > shared/exclusive lock in Log up to the transaction level. Every transaction > operation will get a shared lock on the log before doing any changes to the > log or queue. Checkpoints when they occur obtain an exclusive lock. > > This change introduces four methods to the Log class: > > tryLockShared > tryLockExclusive > unlockShared > unlockExclusive > > Calls to obtain the shared lock have been moved up to the > FileBackedTransaction and use these newly created methods. > > > This addresses bug FLUME-1498. > https://issues.apache.org/jira/browse/FLUME-1498 > > > Diffs > ----- > > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java > b5a0b88 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java > c356ca4 > > Diff: https://reviews.apache.org/r/6719/diff/ > > > Testing > ------- > > Without this patch FileChannel.testRestartLogReplayV2 fails once in awhile > because of lost data. With this patch the test has run several hundred times > without failure. All FileChannel unit tests have been run a dozen times > without failure. > > > Thanks, > > Brock Noland > >
