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


Generally looks good, but I am confused about the configuration code.


flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
<https://reviews.apache.org/r/8379/#comment30405>

    Why do this when you are taking the max of this value and 
FileChannelConfiguration.FLOOR_MINIMUM_REQUIRED_SPACE later? Did you want to do 
a Math.max here?



flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java
<https://reviews.apache.org/r/8379/#comment30404>

    Looks like you are checking for min of the configured value and 
FileChannelConfiguration.FLOOR_MINIMUM_REQUIRED_SPACE in FileChannel.java and 
you are checking the max of that value and 
FileChannelConfiguration.FLOOR_MINIMUM_REQUIRED_SPACE. Doing both these 
together means that minimumRequiredSpace is always equal to 
FileChannelConfiguration.FLOOR_MINIMUM_REQUIRED_SPACE. Am I missing something 
here?


- Hari Shreedharan


On Dec. 6, 2012, 10:03 p.m., Brock Noland wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8379/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2012, 10:03 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Adds a parameter which can be configured by defaults to 500MB. We will refuse 
> to checkpoint or perform log writes in this case.
> 
> 
> This addresses bug FLUME-1763.
>     https://issues.apache.org/jira/browse/FLUME-1763
> 
> 
> Diffs
> -----
> 
>   
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
>  64a1350 
>   
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannelConfiguration.java
>  92cad77 
>   
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java
>  6d1cf51 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLog.java
>  a165d6a 
>   flume-ng-doc/sphinx/FlumeUserGuide.rst f8528bb 
> 
> Diff: https://reviews.apache.org/r/8379/diff/
> 
> 
> Testing
> -------
> 
> Two unit tests added.
> 
> 
> Thanks,
> 
> Brock Noland
> 
>

Reply via email to