> On 2012-03-11 07:45:41, Mike Percy wrote: > > Hi! I just wanted to butt in here and confess that I'm concerned about this > > change. I understand that there is a nonzero cost associated with > > deserializing a String to a long but please consider the following: > > > > 1) It clutters the Event interface, which is now extremely general and > > flexible (it's very easy to understate the value of this) > > 2) What if someone has timestamps embedded in their body data? What if they > > don't need a timestamp for their use case? > > 3) If we want to support efficient deserialization of metadata maybe we > > should be talking about enabling (Avro-) serialized data in the headers by > > using byte[] as the header map value type. OTOH, one could argue that we > > should be wary of premature optimization this early in the life of the > > project. > > > > Just my 2c. > > > > Regards > > Ralph Goers wrote: > I was actually surprised when the timestamp was removed in the transition > from OG to NG. Virtually every log record will have a timestamp. It is > actually more of a pain for me to transfer it as a string field than as a > fixed integer attribute. In addition, I wish the event also included a unique > id field. Unless this has changed from OG we had to include a UUID in the > event as Flume couldn't guarantee the event would only be written to the > eventual target only once. > > In case it isn't obvious, I'm all for adding the timestamp to the event, > especially now before 1.x is cast in concrete.
Hmm, I misread one part of the patch in which I thought that the timestamp had been added as a field to the wire protocol. Actually, basically helper functions were added to the core Event interface & implementation for serialization & deserialization of the timestamp from the String in the header, and the wire protocol was not changed. Seems pretty tacked on to me and I agree with the comment about expensive getters especially in such a core class. I understand what you're saying though. Valid points. - Mike ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3088/#review5825 ----------------------------------------------------------- On 2011-12-09 04:21:34, Vibul Imtarnasan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3088/ > ----------------------------------------------------------- > > (Updated 2011-12-09 04:21:34) > > > Review request for Flume. > > > Summary > ------- > > FLUME-872 > > > Diffs > ----- > > > /branches/flume-728/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSTextFormatter.java > 1212231 > > /branches/flume-728/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSWritableFormatter.java > 1212231 > > /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/event/EventBuilder.java > 1212231 > > /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java > 1212231 > > /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/formatter/output/BucketPath.java > 1212231 > > /branches/flume-728/flume-ng-core/src/test/java/org/apache/flume/event/TestSimpleEvent.java > PRE-CREATION > > /branches/flume-728/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java > 1212231 > /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/Event.java > 1212231 > > Diff: https://reviews.apache.org/r/3088/diff > > > Testing > ------- > > Unit test cases included > > > Thanks, > > Vibul > >
