You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Jonathan Hurley <jh...@hortonworks.com> on 2014/09/15 20:12:58 UTC
Review Request 25652: Alerts: Create Alert Notices For Incoming Alert
State Changes
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25652/
-----------------------------------------------------------
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
Re: Review Request 25652: Alerts: Create Alert Notices For Incoming
Alert State Changes
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> 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
>
>
Re: Review Request 25652: Alerts: Create Alert Notices For Incoming
Alert State Changes
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25652/#review53371
-----------------------------------------------------------
Ship it!
ambari-server/src/main/java/org/apache/ambari/server/events/AlertReceivedEvent.java
<https://reviews.apache.org/r/25652/#comment93009>
This subclass is doing nothing.
ambari-server/src/main/java/org/apache/ambari/server/events/AlertStateChangeEvent.java
<https://reviews.apache.org/r/25652/#comment93007>
Why final? They're not being reassigned outside the ctor.
ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertReceivedListener.java
<https://reviews.apache.org/r/25652/#comment93015>
In the case of a non-OK (probably also non-UNKNOWN) alert, do we need a notice to get sent for it?
- Nate Cole
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
>
>
Re: Review Request 25652: Alerts: Create Alert Notices For Incoming
Alert State Changes
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25652/
-----------------------------------------------------------
(Updated Sept. 15, 2014, 3:19 p.m.)
Review request for Ambari, Mahadev Konar and Nate Cole.
Changes
-------
Updating with test results.
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 (updated)
-------
mvn clean test
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 20:21 min
[INFO] Finished at: 2014-09-15T15:00:25-04:00
[INFO] Final Memory: 29M/220M
[INFO] ------------------------------------------------------------------------
Thanks,
Jonathan Hurley