-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8414/
-----------------------------------------------------------

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

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8414/
-----------------------------------------------------------

(Updated Dec. 11, 2012, 4:38 p.m.)
Review request for Flume.
Changes
-------

Fixed test
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 (updated)
-----

  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

-----------------------------------------------------------
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
>
>

> On Dec. 11, 2012, 6:29 a.m., Hari Shreedharan wrote:
> > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLog.java, lines 287-288
> > <https://reviews.apache.org/r/8414/diff/2/?file=235796#file235796line287>
> >
> >     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?

Good catch!
- Brock
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8414/#review14286
-----------------------------------------------------------
On Dec. 11, 2012, 4:38 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8414/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2012, 4:38 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
>
>