> On 2012-04-16 01:49:48, Hari Shreedharan wrote: > > I have one general question about this. I am not entirely sure that the > > serializer should handle the stream directly. I think it would be better if > > we simply return the serialized event as a byte array from the > > EventSerializer, which the class that deals with the output streams(the > > sinks etc) can deal with directly. This would in general remove the > > requirement of the serializer component to have a bunch of functions which > > are not associated with serializing at all, but with writing/flushing etc > > of the output stream. The serializer should simply return a serialized > > event which the sink can write to the stream. The serializer should not > > really be concerned about flushing the stream etc. > > > > The event serializer in my opinion should be simple. It knows the schema > > and simply returns the byte array which can be written.
Hey - I agree with you that implementations should not be calling flush() on the output stream. I've incorporated that feedback in the latest patch (removed that from the BodyTextEventSerializer and documented it in the interface javadocs). The main reason that flush() exists is so that serializer implementations can buffer output. This allows us to implement things like block compression, as well as batching writes on transaction boundaries. We write to a stream instead of returning a byte array for the same reason. - Mike ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4708/#review6928 ----------------------------------------------------------- On 2012-04-16 04:09:32, Mike Percy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4708/ > ----------------------------------------------------------- > > (Updated 2012-04-16 04:09:32) > > > Review request for Flume. > > > Summary > ------- > > Patch with support for Avro Container File format. > > > This addresses bug FLUME-1117. > https://issues.apache.org/jira/browse/FLUME-1117 > > > Diffs > ----- > > flume-ng-core/pom.xml 37fb112 > > flume-ng-core/src/main/java/org/apache/flume/formatter/output/EventFormatter.java > c8b953d > > flume-ng-core/src/main/java/org/apache/flume/formatter/output/TextDelimitedOutputFormatter.java > a12afd3 > > flume-ng-core/src/main/java/org/apache/flume/serialization/AbstractAvroEventSerializer.java > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/serialization/BodyTextEventSerializer.java > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/serialization/EventSerializer.java > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/serialization/EventSerializerFactory.java > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/serialization/EventSerializerType.java > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/serialization/FlumeEventAvroEventSerializer.java > PRE-CREATION > flume-ng-core/src/main/java/org/apache/flume/sink/RollingFileSink.java > a2f4f66 > > flume-ng-core/src/test/java/org/apache/flume/serialization/SyslogAvroEventSerializer.java > PRE-CREATION > > flume-ng-core/src/test/java/org/apache/flume/serialization/TestBodyTextEventSerializer.java > PRE-CREATION > > flume-ng-core/src/test/java/org/apache/flume/serialization/TestFlumeEventAvroEventSerializer.java > PRE-CREATION > > flume-ng-core/src/test/java/org/apache/flume/serialization/TestSyslogAvroEventSerializer.java > PRE-CREATION > flume-ng-core/src/test/resources/syslog_event.avsc PRE-CREATION > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/BucketWriter.java > fdad75b > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSCompressedDataStream.java > c8e1df9 > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSDataStream.java > 39a4456 > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java > 9f28d82 > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSSequenceFile.java > 49a62df > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSWriter.java > 2e5470e > > flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/MockHDFSWriter.java > 73d3284 > > flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/TestBucketWriter.java > fb61092 > > flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/TestHDFSEventSink.java > 5a6ad47 > pom.xml 3a9bc42 > > Diff: https://reviews.apache.org/r/4708/diff > > > Testing > ------- > > Unit tests pass. New unit tests added for new functionality. > > > Thanks, > > Mike > >
