You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Hari Shreedharan <hs...@cloudera.com> on 2012/12/07 00:34:57 UTC

Review Request: FLUME-1768. Multiplexing channel selector should allow optional-only channels

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

Review request for Flume.


Description
-------

Added support for an optional channels only configuration.


This addresses bug FLUME-1768.
    https://issues.apache.org/jira/browse/FLUME-1768


Diffs
-----

  flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java 866d9dc 
  flume-ng-core/src/test/java/org/apache/flume/channel/TestMultiplexingChannelSelector.java 9dff5bb 

Diff: https://reviews.apache.org/r/8387/diff/


Testing
-------

Added 2 unit tests to test various sets of configs.


Thanks,

Hari Shreedharan


Re: Review Request: FLUME-1768. Multiplexing channel selector should allow optional-only channels

Posted by Brock Noland <br...@cloudera.com>.

> On Dec. 7, 2012, 6:25 p.m., Brock Noland wrote:
> > Sorry, I found one more item. Looks like the user guide needs to be updated as well. For example I found this statement "The default must be set for a multiplexing select which can also contain any number of channels."

Tests passed though


- Brock


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


On Dec. 6, 2012, 11:34 p.m., Hari Shreedharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8387/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2012, 11:34 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Added support for an optional channels only configuration.
> 
> 
> This addresses bug FLUME-1768.
>     https://issues.apache.org/jira/browse/FLUME-1768
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java 866d9dc 
>   flume-ng-core/src/test/java/org/apache/flume/channel/TestMultiplexingChannelSelector.java 9dff5bb 
> 
> Diff: https://reviews.apache.org/r/8387/diff/
> 
> 
> Testing
> -------
> 
> Added 2 unit tests to test various sets of configs.
> 
> 
> Thanks,
> 
> Hari Shreedharan
> 
>


Re: Review Request: FLUME-1768. Multiplexing channel selector should allow optional-only channels

Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8387/#review14173
-----------------------------------------------------------


Sorry, I found one more item. Looks like the user guide needs to be updated as well. For example I found this statement "The default must be set for a multiplexing select which can also contain any number of channels."

- Brock Noland


On Dec. 6, 2012, 11:34 p.m., Hari Shreedharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8387/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2012, 11:34 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Added support for an optional channels only configuration.
> 
> 
> This addresses bug FLUME-1768.
>     https://issues.apache.org/jira/browse/FLUME-1768
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java 866d9dc 
>   flume-ng-core/src/test/java/org/apache/flume/channel/TestMultiplexingChannelSelector.java 9dff5bb 
> 
> Diff: https://reviews.apache.org/r/8387/diff/
> 
> 
> Testing
> -------
> 
> Added 2 unit tests to test various sets of configs.
> 
> 
> Thanks,
> 
> Hari Shreedharan
> 
>


Re: Review Request: FLUME-1768. Multiplexing channel selector should allow optional-only channels

Posted by Hari Shreedharan <hs...@cloudera.com>.

> On Dec. 7, 2012, 3:47 p.m., Brock Noland wrote:
> > Looks good!  One quick question to make sure I understand this correctly. If someone specifies no required channels the required channels list will just be empty and as such we'll only put the event on the optional channels. Similarly if there is no default channel and the event does not match a channel, the list returned will be empty?
> > 
> > That makes sense to me, I just wanted to confirm.

Yes, that is correct. It is not advised to not have a default, but it might be ok for users to drop events if none of the channels can accomodate it - so there is no back pressure on the previous tier. 


- Hari


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


On Dec. 6, 2012, 11:34 p.m., Hari Shreedharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8387/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2012, 11:34 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Added support for an optional channels only configuration.
> 
> 
> This addresses bug FLUME-1768.
>     https://issues.apache.org/jira/browse/FLUME-1768
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java 866d9dc 
>   flume-ng-core/src/test/java/org/apache/flume/channel/TestMultiplexingChannelSelector.java 9dff5bb 
> 
> Diff: https://reviews.apache.org/r/8387/diff/
> 
> 
> Testing
> -------
> 
> Added 2 unit tests to test various sets of configs.
> 
> 
> Thanks,
> 
> Hari Shreedharan
> 
>


Re: Review Request: FLUME-1768. Multiplexing channel selector should allow optional-only channels

Posted by Hari Shreedharan <hs...@cloudera.com>.

> On Dec. 7, 2012, 3:47 p.m., Brock Noland wrote:
> > Looks good!  One quick question to make sure I understand this correctly. If someone specifies no required channels the required channels list will just be empty and as such we'll only put the event on the optional channels. Similarly if there is no default channel and the event does not match a channel, the list returned will be empty?
> > 
> > That makes sense to me, I just wanted to confirm.
> 
> Hari Shreedharan wrote:
>     Yes, that is correct. It is not advised to not have a default, but it might be ok for users to drop events if none of the channels can accomodate it - so there is no back pressure on the previous tier.

Could you hold off just a bit before committing? 


- Hari


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


