Revert "AMBARI-3811 - API performance issue on large cluster"

This reverts commit e311d66c6ed2f7b880d90e445ca67706779e0bd1.

ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java

@@ -23,6 +23,7 @@ import org.apache.ambari.server.api.resources.ResourceInstance;
 import org.apache.ambari.server.api.resources.ResourceInstanceFactoryImpl;
 import org.apache.ambari.server.api.resources.SubResourceDefinition;
 import org.apache.ambari.server.api.services.ResultImpl;
+import org.apache.ambari.server.controller.predicate.OrPredicate;
 import org.apache.ambari.server.controller.utilities.PropertyHelper;
 import org.apache.ambari.server.controller.predicate.AndPredicate;
 import org.apache.ambari.server.controller.predicate.EqualsPredicate;
@@ -66,9 +67,9 @@ public class QueryImpl implements Query, ResourceInstance {
   private final Map<Resource.Type, String> keyValueMap = new HashMap<Resource.Type, String>();
 
   /**
-   * Set of query results.
+   * Set of maps of primary and foreign key values.
    */
-  Set<QueryResult> queryResults = new LinkedHashSet<QueryResult>();
+  Set<Map<Resource.Type, String>> keyValueMaps = new HashSet<Map<Resource.Type, String>>();
 
   /**
    * Sub-resources of the resource which is being operated on.
@@ -96,6 +97,11 @@ public class QueryImpl implements Query, ResourceInstance {
    */
   private PageRequest pageRequest;
 
+  /**
+   * Query resources.
+   */
+  private Set<Resource> providerResourceSet;
+
   /**
    * The logger.
    */
@@ -297,21 +303,14 @@ public class QueryImpl implements Query, ResourceInstance {
       NoSuchResourceException,
       NoSuchParentResourceException {
 
-    Set<Resource> providerResourceSet = new LinkedHashSet<Resource>();
-
-    Resource.Type resourceType = getResourceDefinition().getType();
-    Request       request      = createRequest();
-    Predicate     predicate    = getPredicate();
-
-    Set<Resource> resourceSet = new LinkedHashSet<Resource>();
+    providerResourceSet = new LinkedHashSet<Resource>();
 
-    for (Resource queryResource : doQuery(resourceType, request, predicate)) {
-      providerResourceSet.add(queryResource);
-      resourceSet.add(queryResource);
+    // save the top level resources
+    for (Resource resource : doQuery(getPredicate())) {
+      providerResourceSet.add(resource);
     }
-    queryResults.add(new QueryResult(request, predicate, getKeyValueMap(), resourceSet));
+    keyValueMaps.add(getKeyValueMap());
 
-    clusterController.populateResources(resourceType, providerResourceSet, request, predicate);
     queryForSubResources();
   }
 
@@ -326,31 +325,17 @@ public class QueryImpl implements Query, ResourceInstance {
       NoSuchResourceException,
       NoSuchParentResourceException {
 
-    for (Map.Entry<String, QueryImpl> entry : querySubResourceSet.entrySet()) {
-
-      QueryImpl     subResource  = entry.getValue();
-      Resource.Type resourceType = subResource.getResourceDefinition().getType();
-      Request       request      = subResource.createRequest();
-
-      Set<Resource> providerResourceSet = new LinkedHashSet<Resource>();
-
-      for (QueryResult queryResult : queryResults) {
-        for (Resource resource : queryResult.getProviderResourceSet()) {
+    // get predicates for all of the sub resource types
+    Map<String, Predicate> predicateMap = getSubResourcePredicates(providerResourceSet);
 
-          Map<Resource.Type, String> map = getKeyValueMap(resource, queryResult.getKeyValueMap());
-
-          Predicate predicate = subResource.createPredicate(map);
+    for (Map.Entry<String, QueryImpl> entry : querySubResourceSet.entrySet()) {
+      QueryImpl subResource  = entry.getValue();
 
-          Set<Resource> resourceSet = new LinkedHashSet<Resource>();
+      subResource.providerResourceSet = new LinkedHashSet<Resource>();
 
-          for (Resource queryResource : subResource.doQuery(resourceType, request, predicate)) {
-            providerResourceSet.add(queryResource);
-            resourceSet.add(queryResource);
-          }
-          subResource.queryResults.add(new QueryResult(request, predicate, map, resourceSet));
-        }
+      for (Resource resource : subResource.doQuery(predicateMap.get(entry.getKey()))) {
+        subResource.providerResourceSet.add(resource);
       }
-      clusterController.populateResources(resourceType, providerResourceSet, request, null);
       subResource.queryForSubResources();
     }
   }
@@ -358,24 +343,59 @@ public class QueryImpl implements Query, ResourceInstance {
   /**
    * Query the cluster controller for the resources.
    */
-  private Set<Resource> doQuery(Resource.Type type, Request request, Predicate predicate)
+  private Set<Resource> doQuery(Predicate predicate)
       throws UnsupportedPropertyException,
       SystemException,
       NoSuchResourceException,
       NoSuchParentResourceException {
 
-    if (getKeyValueMap().get(type) == null) {
-      addCollectionProperties(type);
+    Resource.Type resourceType1 = getResourceDefinition().getType();
+
+    if (getKeyValueMap().get(resourceType1) == null) {
+      addCollectionProperties(resourceType1);
     }
     if (queryPropertySet.isEmpty() && querySubResourceSet.isEmpty()) {
       //Add sub resource properties for default case where no fields are specified.
       querySubResourceSet.putAll(ensureSubResources());
     }
 
+    Resource.Type resourceType = getResourceDefinition().getType();
+    Request       request      = createRequest();
+
     if (LOG.isDebugEnabled()) {
       LOG.debug("Executing resource query: " + request + " where " + predicate);
     }
-    return clusterController.getResources(type, request, predicate);
+    return clusterController.getResources(resourceType, request, predicate);
+  }
+
+  /**
+   * Get a map of predicates for the given resource's sub-resources keyed 
+   * by resource type.  Each predicate is a combined predicate of all 
+   * the sub resource predicates for a given type OR'd together.  This 
+   * allows for all of the sub-resources of a given type to be 
+   * acquired in a single query.
+   */
+  private Map<String, Predicate> getSubResourcePredicates(Set<Resource> resources) {
+    Map<String, Predicate> predicateMap = new HashMap<String, Predicate>();
+
+    for (Resource resource : resources) {
+      for (Map.Entry<String, QueryImpl> entry : querySubResourceSet.entrySet()) {
+        QueryImpl subResourceInstance = entry.getValue();
+        String    subResCategory      = entry.getKey();
+
+        Set<Map<Resource.Type, String>> resourceKeyValueMaps = getKeyValueMaps(resource, keyValueMaps);
+
+        for( Map<Resource.Type, String> map : resourceKeyValueMaps) {
+          Predicate predicate = predicateMap.get(subResCategory);
+
+          predicateMap.put(subResCategory, predicate == null ?
+              subResourceInstance.createPredicate(map) :
+              new OrPredicate(predicate, subResourceInstance.createPredicate(map)));
+        }
+        subResourceInstance.keyValueMaps.addAll(resourceKeyValueMaps);
+      }
+    }
+    return predicateMap;
   }
 
   /**
@@ -386,44 +406,40 @@ public class QueryImpl implements Query, ResourceInstance {
 
     Result result = new ResultImpl(true);
     Resource.Type resourceType = getResourceDefinition().getType();
-    TreeNode<Resource> tree = result.getResultTree();
-
-    if (isCollectionResource()) {
-      tree.setProperty("isCollection", "true");
+    if (getKeyValueMap().get(resourceType) == null) {
+      result.getResultTree().setProperty("isCollection", "true");
     }
 
-    for (QueryResult queryResult : queryResults) {
-      Predicate predicate = queryResult.getPredicate();
-      Request   request   = queryResult.getRequest();
+    Predicate predicate = createPredicate();
+    Request   request   = createRequest();
 
-      Iterable<Resource> iterResource;
+    Iterable<Resource> iterResource;
 
-      Set<Resource> providerResourceSet = queryResult.getProviderResourceSet();
+    if (pageRequest == null) {
+      iterResource = clusterController.getIterable(
+          resourceType, providerResourceSet, request, predicate);
+    } else {
+      PageResponse pageResponse = clusterController.getPage(
+          resourceType, providerResourceSet, request, predicate, pageRequest);
+      iterResource = pageResponse.getIterable();
+    }
 
-      if (pageRequest == null) {
-        iterResource = clusterController.getIterable(
-            resourceType, providerResourceSet, request, predicate);
-      } else {
-        PageResponse pageResponse = clusterController.getPage(
-            resourceType, providerResourceSet, request, predicate, pageRequest);
-        iterResource = pageResponse.getIterable();
-      }
+    TreeNode<Resource> tree = result.getResultTree();
 
-      int count = 1;
-      for (Resource resource : iterResource) {
-        // add a child node for the resource and provide a unique name.  The name is never used.
-        TreeNode<Resource> node = tree.addChild(resource, resource.getType() + ":" + count++);
-        for (Map.Entry<String, QueryImpl> entry : querySubResourceSet.entrySet()) {
-          String    subResCategory = entry.getKey();
-          QueryImpl subResource    = entry.getValue();
+    int count = 1;
+    for (Resource resource : iterResource) {
+      // add a child node for the resource and provide a unique name.  The name is never used.
+      TreeNode<Resource> node = tree.addChild(resource, resource.getType() + ":" + count++);
+      for (Map.Entry<String, QueryImpl> entry : querySubResourceSet.entrySet()) {
+        String    subResCategory = entry.getKey();
+        QueryImpl subResource    = entry.getValue();
 
-          subResource.setKeyValueMap(getKeyValueMap(resource, getKeyValueMap()));
+        subResource.setKeyValueMap(getKeyValueMap(resource, getKeyValueMap()));
 
-          TreeNode<Resource> childResult = subResource.getResult().getResultTree();
-          childResult.setName(subResCategory);
-          childResult.setProperty("isCollection", "false");
-          node.addChild(childResult);
-        }
+        TreeNode<Resource> childResult = subResource.getResult().getResultTree();
+        childResult.setName(subResCategory);
+        childResult.setProperty("isCollection", "false");
+        node.addChild(childResult);
       }
     }
     return result;
@@ -551,6 +567,18 @@ public class QueryImpl implements Query, ResourceInstance {
         Collections.<String>emptySet() : setProperties, mapTemporalInfo);
   }
 
+  // Get a set of key value maps based on the given resource and an existing set of key value maps
+  private Set<Map<Resource.Type, String>> getKeyValueMaps(Resource resource,
+                                                          Set<Map<Resource.Type, String>> keyValueMaps) {
+    Set<Map<Resource.Type, String>> resourceKeyValueMaps = new HashSet<Map<Resource.Type, String>>();
+
+    for(Map<Resource.Type, String> keyValueMap : keyValueMaps) {
+      Map<Resource.Type, String> resourceKeyValueMap = getKeyValueMap(resource, keyValueMap);
+      resourceKeyValueMaps.add(resourceKeyValueMap);
+    }
+    return resourceKeyValueMaps;
+  }
+
   // Get a key value map based on the given resource and an existing key value map
   private Map<Resource.Type, String> getKeyValueMap(Resource resource,
                                                     Map<Resource.Type, String> keyValueMap) {
@@ -573,45 +601,4 @@ public class QueryImpl implements Query, ResourceInstance {
     resourceKeyValueMap.put(resource.getType(), resource.getPropertyValue(resourceKeyProp).toString());
     return resourceKeyValueMap;
   }
-
-  // ----- inner class : QueryResult -----------------------------------------
-
-  /**
-   * Maintain information about an individual query and its result.
-   */
-  private static class QueryResult {
-    private final Request request;
-    private final Predicate predicate;
-    private final Map<Resource.Type, String> keyValueMap;
-    private final Set<Resource> providerResourceSet;
-
-    // ----- Constructor -----------------------------------------------------
-
-    private QueryResult(Request request, Predicate predicate,
-                        Map<Resource.Type, String> keyValueMap,
-                        Set<Resource> providerResourceSet) {
-      this.request = request;
-      this.predicate = predicate;
-      this.keyValueMap = keyValueMap;
-      this.providerResourceSet = providerResourceSet;
-    }
-
-    // ----- accessors -------------------------------------------------------
-
-    public Request getRequest() {
-      return request;
-    }
-
-    public Predicate getPredicate() {
-      return predicate;
-    }
-
-    public Map<Resource.Type, String> getKeyValueMap() {
-      return keyValueMap;
-    }
-
-    public Set<Resource> getProviderResourceSet() {
-      return providerResourceSet;
-    }
-  }
 }

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java

@@ -113,23 +113,18 @@ public class ClusterControllerImpl implements ClusterController {
 
       // get the resources
       resources = provider.getResources(request, predicate);
-    }
-    return resources;
-  }
 
-  @Override
-  public Set<Resource> populateResources(Resource.Type type,
-                                         Set<Resource> resources,
-                                         Request request,
-                                         Predicate predicate) throws SystemException {
-    Set<Resource> keepers = resources;
+      // populate the resources with metrics and properties.
+      populateResources(type, resources, request, predicate);
 
-    for (PropertyProvider propertyProvider : propertyProviders.get(type)) {
-      if (providesRequestProperties(propertyProvider, request, predicate)) {
-        keepers = propertyProvider.populateResources(keepers, request, predicate);
+      // filter the fully populated resources with the given predicate
+      Iterable<Resource> iterable = getIterable(type, resources, request, predicate);
+      resources = new LinkedHashSet<Resource>();
+      for (Resource resource : iterable){
+        resources.add(resource);
       }
     }
-    return keepers;
+    return resources;
   }
 
   @Override
@@ -284,8 +279,7 @@ public class ClusterControllerImpl implements ClusterController {
       SystemException,
       NoSuchParentResourceException,
       NoSuchResourceException {
-    PageResponse resources = getResources(type, request, predicate, null);
-    return resources.getIterable();
+    return getResources(type, request, predicate, null).getIterable();
   }
 
   /**
@@ -420,6 +414,34 @@ public class ClusterControllerImpl implements ClusterController {
     return pbWithPredicate == null ? null : pbWithPredicate.toPredicate();
   }
 
+  /**
+   * Populate the given resources from the associated property providers.  This
+   * method may filter the resources based on the predicate and return a subset
+   * of the given resources.
+   *
+   * @param type       the resource type
+   * @param resources  the resources to be populated
+   * @param request    the request
+   * @param predicate  the predicate
+   *
+   * @return the set of resources that were successfully populated
+   *
+   * @throws SystemException if unable to populate the resources
+   */
+  protected Set<Resource> populateResources(Resource.Type type,
+                                         Set<Resource> resources,
+                                         Request request,
+                                         Predicate predicate) throws SystemException {
+    Set<Resource> keepers = resources;
+
+    for (PropertyProvider propertyProvider : propertyProviders.get(type)) {
+      if (providesRequestProperties(propertyProvider, request, predicate)) {
+        keepers = propertyProvider.populateResources(keepers, request, predicate);
+      }
+    }
+    return keepers;
+  }
+
   /**
    * Indicates whether or not the given property provider can service the given request.
    *

ambari-server/src/main/java/org/apache/ambari/server/controller/spi/ClusterController.java

@@ -51,25 +51,6 @@ public interface ClusterController {
       NoSuchParentResourceException,
       SystemException;
 
-  /**
-   * Populate the given resources from the associated property providers.  This
-   * method may filter the resources based on the predicate and return a subset
-   * of the given resources.
-   *
-   * @param type       the resource type
-   * @param resources  the resources to be populated
-   * @param request    the request
-   * @param predicate  the predicate
-   *
-   * @return the set of resources that were successfully populated
-   *
-   * @throws SystemException if unable to populate the resources
-   */
-  public Set<Resource> populateResources(Resource.Type type,
-                                            Set<Resource> resources,
-                                            Request request,
-                                            Predicate predicate) throws SystemException;
-
   /**
    * Get an iterable set of resources from the given set of resources filtered by the
    * given request and predicate objects.