HI Juhani, Thanks for the patch. I am looking through the patch as I get time, since it is quite huge. Also, I am not too familiar with the Guava lifecycle classes. I will look through it and let you know soon :-)
Thanks Hari -- Hari Shreedharan On Sunday, June 24, 2012 at 6:48 PM, Juhani Connolly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5417/#review8524 > ----------------------------------------------------------- > > > Pinging for someone to have a look at this. At this stage I just want some > general opinions so a detailed review isn't necessary. > > Places to look for major changes are lifecycle classes and the > node/configuration. > > - Juhani Connolly > > > On June 19, 2012, 9:32 a.m., Juhani Connolly wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/5417/ > > ----------------------------------------------------------- > > > > (Updated June 19, 2012, 9:32 a.m.) > > > > > > Review request for Flume. > > > > > > Description > > ------- > > > > Early refactoring of the lifecycle code as Guava Services > > > > General overview: > > > > - All components are created as instances of FlumeComponent which extends > > guava Service > > - Components are free to choose what service they implement though in > > general I have used IdleAbstractService for anything that doesn't need its > > own thread and AbstractExecutionThreadService for active services(like > > sink/source runners) > > - FlumeComponents are created by ComponentSupplier which provides some kind > > of factory method. Generally this will also store the configuration. > > - The supervisor model watches the state of items created from the supplier > > and generates/starts fresh items from the component supplier. > > > > Some places the refactorings feel a bit forced/awkward. Better solutions > > welcome. > > > > > > This addresses bug FLUME-966. > > https://issues.apache.org/jira/browse/FLUME-966 > > > > > > Diffs > > ----- > > > > conf/log4j.properties 2c476b3 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java > > 741f9f4 > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java > > cd59be4 > > flume-ng-channels/flume-jdbc-channel/src/main/java/org/apache/flume/channel/jdbc/JdbcChannel.java > > bca0c50 > > flume-ng-channels/flume-recoverable-memory-channel/src/main/java/org/apache/flume/channel/recoverable/memory/RecoverableMemoryChannel.java > > 18682ec > > flume-ng-channels/flume-recoverable-memory-channel/src/test/java/org/apache/flume/channel/recoverable/memory/TestRecoverableMemoryChannel.java > > 6e0ec2b > > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java > > 68d95fb > > flume-ng-core/src/main/java/org/apache/flume/Channel.java 91ea7b6 > > flume-ng-core/src/main/java/org/apache/flume/Sink.java 2567140 > > flume-ng-core/src/main/java/org/apache/flume/SinkProcessor.java c0c7c29 > > flume-ng-core/src/main/java/org/apache/flume/SinkRunner.java c353d1f > > flume-ng-core/src/main/java/org/apache/flume/Source.java 4697126 > > flume-ng-core/src/main/java/org/apache/flume/SourceRunner.java 3246151 > > flume-ng-core/src/main/java/org/apache/flume/channel/AbstractChannel.java > > 352bf08 > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/AgentLifecycleSupervisor.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/ComponentSupplier.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/ConfiguredChannelBuilder.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/ConfiguredSinkBuilder.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/ConfiguredSinkRunnerBuilder.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/ConfiguredSourceRunnerBuilder.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/ContextConfiguredChannelBuilder.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/ContextConfiguredSinkBuilder.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/ContextConfiguredSourceRunnerBuilder.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/DefaultSinkRunnerBuilder.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/FlumeComponent.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/FlumeComponentSupervisor.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/FlumeServicePolicy.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/LifecycleAware.java > > 0fb82fb > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/LifecycleController.java > > cc1a712 > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/LifecycleException.java > > 77f4897 > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/LifecycleState.java > > e64dec5 > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/LifecycleSupervisor.java > > 2935e64 > > flume-ng-core/src/main/java/org/apache/flume/sink/AbstractSink.java 7ecc1c9 > > flume-ng-core/src/main/java/org/apache/flume/sink/AbstractSinkProcessor.java > > 528a309 > > flume-ng-core/src/main/java/org/apache/flume/sink/AbstractSinkSelector.java > > 63397a5 > > flume-ng-core/src/main/java/org/apache/flume/sink/AvroSink.java 80b1d27 > > flume-ng-core/src/main/java/org/apache/flume/sink/DefaultSinkProcessor.java > > 00a362b > > flume-ng-core/src/main/java/org/apache/flume/sink/FailoverSinkProcessor.java > > 3bd52f2 > > flume-ng-core/src/main/java/org/apache/flume/sink/LoadBalancingSinkProcessor.java > > 18d4509 > > flume-ng-core/src/main/java/org/apache/flume/sink/NullSink.java c812851 > > flume-ng-core/src/main/java/org/apache/flume/sink/RollingFileSink.java > > e5e97ff > > flume-ng-core/src/main/java/org/apache/flume/source/AbstractSource.java > > 5eeb687 > > flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java 1d68d87 > > flume-ng-core/src/main/java/org/apache/flume/source/EventDrivenSourceRunner.java > > 92eb97c > > flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java dbf79e0 > > flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java > > 9d28cda > > flume-ng-core/src/main/java/org/apache/flume/source/PollableSourceRunner.java > > d8ef87b > > flume-ng-core/src/main/java/org/apache/flume/source/SequenceGeneratorSource.java > > 440c5a9 > > flume-ng-core/src/main/java/org/apache/flume/source/SourceRunnerFactory.java > > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/source/StressSource.java > > 4f7b255 > > flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java > > db9e0fd > > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java > > 96a9e85 > > flume-ng-core/src/test/java/org/apache/flume/lifecycle/TestLifecycleController.java > > cb9a070 > > flume-ng-core/src/test/java/org/apache/flume/lifecycle/TestLifecycleSupervisor.java > > 2355ae9 > > flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java 3765924 > > flume-ng-core/src/test/java/org/apache/flume/sink/TestFailoverSinkProcessor.java > > 3358cf4 > > flume-ng-core/src/test/java/org/apache/flume/sink/TestLoggerSink.java > > 92ff6fe > > flume-ng-core/src/test/java/org/apache/flume/sink/TestRollingFileSink.java > > 10c9b82 > > flume-ng-core/src/test/java/org/apache/flume/source/MockSource.java 0d9798e > > flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java > > 4bf36e6 > > flume-ng-core/src/test/java/org/apache/flume/source/TestExecSource.java > > 615f2a3 > > flume-ng-core/src/test/java/org/apache/flume/source/TestPollableSourceRunner.java > > 4d4222d > > flume-ng-core/src/test/java/org/apache/flume/source/TestSequenceGeneratorSource.java > > 579b257 > > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUdpSource.java > > 3a7c486 > > flume-ng-legacy-sources/flume-avro-source/src/main/java/org/apache/flume/source/avroLegacy/AvroLegacySource.java > > dde8f28 > > flume-ng-legacy-sources/flume-avro-source/src/test/java/org/apache/flume/source/avroLegacy/TestLegacyAvroSource.java > > 6e3eb53 > > flume-ng-legacy-sources/flume-thrift-source/src/main/java/org/apache/flume/source/thriftLegacy/ThriftLegacySource.java > > 5b056fc > > flume-ng-legacy-sources/flume-thrift-source/src/test/java/org/apache/flume/source/thriftLegacy/TestThriftLegacySource.java > > 4869665 > > flume-ng-node/src/main/java/org/apache/flume/agent/Application.java > > PRE-CREATION > > flume-ng-node/src/main/java/org/apache/flume/conf/file/AbstractFileConfigurationProvider.java > > 15ee8ad > > flume-ng-node/src/main/java/org/apache/flume/conf/file/SimpleNodeConfiguration.java > > 99b8bcc > > flume-ng-node/src/main/java/org/apache/flume/conf/properties/PropertiesFileConfigurationProvider.java > > 8dbbe57 > > flume-ng-node/src/main/java/org/apache/flume/node/Application.java 405afa3 > > flume-ng-node/src/main/java/org/apache/flume/node/ConfigurationProvider.java > > ae732aa > > flume-ng-node/src/main/java/org/apache/flume/node/FlumeNode.java cee022d > > flume-ng-node/src/main/java/org/apache/flume/node/NodeConfiguration.java > > a24c939 > > flume-ng-node/src/main/java/org/apache/flume/node/NodeManager.java 7457d87 > > flume-ng-node/src/main/java/org/apache/flume/node/nodemanager/AbstractLogicalNodeManager.java > > 1fda07b > > flume-ng-node/src/main/java/org/apache/flume/node/nodemanager/DefaultLogicalNodeManager.java > > 7403dd3 > > flume-ng-node/src/test/java/org/apache/flume/node/TestAbstractLogicalNodeManager.java > > 521b586 > > flume-ng-node/src/test/java/org/apache/flume/node/TestDefaultLogicalNodeManager.java > > 530b299 > > flume-ng-node/src/test/java/org/apache/flume/node/TestFlumeNode.java > > f2dad6f > > flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java > > c195db7 > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java > > fc06754 > > flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/TestHDFSEventSink.java > > b5f8c88 > > flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/TestHDFSEventSinkOnMiniCluster.java > > 4d16d6e > > flume-ng-sinks/flume-irc-sink/src/main/java/org/apache/flume/sink/irc/IRCSink.java > > 8e77218 > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java > > d08d7a7 > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java > > 75682c7 > > > > Diff: https://reviews.apache.org/r/5417/diff/ > > > > > > Testing > > ------- > > > > All tests pass, though some were removed(e.g. the old LifecycleSupervisor). > > > > Will need to add more tests. > > > > > > Thanks, > > > > Juhani Connolly
