You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Brock Noland <br...@cloudera.com> on 2012/09/19 17:17:29 UTC
Review Request: FLUME-1590: ExecSource doesn't kill it's child when shutting
down
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7167/
-----------------------------------------------------------
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
Re: Review Request: FLUME-1590: ExecSource doesn't kill it's child when
shutting down
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7167/#review11855
-----------------------------------------------------------
Ship it!
Patch already committed. Thanks Brock!
- Hari Shreedharan
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
>
>
Re: Review Request: FLUME-1590: ExecSource doesn't kill it's child when
shutting down
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7167/#review11854
-----------------------------------------------------------
Ship it!
Ship It!
- Hari Shreedharan
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
>
>
Re: Review Request: FLUME-1590: ExecSource doesn't kill it's child when
shutting down
Posted by Brock Noland <br...@cloudera.com>.
> 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
>
>
Re: Review Request: FLUME-1590: ExecSource doesn't kill it's child when
shutting down
Posted by Hari Shreedharan <hs...@cloudera.com>.
> 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?
The test does not pass without the patch, so what I am asking was how exactly does this issue occur?
- Hari
-----------------------------------------------------------
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
>
>
Re: Review Request: FLUME-1590: ExecSource doesn't kill it's child when
shutting down
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7167/#review11809
-----------------------------------------------------------
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
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
>
>
Re: Review Request: FLUME-1590: ExecSource doesn't kill it's child when
shutting down
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7167/#review11814
-----------------------------------------------------------
Brock,
can you remove the unneeded whitespace changes and attach the patch to the jira?
- Hari Shreedharan
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
>
>