> On Sept. 22, 2012, 7:04 a.m., Hari Shreedharan wrote: > > Brock, > > > > I don't see anything wrong with the patch except some unneeded whitespace > > changes in files which don't have any changes at all. But what I am trying > > to understand is the problem this is trying to solve. In the current code, > > if the ExecRunnable ever leaves the try block, it will kill the process. So > > is the problem that it never leaves that try block? > > Hari Shreedharan wrote: > The test does not pass without the patch, so what I am asking was how > exactly does this issue occur?
Without this patch, exec source never shuts down correctly. The runnable sits waiting in the readLine() call forever. With the patch it kills the child in the stop method which closes stdout/stderr and then the readLine() returns null due to EOF and the runnable is able to terminate. 2012-09-22 07:25:26,891 (main) [DEBUG - org.apache.flume.source.ExecSource.stop(ExecSource.java:193)] Waiting for exec executor service to stop 2012-09-22 07:25:27,393 (main) [DEBUG - org.apache.flume.source.ExecSource.stop(ExecSource.java:193)] Waiting for exec executor service to stop 2012-09-22 07:25:27,899 (main) [DEBUG - org.apache.flume.source.ExecSource.stop(ExecSource.java:193)] Waiting for exec executor service to stop 2012-09-22 07:25:28,400 (main) [DEBUG - org.apache.flume.source.ExecSource.stop(ExecSource.java:193)] Waiting for exec executor service to stop 2012-09-22 07:25:28,901 (main) [DEBUG - org.apache.flume.source.ExecSource.stop(ExecSource.java:193)] Waiting for exec executor service to stop 2012-09-22 07:25:29,403 (main) [DEBUG - org.apache.flume.source.ExecSource.stop(ExecSource.java:193)] Waiting for exec executor service to stop 2012-09-22 07:25:29,904 (main) [DEBUG - org.apache.flume.source.ExecSource.stop(ExecSource.java:193)] Waiting for exec executor service to stop 2012-09-22 07:25:30,405 (main) [DEBUG - org.apache.flume.source.ExecSource.stop(ExecSource.java:193)] Waiting for exec executor service to stop 2012-09-22 07:25:30,907 (main) [DEBUG - org.apache.flume.source.ExecSource.stop(ExecSource.java:193)] Waiting for exec executor service to stop - Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7167/#review11809 ----------------------------------------------------------- On Sept. 19, 2012, 3:17 p.m., Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7167/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2012, 3:17 p.m.) > > > Review request for Flume. > > > Description > ------- > > Makes exec source kill it's children when stopping. > > > This addresses bug FLUME-1590. > https://issues.apache.org/jira/browse/FLUME-1590 > > > Diffs > ----- > > flume-ng-core/src/main/java/org/apache/flume/source/AbstractSource.java > 861cc42 > flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java 155f0e2 > > flume-ng-core/src/main/java/org/apache/flume/source/PollableSourceRunner.java > f6c64b3 > flume-ng-core/src/test/java/org/apache/flume/source/TestExecSource.java > 615f2a3 > > flume-ng-core/src/test/java/org/apache/flume/source/TestMultiportSyslogTCPSource.java > 680e592 > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogParser.java > 258c2f1 > > Diff: https://reviews.apache.org/r/7167/diff/ > > > Testing > ------- > > Adds test to make sure this occurs. > > > Thanks, > > Brock Noland > >
