----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4713/#review7406 -----------------------------------------------------------
Hi Brock, Thanks for the patch! This is not really a review: I just have one feedback. Could you please not include the cleanup in this patch and submit a separate patch for that. The revision history will show this patch affecting every single file even though this is supposed to only fix the recoverable memory channel, and we will need to look at detailed revision history to look at what changes were caused by this patch. I'd rather see only the changes related to this bug. - Hari On 2012-04-13 20:12:25, Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4713/ > ----------------------------------------------------------- > > (Updated 2012-04-13 20:12:25) > > > Review request for Flume. > > > Summary > ------- > > Channel.start() is not being called. This is fixed in > DefaultLogicalNodeManager. > > Additionally RecoverableMemoryChannel now tracks it's own capacity due to the > MemoryChannel semantics being completely different. Basically, if we rely on > MemoryChannel capacity, then an error will be thrown when we commit the > MemoryChannelTransaction. However, we will have already committed this data > to disk. If we commit to MemoryChannel first (there by checking capacity) we > could fail to write to disk resulting in data which is only in memory. > > Also, I ran cleanup on modules touched. This removes whitespace, unused > imports, and adds @Override tags where needed. This is one time cleanup which > allows automated cleanup in the future. > > > This addresses bug FLUME-1121. > https://issues.apache.org/jira/browse/FLUME-1121 > > > Diffs > ----- > > flume-ng-core/src/test/java/org/apache/flume/channel/TestChannelUtils.java > 1421449 > > flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannelConcurrency.java > 8dad0b2 > > flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannelTransaction.java > bc81f26 > > flume-ng-node/src/main/java/org/apache/flume/conf/properties/FlumeConfiguration.java > d66f6d1 > > flume-ng-node/src/main/java/org/apache/flume/conf/properties/PropertiesFileConfigurationProvider.java > 1f0e8c6 > > flume-ng-core/src/test/java/org/apache/flume/channel/TestBasicChannelSemantics.java > 80020fc > > flume-ng-core/src/test/java/org/apache/flume/channel/AbstractBasicChannelSemanticsTest.java > 6e71e46 > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java > 732cce5 > flume-ng-core/src/main/java/org/apache/flume/sink/AvroSink.java ca5212e > flume-ng-core/src/main/java/org/apache/flume/sink/SinkType.java 6b08c09 > flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java > b0485b1 > > flume-ng-channels/flume-recoverable-memory-channel/src/main/java/org/apache/flume/channel/recoverable/memory/wal/WALDataFile.java > 9d4a1fd > > flume-ng-channels/flume-recoverable-memory-channel/src/test/java/org/apache/flume/channel/recoverable/memory/TestRecoverableMemoryChannel.java > edd8a8b > flume-ng-core/src/main/java/org/apache/flume/SinkProcessorType.java be1891b > flume-ng-core/src/main/java/org/apache/flume/conf/Configurables.java > 84492e5 > > flume-ng-core/src/main/java/org/apache/flume/formatter/output/BucketPath.java > 4722819 > flume-ng-core/src/main/java/org/apache/flume/lifecycle/LifecycleAware.java > f179de0 > flume-ng-core/src/main/java/org/apache/flume/sink/AbstractSink.java 2334059 > > flume-ng-channels/flume-recoverable-memory-channel/src/main/java/org/apache/flume/channel/recoverable/memory/wal/WAL.java > 97ef796 > > flume-ng-channels/flume-recoverable-memory-channel/src/main/java/org/apache/flume/channel/recoverable/memory/wal/SequenceIDBuffer.java > fa63b73 > > flume-ng-channels/flume-recoverable-memory-channel/src/main/java/org/apache/flume/channel/recoverable/memory/RecoverableMemoryChannel.java > 0622f27 > > flume-ng-node/src/main/java/org/apache/flume/node/nodemanager/DefaultLogicalNodeManager.java > 07c3d0b > flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java > 225cd34 > flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java > 0b8a2c0 > > Diff: https://reviews.apache.org/r/4713/diff > > > Testing > ------- > > All unit tests pass and manual testing passes as well. > > > Thanks, > > Brock > >
