> 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 > >
