You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Arvind Prabhakar <ar...@apache.org> on 2012/03/21 04:09:20 UTC
Re: Review Request: FLUME-979: ExecSource should optionally restart the
command when it exits
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3962/#review6159
-----------------------------------------------------------
Changes look good Brock. Some minor feedback:
* Please remove the extra whitespaces/tabs that are highlighted as red on the review
flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java
<https://reviews.apache.org/r/3962/#comment13244>
Please create a ExecSourceConfigurationConstants class and declare these strings as public static final String CONFIG_RESTART_THROTTLE etc.
The patch will also need to be rebased on the trunk as the sources have changed a bit since the last time.
- Arvind
On 2012-02-20 14:21:02, Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3962/
> -----------------------------------------------------------
>
> (Updated 2012-02-20 14:21:02)
>
>
> Review request for Flume.
>
>
> Summary
> -------
>
> 1) Allows users to configure a restart flag and sleep interval for the ExecSource should the command exit.
> 2) Currently no one is reading standard error for the sub process. It is not ignored like the documentation suggests. If the command generates enough stderr to fill it's buffer, it will be blocked until someone reads that buffer. A thread is added which by default reads and throws away stderr. If a flag is turned on it will logstderr. This kind of flag can be very useful when debugging why a command is not outputting the expected data.
>
>
> This addresses bug FLUME-979.
> https://issues.apache.org/jira/browse/FLUME-979
>
>
> Diffs
> -----
>
> flume-ng-core/src/test/java/org/apache/flume/source/TestExecSource.java 3014368
> flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java a96016c
>
> Diff: https://reviews.apache.org/r/3962/diff
>
>
> Testing
> -------
>
> A test for the restart flag and restartThrottle flag were added. All unit tests pass.
>
>
> Thanks,
>
> Brock
>
>
Re: Review Request: FLUME-979: ExecSource should optionally restart the
command when it exits
Posted by Brock Noland <br...@cloudera.com>.
> On 2012-03-21 03:09:20, Arvind Prabhakar wrote:
> > Changes look good Brock. Some minor feedback:
> > * Please remove the extra whitespaces/tabs that are highlighted as red on the review
> >
done
> On 2012-03-21 03:09:20, Arvind Prabhakar wrote:
> > flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java, line 211
> > <https://reviews.apache.org/r/3962/diff/3/?file=75933#file75933line211>
> >
> > Please create a ExecSourceConfigurationConstants class and declare these strings as public static final String CONFIG_RESTART_THROTTLE etc.
done
On 2012-03-21 03:09:20, Brock Noland wrote:
> > The patch will also need to be rebased on the trunk as the sources have changed a bit since the last time.
done
- Brock
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3962/#review6159
-----------------------------------------------------------
On 2012-02-20 14:21:02, Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3962/
> -----------------------------------------------------------
>
> (Updated 2012-02-20 14:21:02)
>
>
> Review request for Flume.
>
>
> Summary
> -------
>
> 1) Allows users to configure a restart flag and sleep interval for the ExecSource should the command exit.
> 2) Currently no one is reading standard error for the sub process. It is not ignored like the documentation suggests. If the command generates enough stderr to fill it's buffer, it will be blocked until someone reads that buffer. A thread is added which by default reads and throws away stderr. If a flag is turned on it will logstderr. This kind of flag can be very useful when debugging why a command is not outputting the expected data.
>
>
> This addresses bug FLUME-979.
> https://issues.apache.org/jira/browse/FLUME-979
>
>
> Diffs
> -----
>
> flume-ng-core/src/test/java/org/apache/flume/source/TestExecSource.java 3014368
> flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java a96016c
>
> Diff: https://reviews.apache.org/r/3962/diff
>
>
> Testing
> -------
>
> A test for the restart flag and restartThrottle flag were added. All unit tests pass.
>
>
> Thanks,
>
> Brock
>
>