> 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 ... > > > > > > Arvind Prabhakar wrote: > 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 Prabhakar wrote: > Created the following issues: > https://issues.apache.org/jira/browse/FLUME-944 > https://issues.apache.org/jira/browse/FLUME-945
Also created an issue for alternate implementation to address #2: https://issues.apache.org/jira/browse/FLUME-946 - 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 > >
