----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3831/#review5027 -----------------------------------------------------------
See comments below. flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java <https://reviews.apache.org/r/3831/#comment11030> SyslogUtils.extractEvent can return null. It doesn't appear as though ChannelProcessor.processEvent is prepared to handle null. flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java <https://reviews.apache.org/r/3831/#comment11036> PollableSourceRunner expects the process method to return non-null. Should BACKOFF be returned or should the IOException be wrapped and rethrown? flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java <https://reviews.apache.org/r/3831/#comment11031> Is Source.process supposed to be thread safe? If not, do we have to allocate this memory each time? flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java <https://reviews.apache.org/r/3831/#comment11032> Although 0 is very likely the correct offset, shouldn't we be using pkt.getOffset()? flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java <https://reviews.apache.org/r/3831/#comment11034> As stated earlier, extractEvent could return null? flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java <https://reviews.apache.org/r/3831/#comment11035> PollableSourceRunner expects the process method to return non-null. Should BACKOFF be returned or should the IOException be wrapped and rethrown? flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java <https://reviews.apache.org/r/3831/#comment11037> From looking at PollableSourceRunner it looks like the proccess method will be called if the start method returns non-error. At this point the process method will NPE correct? flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java <https://reviews.apache.org/r/3831/#comment11033> It looks like this method will not throw an IOException? - Brock On 2012-02-10 00:43:59, Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3831/ > ----------------------------------------------------------- > > (Updated 2012-02-10 00:43:59) > > > Review request for Flume. > > > Summary > ------- > > syslog source support for Flume 1.x > > > This addresses bug FLUME-892. > https://issues.apache.org/jira/browse/FLUME-892 > > > Diffs > ----- > > > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUdpSource.java > PRE-CREATION > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java > PRE-CREATION > flume-ng-core/pom.xml d753fa1 > flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java > PRE-CREATION > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/3831/diff > > > Testing > ------- > > regression tests > Added new test for UDP soruce. tested the TCP source manually. Haven't found > a package that has the tcp syslog client. will add a testcase with custom tcp > client to test the source. > > > Thanks, > > Prasad > >
