|
|
-
Re: Review Request: FLUME-1761: FileChannel can NPE when log metadata file is emptyHari Shreedharan 2012-12-11, 06:29
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8414/#review14286 ----------------------------------------------------------- Generally looks good. I think one test needs some change as noted below. flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLog.java <https://reviews.apache.org/r/8414/#comment30436> The log.replay() call here does not replay anything since there is nothing in the logs at this point. I think we need to pass a param to the doTestReplayFailsWithAllEmptyLogMetaData() method, and based on that call setUseFastReplay(true) on the builder (and delete the checkpoint files) - otherwise this test is not really using fast replay - is it? - Hari Shreedharan On Dec. 7, 2012, 6:15 p.m., Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8414/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2012, 6:15 p.m.) > > > Review request for Flume. > > > Description > ------- > > 1) Adds better error messages if for any reason Commit,Take,Put,Rollback encountered an null pointer. Shouldn't happen. > 2) Fast replay handles the case where we have an empty log or metadata file. > 3) LogFileFactory throws an informative error message when the meta file is empty but the log not. > 4) LogFileFactory throws an EOF when log file and meta are empty sot he log can be ignored during replay. > > > This addresses bug FLUME-1761. > https://issues.apache.org/jira/browse/FLUME-1761 > > > Diffs > ----- > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/CheckpointRebuilder.java 748f49a > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Commit.java 62f4451 > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java 4783448 > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java b4c197e > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Put.java d47b1c8 > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Rollback.java cc9ce86 > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Take.java e61bf7e > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/TransactionEventRecord.java 70098a0 > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLog.java a165d6a > > Diff: https://reviews.apache.org/r/8414/diff/ > > > Testing > ------- > > Added tests which test 2, 3, 4. The behavior is the same for 1 we just have a better error message. > > > Thanks, > > Brock Noland > > |