ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tom Beerbower" <tbeerbo...@hortonworks.com>
Subject Re: Review Request 26327: Alerts: Maintenance Mode Should Prevent Alert Notifications
Date Fri, 03 Oct 2014 21:58:50 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26327/#review55400
-----------------------------------------------------------

Ship it!


Minor comments / questions.  Looks good.


ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java
<https://reviews.apache.org/r/26327/#comment95754>

    Is a null return from findCandidateComponents actually possible?  If so, how did you decide
that null means error and not just that no matching components were found?
    
    The docs for ClassPathScanningCandidateComponentProvider aren't very helpful.



ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertAggregateListener.java
<https://reviews.apache.org/r/26327/#comment95757>

    I know that you mentioned it in your comment, but it does seem redundant to say this object
is both a singleton and an eager singleton.



ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java
<https://reviews.apache.org/r/26327/#comment95760>

    I wonder if we should standardize on the use of m_.
    
    I used to use it but switched away since it looked like most of the group didn't.
    
    I don't don't really care either way and I understand that we each have our own style.
 I just like consistency across the project.


- Tom Beerbower


On Oct. 3, 2014, 9:01 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26327/
> -----------------------------------------------------------
> 
> (Updated Oct. 3, 2014, 9:01 p.m.)
> 
> 
> Review request for Ambari, Nate Cole and Tom Beerbower.
> 
> 
> Bugs: AMBARI-7642
>     https://issues.apache.org/jira/browse/AMBARI-7642
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Putting a service or a host into maintenance mode should cause all instances of alerts
that would be affected to also go into maintenance mode.
> 
> During this mode, the database is updated to reflect that maintenance mode is on, however
alerts still remain scheduled and running on the agents.
> 
> Data that is returned is stored as usual, but no alert state changes are triggered.
> 
> Some explanation of side effect changes:
> - EagerSingleton is a way for us to annotate a listener that needs to be registerd with
an EventBus. Previously, ControllerModule would need to call out each class individually and
it was an emerging anti-pattern. Now, ControllerModule just scans for EagerSingletons and
binds them generically. It actually might be possible to remove @Singleton since we are doing
a `bind(...).asEagerSingleton` which should enforce it being a real Guice singleton; but then
I figured someone might also want to key off of @Singleton as well, so I left that annotation.
> 
> - HostImpl and ServiceImpl had a ton of code formatting change (I _hate_ the use of `this.`
when it's not necessary). The only real changes here were to fire the MM event.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/EagerSingleton.java PRE-CREATION

>   ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java
da78639f13e0c5877b6f715017f094e29a0c687a 
>   ambari-server/src/main/java/org/apache/ambari/server/events/AlertStateChangeEvent.java
efb7119a21595ee5424a4ff463861ec6d7813fec 
>   ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java 3dae67667413667c7266fe80e23f2f3248a185a7

>   ambari-server/src/main/java/org/apache/ambari/server/events/MaintenanceModeEvent.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertAggregateListener.java
a0c8e467c304da4ffcaf59cf0fbb90a153ba0a70 
>   ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertLifecycleListener.java
9d2f4c623243bebdf6b6841803fe405425cf8511 
>   ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertMaintenanceModeListener.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertReceivedListener.java
78d8e664755e35c73f9eff23646c3283dbe7563f 
>   ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertServiceStateListener.java
6215cc1ba6f1f12914aa25bbcc3f17c988f60406 
>   ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertStateChangedListener.java
b2f08d7ca401b6d22a6c85587e18c2ed0c3746bf 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java e6b5921f681a80f2117ecc8457cdf8f0104b95b4

>   ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java 619859c270baa00b5780329cf5639efee9bd308f

>   ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java
75265b7a52d5ee562f748b3e2164a3a68a197c72 
>   ambari-server/src/test/java/org/apache/ambari/server/events/EventsTest.java PRE-CREATION

>   ambari-server/src/test/java/org/apache/ambari/server/events/MockEventListener.java
PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java 0cc2ae7484674db044c6639077d726cd2943874f

>   ambari-server/src/test/java/org/apache/ambari/server/state/cluster/AlertDataManagerTest.java
502c3b9be88c111aea49f537e40d1fded1fe1ae6 
> 
> Diff: https://reviews.apache.org/r/26327/diff/
> 
> 
> Testing
> -------
> 
> New tests added; mvn clean test
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 23:04 min
> [INFO] Finished at: 2014-10-03T16:47:18-04:00
> [INFO] Final Memory: 29M/220M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message