Flume, mail # dev - Re: Review Request: FLUME-1768. Multiplexing channel selector should allow optional-only channels

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

Hari Shreedharan 2012-12-06, 23:34

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

Brock Noland 2012-12-07, 15:47

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

Hari Shreedharan 2012-12-07, 17:31

> 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

Hari Shreedharan 2012-12-07, 17:34

> 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

Hari Shreedharan 2012-12-07, 17:35

> 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

Brock Noland 2012-12-07, 18:25

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

Brock Noland 2012-12-07, 18:37

> 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

Hari Shreedharan 2012-12-07, 18:53

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

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

Brock Noland 2012-12-07, 19:44

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