[
https://issues.apache.org/jira/browse/FLUME-1134?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13259727#comment-13259727
]
[email protected] commented on FLUME-1134:
------------------------------------------------------
bq. On 2012-04-23 06:35:54, Hari Shreedharan wrote:
bq. > Looks good, if the intent was simply to log it.
bq. >
bq. > I am not sure of the intent here, is it just to log or to try and take
an action to fix the situation(the propogate call seems to suggest that you
want to get the exception thrown to one of the other threads or the process
terminated).
bq. >
bq. > If the intent is to really check for exceptions in each of the
monitorRunnables and take some action, like shutdown the monitorService or
something, we could have a scheduled thread execute every few seconds and call
monitorFutures.get(lifecyleAware).get() on all the futures stored in
monitorFutures map. If one of them died due to an exception we can catch it.
bq.
bq. Hari Shreedharan wrote:
bq. If one of them died due to an exception we can catch it. <-- By this
I meant, take any action as seen fit at the time.
bq.
bq. Brock Noland wrote:
bq. The intent is a small incremental improvement. Currently all
Throwables are eaten and not logged, so the intent is to log them. I am
propagating because it costs nothing to do and should the way the runnable is
created changes, say the implementation of ScheduledExecutorService changes,
they should be propagated.
Thanks for the explanation, Brock. It is highly unlikely that
ScheduledExecutorService will ever actually throw the exception which is thrown
by a runnable. This is because of the fact that it is impossible to determine
which thread to throw the exception to, since scheduled execution is
asynchronous (and the original thread which scheduled this is now doing
something else), which is the reason the Future.get() function throws the
exception which is thrown by the runnable(wrapped in an ExecutionException).
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4839/#review7123
-----------------------------------------------------------
On 2012-04-22 23:23:14, Brock Noland wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4839/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-04-22 23:23:14)
bq.
bq.
bq. Review request for Flume.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Currently we use an ScheduledExecutorService which eats any throwable from
MonitoredRunnable. The change is to log the error and then propagate in case
the caller implementation should change.
bq.
bq.
bq. This addresses bug FLUME-1134.
bq. https://issues.apache.org/jira/browse/FLUME-1134
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
flume-ng-core/src/main/java/org/apache/flume/lifecycle/LifecycleSupervisor.java
2935e64
bq.
bq. Diff: https://reviews.apache.org/r/4839/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Logging only change.
bq.
bq.
bq. Thanks,
bq.
bq. Brock
bq.
bq.
> LifecycleSupervisor.MonitorRunnable does not log runtime errors
> ---------------------------------------------------------------
>
> Key: FLUME-1134
> URL: https://issues.apache.org/jira/browse/FLUME-1134
> Project: Flume
> Issue Type: Bug
> Affects Versions: v1.1.0
> Reporter: Brock Noland
> Assignee: Brock Noland
> Attachments: FLUME-1134-0.patch
>
>
> LifecycleSupervisor.MonitorRunnable does not log RuntimeException/Error but
> the class we use to execute these runnables eats the exceptions. The run
> method should be wrapped in a try catch and log.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
