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


In LoadBalancingChannelSelector why does ChannelPicker have a setChannels 
method?  As a general rule it would be much better to require the channels to 
be passed on the constructor and then have the List be final in each 
implementation as this helps guarantee thread safety, plus the channels are 
never modified after the Policy is in use so this documents them as being 
immutable.  As was already pointed out, next should be volatile.

- Ralph Goers


On June 23, 2012, 9:12 a.m., Mike Percy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5537/
> -----------------------------------------------------------
> 
> (Updated June 23, 2012, 9:12 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Implement load balancing channel selector.
> 
> 
> This addresses bug FLUME-944.
>     https://issues.apache.org/jira/browse/FLUME-944
> 
> 
> Diffs
> -----
> 
>   
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
>  03fbd19 
>   flume-ng-core/src/main/java/org/apache/flume/channel/CapacityAware.java 
> PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/channel/LoadBalancingChannelSelector.java
>  PRE-CREATION 
>   flume-ng-core/src/main/java/org/apache/flume/channel/MemoryChannel.java 
> 8c33f35 
>   
> flume-ng-core/src/test/java/org/apache/flume/channel/TestLoadBalancingChannelSelector.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/5537/diff/
> 
> 
> Testing
> -------
> 
> Added 3 unit tests. All unit tests pass.
> 
> 
> Thanks,
> 
> Mike Percy
> 
>

Reply via email to