----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60357/#review178679 -----------------------------------------------------------
flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java Lines 176 (patched) <https://reviews.apache.org/r/60357/#comment252825> Please remove the unneccessary whitespaces (highlighted in red). Here and in the test class. flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatSource.java Lines 322-324 (patched) <https://reviews.apache.org/r/60357/#comment252849> Nit: You can merge line #322 and #324 and use try-with-resources ``` try (ServerSocketChannel dummyServerSocket = ServerSocketChannel.open()) { ... } ``` and remove the dummyServerSocket.close() from the end flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatSource.java Lines 330 (patched) <https://reviews.apache.org/r/60357/#comment252848> I think you should avoid using the startSource() for this test as it would be responsible for selecting a free port (even though it is buggy and doesn't do that). This would be the replacement: ``` Context context = new Context(); context.put("port", String.valueOf(PORT)); context.put("bind", "0.0.0.0"); context.put("ack-every-event", "false"); Configurables.configure(source, context); source.start(); ``` - Attila Simon On June 22, 2017, 5:48 a.m., Siddharth Ahuja wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60357/ > ----------------------------------------------------------- > > (Updated June 22, 2017, 5:48 a.m.) > > > Review request for Flume and Attila Simon. > > > Repository: flume-git > > > Description > ------- > > Review request for: https://issues.apache.org/jira/browse/FLUME-2905 which is > trying to prevent socket leaks when a netcat port is already bound to an > existing process. > > > Diffs > ----- > > flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java > 9513902 > flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatSource.java > 99d413a > > > Diff: https://reviews.apache.org/r/60357/diff/1/ > > > Testing > ------- > > I have tested flume-ng executable generated from my changes and I can confirm > from the lsof output that the sockets do not keep increasing if the port to > which netcat source is trying to bind to is already in use. > The junits are also passing for me for the NetcatSource. > > > Thanks, > > Siddharth Ahuja > >
