----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4826/#review7092 -----------------------------------------------------------
Looks pretty good. I have some feedback which is inline. Suggestion: Do you think you could add support for the decorators in the sources as well (though we can do that later, if you want to leave it for now)? Eg: ExecSource might want to add some header based on the event size etc? It would be good to have that flume-ng-core/src/main/java/org/apache/flume/event/EventHeaderDecoratorFactory.java <https://reviews.apache.org/r/4826/#comment15699> It would be good to refine this factory a bit. Please take a look at the other factory classes. Use an enum to specify the type of the decorator to build, or if it is not a know type, use the class name specified. Take a look at DefaultSourceFactory etc. flume-ng-core/src/main/java/org/apache/flume/event/SinkEventHeaderDecorator.java <https://reviews.apache.org/r/4826/#comment15698> This should be named something like TimeStampEventDecorator or something. We might want to add decorators that add other stuff at the sink. - Hari On 2012-04-20 10:33:14, Inder Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4826/ > ----------------------------------------------------------- > > (Updated 2012-04-20 10:33:14) > > > Review request for Flume, Arvind Prabhakar, Mike Percy, and Hari Shreedharan. > > > Summary > ------- > > Overview of Changes > > 1.EventHeaderDecorator -> generic class having processHeader() > 2.SinkEventHeaderDecorator is a EventHeaderDecorator which overrides > timestamp header at this hop. > 3.EventHeaderDecoratorFactory provides EventHeaderDecorator's based on > <sink>.headerDecorator config > 4.AbstractSink -> has a eventHeaderDecortaor > 5.HDFSEventSink -> checks for decorator through config and calls > processHeader() for each event which modifies/add headers based on event > decorator defined through config. > > Sample Configuration for testing > ---------------------------------- > agent1.sinks.log-sink1.hdfs.headerDecorator = sinkDecorator > > > This addresses bug FLUME-1097. > https://issues.apache.org/jira/browse/FLUME-1097 > > > Diffs > ----- > > > flume-ng-core/src/main/java/org/apache/flume/event/EventHeaderDecorator.java > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/event/EventHeaderDecoratorFactory.java > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/event/SinkEventHeaderDecorator.java > PRE-CREATION > flume-ng-core/src/main/java/org/apache/flume/sink/AbstractSink.java 2334059 > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java > 1b61cad > > Diff: https://reviews.apache.org/r/4826/diff > > > Testing > ------- > > Testing Done > ------------- > 1. Unit tests > 2. manual tests done - agent1.sinks.log-sink1.hdfs.filePrefix = flume-data/%D > results in publishing data with timestamp at agent's hop running HDFSEventSink > > > Thanks, > > Inder > >
