AMBARI-3811 - API performance issue on large cluster

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

@@ -23,7 +23,6 @@ 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;
@@ -67,9 +66,9 @@ public class QueryImpl implements Query, ResourceInstance {
   private final Map<Resource.Type, String> keyValueMap = new HashMap<Resource.Type, String>();
 
   /**
-   * Set of maps of primary and foreign key values.
+   * Set of query results.
    */
-  Set<Map<Resource.Type, String>> keyValueMaps = new HashSet<Map<Resource.Type, String>>();
+  Set<QueryResult> queryResults = new LinkedHashSet<QueryResult>();
 
   /**
    * Sub-resources of the resource which is being operated on.
@@ -97,11 +96,6 @@ public class QueryImpl implements Query, ResourceInstance {
    */
   private PageRequest pageRequest;
 
-  /**
-   * Query resources.
-   */
-  private Set<Resource> providerResourceSet;
-
   /**
    * The logger.
    */
@@ -303,14 +297,21 @@ public class QueryImpl implements Query, ResourceInstance {
       NoSuchResourceException,
       NoSuchParentResourceException {
 
-    providerResourceSet = new LinkedHashSet<Resource>();
+    Set<Resource> providerResourceSet = new LinkedHashSet<Resource>();
+
+    Resource.Type resourceType = getResourceDefinition().getType();
+    Request       request      = createRequest();
+    Predicate     predicate    = getPredicate();
+
+    Set<Resource> resourceSet = new LinkedHashSet<Resource>();
 
-    // save the top level resources
-    for (Resource resource : doQuery(getPredicate())) {
-      providerResourceSet.add(resource);
+    for (Resource queryResource : doQuery(resourceType, request, predicate)) {
+      providerResourceSet.add(queryResource);
+      resourceSet.add(queryResource);
     }
-    keyValueMaps.add(getKeyValueMap());
+    queryResults.add(new QueryResult(request, predicate, getKeyValueMap(), resourceSet));
 
+    clusterController.populateResources(resourceType, providerResourceSet, request, predicate);
     queryForSubResources();
   }
 
@@ -325,17 +326,31 @@ public class QueryImpl implements Query, ResourceInstance {
       NoSuchResourceException,
       NoSuchParentResourceException {
 
-    // get predicates for all of the sub resource types
-    Map<String, Predicate> predicateMap = getSubResourcePredicates(providerResourceSet);
-
     for (Map.Entry<String, QueryImpl> entry : querySubResourceSet.entrySet()) {
-      QueryImpl subResource  = entry.getValue();
 
-      subResource.providerResourceSet = new LinkedHashSet<Resource>();
+      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()) {
+
+          Map<Resource.Type, String> map = getKeyValueMap(resource, queryResult.getKeyValueMap());
 
-      for (Resource resource : subResource.doQuery(predicateMap.get(entry.getKey()))) {
-        subResource.providerResourceSet.add(resource);
+          Predicate predicate = subResource.createPredicate(map);
+
+          Set<Resource> resourceSet = 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));
+        }
       }
+      clusterController.populateResources(resourceType, providerResourceSet, request, null);
       subResource.queryForSubResources();
     }
   }
@@ -343,59 +358,24 @@ public class QueryImpl implements Query, ResourceInstance {
   /**
    * Query the cluster controller for the resources.
    */
-  private Set<Resource> doQuery(Predicate predicate)
+  private Set<Resource> doQuery(Resource.Type type, Request request, Predicate predicate)
       throws UnsupportedPropertyException,
       SystemException,
       NoSuchResourceException,
       NoSuchParentResourceException {
 
-    Resource.Type resourceType1 = getResourceDefinition().getType();
-
-    if (getKeyValueMap().get(resourceType1) == null) {
-      addCollectionProperties(resourceType1);
+    if (getKeyValueMap().get(type) == null) {
+      addCollectionProperties(type);
     }
     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(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;
+    return clusterController.getResources(type, request, predicate);
   }
 
   /**
@@ -406,40 +386,44 @@ public class QueryImpl implements Query, ResourceInstance {
 
     Result result = new ResultImpl(true);
     Resource.Type resourceType = getResourceDefinition().getType();
-    if (getKeyValueMap().get(resourceType) == null) {
-      result.getResultTree().setProperty("isCollection", "true");
+    TreeNode<Resource> tree = result.getResultTree();
+
+    if (isCollectionResource()) {
+      tree.setProperty("isCollection", "true");
     }
 
-    Predicate predicate = createPredicate();
-    Request   request   = createRequest();
+    for (QueryResult queryResult : queryResults) {
+      Predicate predicate = queryResult.getPredicate();
+      Request   request   = queryResult.getRequest();
 
-    Iterable<Resource> iterResource;
+      Iterable<Resource> iterResource;
 
-    if (pageRequest == null) {
-      iterResource = clusterController.getIterable(
-          resourceType, providerResourceSet, request, predicate);
-    } else {
-      PageResponse pageResponse = clusterController.getPage(
-          resourceType, providerResourceSet, request, predicate, pageRequest);
-      iterResource = pageResponse.getIterable();
-    }
+      Set<Resource> providerResourceSet = queryResult.getProviderResourceSet();
 
-    TreeNode<Resource> tree = result.getResultTree();
+      if (pageRequest == null) {
+        iterResource = clusterController.getIterable(
+            resourceType, providerResourceSet, request, predicate);
+      } else {
+        PageResponse pageResponse = clusterController.getPage(
+            resourceType, providerResourceSet, request, predicate, pageRequest);
+        iterResource = pageResponse.getIterable();
+      }
 
-    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;
@@ -567,18 +551,6 @@ 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) {
@@ -601,4 +573,45 @@ 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,18 +113,23 @@ public class ClusterControllerImpl implements ClusterController {
 
       // get the resources
       resources = provider.getResources(request, predicate);
+    }
+    return resources;
+  }
 
-      // populate the resources with metrics and properties.
-      populateResources(type, resources, request, predicate);
+  @Override
+  public Set<Resource> populateResources(Resource.Type type,
+                                         Set<Resource> resources,
+                                         Request request,
+                                         Predicate predicate) throws SystemException {
+    Set<Resource> keepers = resources;
 
-      // 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);
+    for (PropertyProvider propertyProvider : propertyProviders.get(type)) {
+      if (providesRequestProperties(propertyProvider, request, predicate)) {
+        keepers = propertyProvider.populateResources(keepers, request, predicate);
       }
     }
-    return resources;
+    return keepers;
   }
 
   @Override
@@ -279,7 +284,8 @@ public class ClusterControllerImpl implements ClusterController {
       SystemException,
       NoSuchParentResourceException,
       NoSuchResourceException {
-    return getResources(type, request, predicate, null).getIterable();
+    PageResponse resources = getResources(type, request, predicate, null);
+    return resources.getIterable();
   }
 
   /**
@@ -414,34 +420,6 @@ 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,6 +51,25 @@ 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.