> On Sept. 15, 2014, 2:44 p.m., Nate Cole wrote: > >
Thanks for the review! > On Sept. 15, 2014, 2:44 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/events/AlertReceivedEvent.java, > > line 26 > > <https://reviews.apache.org/r/25652/diff/1/?file=689370#file689370line26> > > > > This subclass is doing nothing. It's part of the AlertEvent hierarchy. Subscribers can listen for all AlertEvent or just specific types (like AlertReceivedEvent). Also, it's a placeholder for adding information about the received event (such as whether it was from an agent or from the in-server aggregation). > On Sept. 15, 2014, 2:44 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/events/AlertStateChangeEvent.java, > > lines 33-38 > > <https://reviews.apache.org/r/25652/diff/1/?file=689371#file689371line33> > > > > Why final? They're not being reassigned outside the ctor. "final" fields ensure that they are set in the constructor; provides a level of safety. > On Sept. 15, 2014, 2:44 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertReceivedListener.java, > > lines 89-100 > > <https://reviews.apache.org/r/25652/diff/1/?file=689372#file689372line89> > > > > In the case of a non-OK (probably also non-UNKNOWN) alert, do we need a > > notice to get sent for it? I don't believe so; AlertNotices are only on a state change and I believe that this excludes a change from NONE. If the first state is CRITICAL, I don't believe we need to sent an alert through the system. I could see the argument both ways, actually, so I'm open to changing it. UNKNOWN to * is also weird b/c UNKNOWN could be that the agent was restarted. Should that kick of an alert? Probably not. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25652/#review53371 ----------------------------------------------------------- On Sept. 15, 2014, 2:12 p.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25652/ > ----------------------------------------------------------- > > (Updated Sept. 15, 2014, 2:12 p.m.) > > > Review request for Ambari, Mahadev Konar and Nate Cole. > > > Bugs: AMBARI-7316 > https://issues.apache.org/jira/browse/AMBARI-7316 > > > Repository: ambari > > > Description > ------- > > Created the foundation for an event framework to handle alert events in the > system and then hooked up various listeners to handle: > > - incoming alerts (insertion into the database for history/current) > - state changes (insertion of alert notices into the DB) > > Discussed with mahadev and ncole about using Spring events vs Guava/Guice and > it was decided to use the latter. Although Spring seems to be able to > autowire things more seamlessly, using the eager singleton in Guice only > caused some minor coupling between the subscribers and the publishers. > > Alerts can create a lot of incoming events (especially in large clusters). > For this reason, I decided to use an AsyncEventBus that was private only for > alerting events. Ambari as an application can one day create its own > publisher (or just bind EventBus to a specific instance for @Inject) but it > seems like separation here was a good idea. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java > 492d83247d0c2d4e957da96f855292aac4aa8421 > > ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java > c395df628097037c17cf6b1848099a8a19b843a7 > ambari-server/src/main/java/org/apache/ambari/server/events/AlertEvent.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/events/AlertReceivedEvent.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/events/AlertStateChangeEvent.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertReceivedListener.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertStateChangedListener.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/events/publishers/AlertEventPublisher.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java > e08c948c3ff4aee343076ce0277d67ebfad5c4e1 > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertGroupEntity.java > f97a0eb3de4197fa595872e80fd92fb2a69c4f3e > ambari-server/src/main/java/org/apache/ambari/server/state/Alert.java > 7b8aabd2b6e1e440b570110a4f29359f08335454 > > ambari-server/src/main/java/org/apache/ambari/server/state/cluster/AlertDataManager.java > 4a65d5acf136b7d7d653414d7e81d332556998c6 > > ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java > 8451c9b9be3f72e8509a6d0ac989ac2680dacda7 > > ambari-server/src/test/java/org/apache/ambari/server/state/cluster/AlertDataManagerTest.java > eae1de6362758f1ffe66cf54ce8440dc98883b94 > > Diff: https://reviews.apache.org/r/25652/diff/ > > > Testing > ------- > > mvn clean test > > > Thanks, > > Jonathan Hurley > >