On Dec. 6, 2012, 11:34 p.m., Hari Shreedharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8387/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2012, 11:34 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Added support for an optional channels only configuration.
> 
> 
> This addresses bug FLUME-1768.
>     https://issues.apache.org/jira/browse/FLUME-1768
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java 866d9dc 
>   flume-ng-core/src/test/java/org/apache/flume/channel/TestMultiplexingChannelSelector.java 9dff5bb 
> 
> Diff: https://reviews.apache.org/r/8387/diff/
> 
> 
> Testing
> -------
> 
> Added 2 unit tests to test various sets of configs.
> 
> 
> Thanks,
> 
> Hari Shreedharan
> 
>


Re: Review Request: FLUME-1768. Multiplexing channel selector should allow optional-only channels

Posted by Hari Shreedharan <hs...@cloudera.com>.

> On Dec. 7, 2012, 3:47 p.m., Brock Noland wrote:
> > Looks good!  One quick question to make sure I understand this correctly. If someone specifies no required channels the required channels list will just be empty and as such we'll only put the event on the optional channels. Similarly if there is no default channel and the event does not match a channel, the list returned will be empty?
> > 
> > That makes sense to me, I just wanted to confirm.
> 
> Hari Shreedharan wrote:
>     Yes, that is correct. It is not advised to not have a default, but it might be ok for users to drop events if none of the channels can accomodate it - so there is no back pressure on the previous tier.
> 
> Hari Shreedharan wrote:
>     Could you hold off just a bit before committing?

Ok, we are good. Please go ahead and continue reviewing, sorry for the false alarm


- Hari


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


On Dec. 6, 2012, 11:34 p.m., Hari Shreedharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8387/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2012, 11:34 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Added support for an optional channels only configuration.
> 
> 
> This addresses bug FLUME-1768.
>     https://issues.apache.org/jira/browse/FLUME-1768
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java 866d9dc 
>   flume-ng-core/src/test/java/org/apache/flume/channel/TestMultiplexingChannelSelector.java 9dff5bb 
> 
> Diff: https://reviews.apache.org/r/8387/diff/
> 
> 
> Testing
> -------
> 
> Added 2 unit tests to test various sets of configs.
> 
> 
> Thanks,
> 
> Hari Shreedharan
> 
>


Re: Review Request: FLUME-1768. Multiplexing channel selector should allow optional-only channels

Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8387/#review14154
-----------------------------------------------------------


Looks good!  One quick question to make sure I understand this correctly. If someone specifies no required channels the required channels list will just be empty and as such we'll only put the event on the optional channels. Similarly if there is no default channel and the event does not match a channel, the list returned will be empty?

That makes sense to me, I just wanted to confirm.

- Brock Noland


On Dec. 6, 2012, 11:34 p.m., Hari Shreedharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8387/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2012, 11:34 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Added support for an optional channels only configuration.
> 
> 
> This addresses bug FLUME-1768.
>     https://issues.apache.org/jira/browse/FLUME-1768
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java 866d9dc 
>   flume-ng-core/src/test/java/org/apache/flume/channel/TestMultiplexingChannelSelector.java 9dff5bb 
> 
> Diff: https://reviews.apache.org/r/8387/diff/
> 
> 
> Testing
> -------
> 
> Added 2 unit tests to test various sets of configs.
> 
> 
> Thanks,
> 
> Hari Shreedharan
> 
>


Re: Review Request: FLUME-1768. Multiplexing channel selector should allow optional-only channels

Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8387/#review14177
-----------------------------------------------------------

Ship it!


Ship It!

- Brock Noland


On Dec. 7, 2012, 6:53 p.m., Hari Shreedharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8387/
> -----------------------------------------------------------
> 
> (Updated Dec. 7, 2012, 6:53 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Added support for an optional channels only configuration.
> 
> 
> This addresses bug FLUME-1768.
>     https://issues.apache.org/jira/browse/FLUME-1768
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java 866d9dc 
>   flume-ng-core/src/test/java/org/apache/flume/channel/TestMultiplexingChannelSelector.java 9dff5bb 
>   flume-ng-doc/sphinx/FlumeUserGuide.rst f8528bb 
> 
> Diff: https://reviews.apache.org/r/8387/diff/
> 
> 
> Testing
> -------
> 
> Added 2 unit tests to test various sets of configs.
> 
> 
> Thanks,
> 
> Hari Shreedharan
> 
>


Re: Review Request: FLUME-1768. Multiplexing channel selector should allow optional-only channels

Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8387/
-----------------------------------------------------------

(Updated Dec. 7, 2012, 6:53 p.m.)


Review request for Flume.


Changes
-------

Update user guide.


Description
-------

Added support for an optional channels only configuration.


This addresses bug FLUME-1768.
    https://issues.apache.org/jira/browse/FLUME-1768


Diffs (updated)
-----

  flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java 866d9dc 
  flume-ng-core/src/test/java/org/apache/flume/channel/TestMultiplexingChannelSelector.java 9dff5bb 
  flume-ng-doc/sphinx/FlumeUserGuide.rst f8528bb 

Diff: https://reviews.apache.org/r/8387/diff/


Testing
-------

Added 2 unit tests to test various sets of configs.


Thanks,

Hari Shreedharan