> On 2012-04-23 06:35:54, Hari Shreedharan wrote: > > 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 Shreedharan wrote: > 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.
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. - Brock ----------------------------------------------------------- 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: > > ----------------------------------------------------------- > 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 > >
