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 26744: Alerts: Expose Alert Notices via REST APIs
Date Wed, 15 Oct 2014 16:57:57 GMT


> On Oct. 15, 2014, 1:05 p.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java,
lines 170-171
> > <https://reviews.apache.org/r/26744/diff/1/?file=722080#file722080line170>
> >
> >     Is getPropertyMaps() needed for this RP?  The purpose of doing this was so that
a predicate like '( A && B ) || ( A && C )' could be split up into multiple
(two in this case) backend requests.  This was required because the AMC model uses request
objects that take a set of properties... there is no way to handle OR (see doc for SimplifyingPredicateVisitor).
> >     
> >     It looks like you generate the backend JPA query based on the entire predicate,
which will be done twice with the above example.
> 
> Jonathan Hurley wrote:
>     You are correct that I pass the entire Predicate to the backened JPA query. However,
I still need to extract some properties from the request (like the cluster name and possibly
the ID of the resource if this is a non-collection request). 
>     
>     If I don't use the predicate to get the properties, where would they come from? I
think I have to use Set<Map<String, Object>> propertyMaps = getPropertyMaps(predicate);
in order to get those properties, no?

You can use getPropertyMaps(predicate) but I think that there may be an issue.  It might not
ever cause incorrect results but may not be optimal.  

Consider this predicate 'AlertNotice/id=5|AlertNotice/notification_state=DELIVERED'.  The
call to getPropertyMaps(predicate) would return 2 maps.  The first iteration would get the
notice where 'AlertNotice/id=5' and the second iteration would get all the notices where 'AlertNotice/id=5|AlertNotice/notification_state=DELIVERED'.
 The results would be correct but you are making 2 queries where just the second would do.

For 'AlertNotice/id=5&AlertNotice/notification_state=DELIVERED'. The call to getPropertyMaps(predicate)
would return 1 map.  The single query would be for 'AlertNotice/id=5' so the second part of
the predicate gets ignored.  The results would be correctly filtered down the line but the
RP is potentially returning more than the desrired set.

For 'AlertNotice/notification_state=PENDING|AlertNotice/notification_state=DELIVERED'.  The
call to getPropertyMaps(predicate) would return 2 maps.  Each iteration would make the same
JPA query with the entire predicate.  Again, the results would be correct but you are making
2 queries where just one would do.

Maybe these are not real world situations so feel free to ignore the comment if you feel that
is best.  Just curious, what if you got rid of the getPropertyMaps() call and the for loop
and just made a single query based on the entire predicate?  Would the results be correct?


- Tom


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


On Oct. 15, 2014, 12:35 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26744/
> -----------------------------------------------------------
> 
> (Updated Oct. 15, 2014, 12:35 p.m.)
> 
> 
> Review request for Ambari, Nate Cole and Tom Beerbower.
> 
> 
> Bugs: AMBARI-7778
>     https://issues.apache.org/jira/browse/AMBARI-7778
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> For every outbound notification, Ambari currently keeps track of the dispatch state (PENDING,
DELIVERED, FAILURE). This data needs to be exposed via the REST APIs so that it can be queried.
> 
> {code}
> http://localhost:8080/api/v1/clusters/c1/alert_notices?fields=*
> http://localhost:8080/api/v1/clusters/c1/alert_notices?AlertNotice/notification_state=DELIVERED&fields=*
> 
> {
>   "href" : "http://localhost:8080/api/v1/clusters/c1/alert_notices?fields=*",
>   "items" : [
>     {
>       "href" : "http://localhost:8080/api/v1/clusters/c1/alert_notices/1",
>       "AlertNotice" : {
>         "cluster_name" : "c1",
>         "history_id" : 1,
>         "id" : 1,
>         "notification_state" : "DELIVERED",
>         "service_name" : "HDFS",
>         "target_id" : 1,
>         "target_name" : "Administrators",
>         "uuid" : "106ecdb4-0970-4c50-22d3-706d53571321"
>       }
>     },
>     {
>       "href" : "http://localhost:8080/api/v1/clusters/c1/alert_notices/2",
>       "AlertNotice" : {
>         "cluster_name" : "c1",
>         "history_id" : 2,
>         "id" : 2,
>         "notification_state" : "DELIVERED",
>         "service_name" : "HDFS",
>         "target_id" : 1,
>         "target_name" : "Administrators",
>         "uuid" : "fffecdb4-0970-4dd0-22d3-706d53571321"
>       }
>     }
>   ]
> }
> {code}
> 
> Same as with history, pagination and sorting will be done together tracked by another
single Jira.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertNoticeResourceDefinition.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java
c7a3277 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertNoticeService.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java
e039b6a 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/HostService.java
7962ee3 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ServiceService.java
7371fad 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AlertNoticeRequest.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java d428b80

>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java
05ae105 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java
40bcb62 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java 4fb8319

>   ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertReceivedListener.java
45d0734 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java
2239c8f 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertNoticeEntity_.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertTargetEntity_.java
PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProviderTest.java
PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/AlertDaoHelper.java PRE-CREATION

>   ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java
015acc0 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java c3da07f

> 
> Diff: https://reviews.apache.org/r/26744/diff/
> 
> 
> Testing
> -------
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 23:05 min
> [INFO] Finished at: 2014-10-14T20:28:54-04:00
> [INFO] Final Memory: 29M/237M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


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