> On April 28, 2015, 9:23 a.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java, > > line 142 > > <https://reviews.apache.org/r/33593/diff/2/?file=943230#file943230line142> > > > > Seems a little dangerous if the process blocks/deadlocks. Maybe wrap > > in a thread that you then invoke join(timeout) on? > > > > Thread t = new Thread(new Runnable() \{ > > try \{ > > process.waitFor(); > > \} catch (Exception e) \{ > > process.kill(); > > \} > > \}); > > t.start(); > > t.join(2000); > > > > (or something like that, maybe extend thread to capture the exit code?)
Agreed that this could be a problem. Implemented a timeout policy along with an executor. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33593/#review81811 ----------------------------------------------------------- On April 27, 2015, 3:32 p.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33593/ > ----------------------------------------------------------- > > (Updated April 27, 2015, 3:32 p.m.) > > > Review request for Ambari, Nate Cole and Tom Beerbower. > > > Bugs: AMBARI-9919 > https://issues.apache.org/jira/browse/AMBARI-9919 > > > Repository: ambari > > > Description > ------- > > Add support for alert dispatchers which invoke a configurable command line > script. This involves several steps: > > - Change the Ambari framework to allow for custom dispatchers to be picked up > via the classpath. > - Allow for dispatchers to receive notifications that do not have > pre-generated content since the {{alert-templates.xml}} may not be able to > provide the correct command-line arguments for the script. > - The script dispatcher should be able to use any script defined in > ambari.properties. A single dispatcher class can therefore dispatch to any > number of scripts if the script path is found in ambari.properties. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java > bbeca38 > > ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java > 184c8db > > ambari-server/src/main/java/org/apache/ambari/server/notifications/Notification.java > 5e34b12 > > ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java > 8a88b42 > > ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java > 4abec3a > > ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java > a147bac > > ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertNotification.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/alert/TargetType.java > 9f3b5d8 > > ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java > 0f0e637 > > ambari-server/src/test/java/org/apache/ambari/server/notifications/DispatchFactoryTest.java > ff626ac > > ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java > 5c248e1 > > ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java > c4c2ed8 > > Diff: https://reviews.apache.org/r/33593/diff/ > > > Testing > ------- > > Added a dummy script to ambari.properties and had it echo out the alert > properties that were being sent. > > mvn clean test > > > Thanks, > > Jonathan Hurley > >
