> On 2012-02-01 01:30:36, Prasad Mujumdar wrote: > > 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 ... > > > >
Thanks for the review Prasad. I will open a JIRA for 1 and 3. For point #2, I am in the other camp. Unless explicitly set, every channel is a required channel. Hence a failure to publish an event to a particular channel is the same as failure of the whole operation. As such, it does not matter if the remaining channels are tried or not, the event will likely be sent to the agent again by the upstream sink. - Arvind ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3688/#review4731 ----------------------------------------------------------- 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 > >
