----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4839/#review7123 -----------------------------------------------------------
Looks good, if the intent was simply to log it. 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). 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. - Hari On 2012-04-22 23:23:14, Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4839/ > ----------------------------------------------------------- > > (Updated 2012-04-22 23:23:14) > > > Review request for Flume. > > > Summary > ------- > > 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. > > > This addresses bug FLUME-1134. > https://issues.apache.org/jira/browse/FLUME-1134 > > > Diffs > ----- > > > flume-ng-core/src/main/java/org/apache/flume/lifecycle/LifecycleSupervisor.java > 2935e64 > > Diff: https://reviews.apache.org/r/4839/diff > > > Testing > ------- > > Logging only change. > > > Thanks, > > Brock > >
