----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5378/#review8353 -----------------------------------------------------------
Leslin, Patch looks good. A few nits: 1. This patch just logs a warning (which is easy to miss) when codeC is set while fileType = DataStream. Please throw an IllegalArgumentException instead. 2. The part of the message saying "To change fileType if want output compressed" is a little unclear. Consider using wording like "Please change the fileType if compressed output is desired" 3. Please remove the extraneous whitespace at the end of the lines in the patch. This shows up as red highlighting on the review board. Thanks! Mike - Mike Percy On June 18, 2012, 2:04 p.m., Leslin (Hong Xiang Lin) wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5378/ > ----------------------------------------------------------- > > (Updated June 18, 2012, 2:04 p.m.) > > > Review request for Flume. > > > Description > ------- > > 1). if user set hdfs.codeC when hdfs.fileType = DataStream, sink will use > DataStream to output file, which is no compress extension like .snappy. > Warning message is added to show the codeC will be ignored. > 2). Pre-check will make sure that codec is required when fileType is set > CompressedStream. > > > This addresses bug FLUME-1200. > https://issues.apache.org/jira/browse/FLUME-1200 > > > Diffs > ----- > > flume-ng-doc/sphinx/FlumeUserGuide.rst e61443b > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java > fc06754 > > Diff: https://reviews.apache.org/r/5378/diff/ > > > Testing > ------- > > 1. compressStream without codec --> there will be exception > agent.sinks.k1.hdfs.fileType = CompressedStream > #agent.sinks.k1.hdfs.codeC = DefaultCodec > 12/06/17 22:46:35 INFO sink.DefaultSinkFactory: Creating instance of sink k1 > typeHDFS > 12/06/17 22:46:35 ERROR properties.PropertiesFileConfigurationProvider: > Failed to load configuration data. Exception follows. > java.lang.NullPointerException: It's essential to set compress codec when > fileType is: CompressedStream > at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:204) > at org.apache.flume.sink.hdfs.HDFSEventSink.configure(HDFSEventSink.java:221) > 2. Works fine, output file with .deflate extension. > agent.sinks.k1.hdfs.fileType = CompressedStream > agent.sinks.k1.hdfs.codeC = DefaultCodec > 3. Works fine, output file without compress extension. > agent.sinks.k1.hdfs.fileType = CompressedStream > agent.sinks.k1.hdfs.codeC = snappyCodec > 4. There is warning, output file without compress extension. > agent.sinks.k1.hdfs.fileType = DataStream > #agent.sinks.k1.hdfs.codeC = snappyCodec > 12/06/17 23:08:44 INFO snappy.LoadSnappy: Snappy native library loaded > 12/06/17 23:08:44 WARN hdfs.HDFSEventSink: CodeC: snappyCodec is ignored as > fileType: DataStream is uncompressed. To change fileType if want output > compressed. > 12/06/17 23:08:44 INFO hdfs.HDFSEventSink: Hadoop Security enabled: false > 5. works fine, output file with .snappy extension > agent.sinks.k1.hdfs.fileType = SequenceFile > agent.sinks.k1.hdfs.codeC = snappyCodec > 6. Works fine, output file without .snappy extension > agent.sinks.k1.hdfs.fileType = SequenceFile > #agent.sinks.k1.hdfs.codeC = snappyCodec > > > Thanks, > > Leslin (Hong Xiang Lin) > >
