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/24 21:59:46 UTC

Review Request 26003: Alerts: data collection starts on install before config values are available

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

Review request for Ambari and Nate Cole.


Bugs: AMBARI-7480
    https://issues.apache.org/jira/browse/AMBARI-7480


Repository: ambari


Description
-------

Configuration properties should exist on all AlertDefinitionCommands sent to the agents. Additionally, alert definitions that are aggregates need to register with the system in order to run after a cluster install.


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/agent/AlertDefinitionCommand.java 5ae1741352711c726d581e55e0ddfb202cb27074 
  ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java a3663019299ffd6b234bc3c1965fd2e6a5f8802c 
  ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java ec53afca0a9049643621d437d88e6d1ccd71cb02 
  ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java cfab1f825b7db6422107f0efad478680039991b5 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java 7cb9886dd6f3dead8e707f400c753313b492356f 
  ambari-server/src/main/java/org/apache/ambari/server/events/AlertDefinitionRegistrationEvent.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java dda18c78c41cd1471e1cbc5219262d13ea5cd7a1 
  ambari-server/src/main/java/org/apache/ambari/server/events/ClusterEvent.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/events/ServiceEvent.java 5e014312816d4c4b4b67789a39c7c64cf92a4bfe 
  ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertAggregateListener.java 8340c264cfed57f1e2eabff1f9b0f157a7dd2b64 
  ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertLifecycleListener.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAO.java 570f26840016ed7ce63c750a8b37f433ee511fe3 
  ambari-server/src/main/java/org/apache/ambari/server/state/alert/AggregateDefinitionMapping.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java f227c0abd001f3f5207c3e4e912b9f2e6a4b1c40 
  ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java f8e44bde0f7841434a7997e2b710c657ecf8ece9 
  ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionHash.java 0a1c73a9bd368f748ebb15d174836cdc55ed8311 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProviderTest.java d2ce6fbc213befd30339f9d6c77836acb7ea17a1 
  ambari-server/src/test/java/org/apache/ambari/server/orm/OrmTestHelper.java 08cd7b83aaca990c3caa040398cbe2a5604bdee5 
  ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAOTest.java 3e20cd66fb3d76e4c16b33c2b8b46ec80f1d494c 
  ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java 7f4deda3e84c6da1e70e9fd2e49d04a02ee96ce1 
  ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java 6e4d4af929fb3c0f5df9dd1a2cd2fae48d61cd02 
  ambari-server/src/test/java/org/apache/ambari/server/state/cluster/AlertDataManagerTest.java 00ba942f125fdc44bfdeb1bee1a4f83ec208a492 

Diff: https://reviews.apache.org/r/26003/diff/


Testing
-------

mvn clean test


Thanks,

Jonathan Hurley


Re: Review Request 26003: Alerts: data collection starts on install before config values are available

Posted by Jonathan Hurley <jh...@hortonworks.com>.

> On Sept. 24, 2014, 4:13 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java, line 479
> > <https://reviews.apache.org/r/26003/diff/1/?file=704666#file704666line479>
> >
> >     Is label defined as a required property in key_properties.json?  Doesn't seem as though that should always be set if it isn't required.

key_properties.json doesn't allow duplicate keys, so I think this is the way to ensure they are always added to the Response.


> On Sept. 24, 2014, 4:13 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/events/ServiceEvent.java, line 24
> > <https://reviews.apache.org/r/26003/diff/1/?file=704670#file704670line24>
> >
> >     Hmmm, not sure how to handle this for services that are not part of a Cluster (like that wack AMBARI_SERVER and such).  A possible kerberos service is coming soon, but I think that should be associated with a cluster.  Probably ok.

That's a very good point. I don't think it matters too much since a value of 0 for the clusterId would just mean it's an unbound service. I don't like that very much though because it breaks the inheritance hierarchy. We could always create a new event to handle those weirdo services in the future.


> On Sept. 24, 2014, 4:13 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/alert/AggregateDefinitionMapping.java, lines 65-69
> > <https://reviews.apache.org/r/26003/diff/1/?file=704674#file704674line65>
> >
> >     Doc doesn't match the args

Copy/paste error. Thanks!


- Jonathan


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


