----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4497/#review6497 -----------------------------------------------------------
Ship it! +1 Changes look good Mike. One request - please consider using constant fields to represent the configuration keys (bind, port, max-line-length etc), preferably declared in a separate class called NetcatSourceConfigurationConstants or similar. Please attach the patch the Jira. - Arvind On 2012-03-28 23:16:36, Mike Percy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4497/ > ----------------------------------------------------------- > > (Updated 2012-03-28 23:16:36) > > > Review request for Flume. > > > Summary > ------- > > The NetcatSource does not create one event per newline. Instead, it creates > one event per TCP session. > > I have addressed multiple issues with NetcatSource in this patch: > > 1. Properly process a new event per newline. > 2. Enforce a maximum length per line, to ensure clients cannot run the server > out of memory. (This is now configurable; the default is 512 characters.) > 3. Properly flush responses to the client. > 4. Increment counters for successful processing and failure. > 5. Use shutdownNow() when shutting down the server, otherwise an open client > connection will cause the server to hang on shutdown. > 6. Made the inner classes of NetcatSource private; I believe making them > public was unintentional. > 7. Corrected unit test so it now sends a newline with each request. Also now > checks for "OK" response from server. > 8. Attempting to sneak in a minor fix for the EventHelper.dumpEvent() method > into this patch, since I'm testing with LoggerSink and it's bugging me when > it throws an exception on a zero-length body. > > > This addresses bug FLUME-1037. > https://issues.apache.org/jira/browse/FLUME-1037 > > > Diffs > ----- > > flume-ng-core/src/main/java/org/apache/flume/event/EventHelper.java a326a70 > flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java > a841b0e > flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java > fb2a960 > > Diff: https://reviews.apache.org/r/4497/diff > > > Testing > ------- > > Unit tests pass. Did a bunch of manual testing using the nc tool with a > Netcat source and a Logger sink. > > > Thanks, > > Mike > >
