You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Mike Percy <mp...@gmail.com> on 2012/06/23 11:12:59 UTC
Review Request: FLUME-944: Implement load balancing channel selector
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5537/
-----------------------------------------------------------
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
Re: Review Request: FLUME-944: Implement load balancing channel selector
Posted by Ralph Goers <rg...@apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request: FLUME-944: Implement load balancing channel selector
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5537/#review8507
-----------------------------------------------------------
Mike, thanks for the patch! Looks good, just one issue - which is not really a amjor one or anything, just one which can affect multithreaded sources writing to the same channel from different threads.
Also, please add more documentation.
flume-ng-core/src/main/java/org/apache/flume/channel/LoadBalancingChannelSelector.java
<https://reviews.apache.org/r/5537/#comment18173>
next is a variable that could be modified by multiple threads right(like if the source has several threads etc - I don't think any of the built in sources actually write to the channel from multiple threads though, so might not be an immediate concern)? It probably still should be volatile
flume-ng-core/src/main/java/org/apache/flume/channel/LoadBalancingChannelSelector.java
<https://reviews.apache.org/r/5537/#comment18174>
If none of the channels are capacity aware, then this will cause the flow to always go to one channel. There is no "correct" solution, since this is probably due to a bad config, but we should log it and also clearly mention in the docs that this selector works properly only if the channels implement capacity aware.
- Hari Shreedharan
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
>
>