incubator-flume-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Prasad Mujumdar" <pras...@cloudera.com>
Subject Re: Review Request: Adding channel processor and channel selector mechanism.
Date Wed, 01 Feb 2012 01:30:36 GMT

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

Ship it!


The approach and code changes look fine to me. Got some high level comments as listed below.
Though none of those are blocker and can be tracked via separate tickets.
1) A default channel for multiplexer -
With the current implementation, if a given event doesn't qualify any mapping, then it will
get thrown away. It would be very useful to provide away to designate a channel as 'default'
for such unqualified event.
2) required vs. optional channel -
Given that there's not 2pc support, I would suggest to treat all channels as 'optional', i.e.
continue processing all the qualified channels even if any one fails. They way this implementation
is treating the 'required' channels make the error handling deterministic. If you have 5 required
channels and the 3rd put fails then the caller has no way to figure what needs to be retried.
Given that we can't undo the previous transactions, aborting the remaining work is not very
helpful.
Let me know what you think.
3) load balancing via multiplexer -
A load balancing selector that rotates channels round-robin would be helpful. But I guess
that can be implemented separately using this framework ...



- Prasad


On 2012-01-28 19:07:21, Arvind Prabhakar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3688/
> -----------------------------------------------------------
> 
> (Updated 2012-01-28 19:07:21)
> 
> 
> Review request for Flume and Prasad Mujumdar.
> 
> 
> Summary
> -------
> 
> Previously source was directly configured with a set of channels. This has changed now
so that the source is configured with a channel processor, which in turn is configured with
a single channel selector. A channel selector is the component that is responsible for selecting
the specific required and optional channels when an event is received by the source. Using
configuration the channel selector can be specified using the sub-namespace of "selector".
Properties within this namespace are used to configure the selector itself.
> 
> By default, when no selector is explicitly specified in the configuration, the default
selector is used - which is the ReplicatingChannelSelector. As the name suggests, the replicating
channel selector ensures that the event is replicated on all channels. An alternate channel
selector is introduced as well - called the MultiplexingChannelSelector - which allows a mapping
of pre-specified header value to a subset of channels from within the source channels. This
selector uses static header values for mapping and does not support any regular-expression
syntax. 
> 
> 
> This addresses bug FLUME-930.
>     https://issues.apache.org/jira/browse/FLUME-930
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/ChannelSelector.java PRE-CREATION 
>   flume-ng-core/src/main/java/org/apache/flume/Source.java 3d6f81d 
>   flume-ng-core/src/main/java/org/apache/flume/channel/AbstractChannelSelector.java PRE-CREATION

>   flume-ng-core/src/main/java/org/apache/flume/channel/ChannelProcessor.java PRE-CREATION

>   flume-ng-core/src/main/java/org/apache/flume/channel/ChannelSelectorFactory.java PRE-CREATION

>   flume-ng-core/src/main/java/org/apache/flume/channel/ChannelSelectorType.java PRE-CREATION

>   flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java
PRE-CREATION 
>   flume-ng-core/src/main/java/org/apache/flume/channel/ReplicatingChannelSelector.java
PRE-CREATION 
>   flume-ng-core/src/main/java/org/apache/flume/source/AbstractSource.java dd76871 
>   flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java b1ca078 
>   flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java 71608b6 
>   flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java b01ef29 
>   flume-ng-core/src/main/java/org/apache/flume/source/SequenceGeneratorSource.java e90f17f

>   flume-ng-core/src/test/java/org/apache/flume/channel/MockChannel.java PRE-CREATION

>   flume-ng-core/src/test/java/org/apache/flume/channel/MockEvent.java PRE-CREATION 
>   flume-ng-core/src/test/java/org/apache/flume/channel/TestMultiplexingChannelSelector.java
PRE-CREATION 
>   flume-ng-core/src/test/java/org/apache/flume/channel/TestReplicatingChannelSelector.java
PRE-CREATION 
>   flume-ng-core/src/test/java/org/apache/flume/source/MockSource.java 04d3cef 
>   flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java 7ffd1f6 
>   flume-ng-core/src/test/java/org/apache/flume/source/TestExecSource.java 6acbbd5 
>   flume-ng-core/src/test/java/org/apache/flume/source/TestPollableSourceRunner.java 5ff570e

>   flume-ng-core/src/test/java/org/apache/flume/source/TestSequenceGeneratorSource.java
a15f9f1 
>   flume-ng-node/src/main/java/org/apache/flume/conf/file/JsonFileConfigurationProvider.java
f48e681 
>   flume-ng-node/src/main/java/org/apache/flume/conf/properties/FlumeConfiguration.java
57fff8c 
>   flume-ng-node/src/main/java/org/apache/flume/conf/properties/PropertiesFileConfigurationProvider.java
bee60ff 
>   flume-ng-node/src/test/java/org/apache/flume/node/TestAbstractLogicalNodeManager.java
32586e0 
>   flume-ng-node/src/test/java/org/apache/flume/node/TestDefaultLogicalNodeManager.java
bc3058c 
>   flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java 8ccffca 
> 
> Diff: https://reviews.apache.org/r/3688/diff
> 
> 
> Testing
> -------
> 
> All unit tests pass. Introduced new tests to exercise channel selector functionality.
> 
> 
> Thanks,
> 
> Arvind
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message