On Sept. 24, 2014, 3:59 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26003/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2014, 3:59 p.m.)
> 
> 
> Review request for Ambari and Nate Cole.
> 
> 
> Bugs: AMBARI-7480
>     https://issues.apache.org/jira/browse/AMBARI-7480
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Configuration properties should exist on all AlertDefinitionCommands sent to the agents. Additionally, alert definitions that are aggregates need to register with the system in order to run after a cluster install.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/agent/AlertDefinitionCommand.java 5ae1741352711c726d581e55e0ddfb202cb27074 
>   ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java a3663019299ffd6b234bc3c1965fd2e6a5f8802c 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java ec53afca0a9049643621d437d88e6d1ccd71cb02 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java cfab1f825b7db6422107f0efad478680039991b5 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java 7cb9886dd6f3dead8e707f400c753313b492356f 
>   ambari-server/src/main/java/org/apache/ambari/server/events/AlertDefinitionRegistrationEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java dda18c78c41cd1471e1cbc5219262d13ea5cd7a1 
>   ambari-server/src/main/java/org/apache/ambari/server/events/ClusterEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/events/ServiceEvent.java 5e014312816d4c4b4b67789a39c7c64cf92a4bfe 
>   ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertAggregateListener.java 8340c264cfed57f1e2eabff1f9b0f157a7dd2b64 
>   ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertLifecycleListener.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAO.java 570f26840016ed7ce63c750a8b37f433ee511fe3 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/AggregateDefinitionMapping.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java f227c0abd001f3f5207c3e4e912b9f2e6a4b1c40 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java f8e44bde0f7841434a7997e2b710c657ecf8ece9 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionHash.java 0a1c73a9bd368f748ebb15d174836cdc55ed8311 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProviderTest.java d2ce6fbc213befd30339f9d6c77836acb7ea17a1 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/OrmTestHelper.java 08cd7b83aaca990c3caa040398cbe2a5604bdee5 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAOTest.java 3e20cd66fb3d76e4c16b33c2b8b46ec80f1d494c 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java 7f4deda3e84c6da1e70e9fd2e49d04a02ee96ce1 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java 6e4d4af929fb3c0f5df9dd1a2cd2fae48d61cd02 
>   ambari-server/src/test/java/org/apache/ambari/server/state/cluster/AlertDataManagerTest.java 00ba942f125fdc44bfdeb1bee1a4f83ec208a492 
> 
> Diff: https://reviews.apache.org/r/26003/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


Re: Review Request 26003: Alerts: data collection starts on install before config values are available

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26003/#review54451
-----------------------------------------------------------

Ship it!



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java
<https://reviews.apache.org/r/26003/#comment94588>

    Is label defined as a required property in key_properties.json?  Doesn't seem as though that should always be set if it isn't required.



ambari-server/src/main/java/org/apache/ambari/server/events/ServiceEvent.java
<https://reviews.apache.org/r/26003/#comment94589>

    Hmmm, not sure how to handle this for services that are not part of a Cluster (like that wack AMBARI_SERVER and such).  A possible kerberos service is coming soon, but I think that should be associated with a cluster.  Probably ok.



ambari-server/src/main/java/org/apache/ambari/server/state/alert/AggregateDefinitionMapping.java
<https://reviews.apache.org/r/26003/#comment94586>

    Doc doesn't match the args


- Nate Cole


On Sept. 24, 2014, 3:59 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26003/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2014, 3:59 p.m.)
> 
> 
> Review request for Ambari and Nate Cole.
> 
> 
> Bugs: AMBARI-7480
>     https://issues.apache.org/jira/browse/AMBARI-7480
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Configuration properties should exist on all AlertDefinitionCommands sent to the agents. Additionally, alert definitions that are aggregates need to register with the system in order to run after a cluster install.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/agent/AlertDefinitionCommand.java 5ae1741352711c726d581e55e0ddfb202cb27074 
>   ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java a3663019299ffd6b234bc3c1965fd2e6a5f8802c 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java ec53afca0a9049643621d437d88e6d1ccd71cb02 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java cfab1f825b7db6422107f0efad478680039991b5 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java 7cb9886dd6f3dead8e707f400c753313b492356f 
>   ambari-server/src/main/java/org/apache/ambari/server/events/AlertDefinitionRegistrationEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java dda18c78c41cd1471e1cbc5219262d13ea5cd7a1 
>   ambari-server/src/main/java/org/apache/ambari/server/events/ClusterEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/events/ServiceEvent.java 5e014312816d4c4b4b67789a39c7c64cf92a4bfe 
>   ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertAggregateListener.java 8340c264cfed57f1e2eabff1f9b0f157a7dd2b64 
>   ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertLifecycleListener.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAO.java 570f26840016ed7ce63c750a8b37f433ee511fe3 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/AggregateDefinitionMapping.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java f227c0abd001f3f5207c3e4e912b9f2e6a4b1c40 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java f8e44bde0f7841434a7997e2b710c657ecf8ece9 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionHash.java 0a1c73a9bd368f748ebb15d174836cdc55ed8311 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProviderTest.java d2ce6fbc213befd30339f9d6c77836acb7ea17a1 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/OrmTestHelper.java 08cd7b83aaca990c3caa040398cbe2a5604bdee5 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAOTest.java 3e20cd66fb3d76e4c16b33c2b8b46ec80f1d494c 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java 7f4deda3e84c6da1e70e9fd2e49d04a02ee96ce1 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java 6e4d4af929fb3c0f5df9dd1a2cd2fae48d61cd02 
>   ambari-server/src/test/java/org/apache/ambari/server/state/cluster/AlertDataManagerTest.java 00ba942f125fdc44bfdeb1bee1a4f83ec208a492 
> 
> Diff: https://reviews.apache.org/r/26003/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>