Repository: ambari
Updated Branches:
  refs/heads/branch-2.0.0 8dbdbf66d -> 1774c9f7f


AMBARI-10050 - Querying For Requests By Task Status Has Poor Performance 
(jonathanhurley)


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/1774c9f7
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/1774c9f7
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/1774c9f7

Branch: refs/heads/branch-2.0.0
Commit: 1774c9f7fbd3226069328ad691b7a272efd02629
Parents: 8dbdbf6
Author: Jonathan Hurley <jhur...@hortonworks.com>
Authored: Thu Mar 12 14:49:22 2015 -0400
Committer: Jonathan Hurley <jhur...@hortonworks.com>
Committed: Thu Mar 12 16:27:36 2015 -0400

----------------------------------------------------------------------
 .../actionmanager/ActionDBAccessorImpl.java     | 42 +++++-----
 .../server/actionmanager/HostRoleStatus.java    | 23 +++---
 .../ambari/server/controller/AmbariServer.java  | 13 +--
 .../server/orm/dao/HostRoleCommandDAO.java      | 83 +++++++++++---------
 .../ambari/server/orm/dao/RequestDAO.java       | 48 ++++++++---
 .../server/state/cluster/ClustersImpl.java      |  4 -
 .../ambari/server/orm/dao/RequestDAOTest.java   |  8 +-
 7 files changed, 130 insertions(+), 91 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/1774c9f7/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
index bd4f2d8..df75c0c 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
@@ -18,15 +18,13 @@
 package org.apache.ambari.server.actionmanager;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.EnumSet;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.ambari.server.AmbariException;
@@ -601,26 +599,26 @@ public class ActionDBAccessorImpl implements 
ActionDBAccessor {
   public List<Long> getRequestsByStatus(RequestStatus status, int maxResults,
     boolean ascOrder) {
 
-    boolean match = true;
-    boolean checkAllTasks = false;
-    Set<HostRoleStatus> statuses = new HashSet<HostRoleStatus>();
-    if (status == RequestStatus.IN_PROGRESS) {
-      statuses.addAll(Arrays.asList(HostRoleStatus.PENDING,
-          HostRoleStatus.IN_PROGRESS, HostRoleStatus.QUEUED,
-          HostRoleStatus.HOLDING, HostRoleStatus.HOLDING_FAILED, 
HostRoleStatus.HOLDING_TIMEDOUT));
-    } else if (status == RequestStatus.COMPLETED) {
-      match = false;
-      checkAllTasks = true;
-      statuses.addAll(Arrays.asList(HostRoleStatus.PENDING,
-          HostRoleStatus.IN_PROGRESS, HostRoleStatus.QUEUED,
-          HostRoleStatus.ABORTED, HostRoleStatus.FAILED,
-          HostRoleStatus.TIMEDOUT));
-    } else if (status == RequestStatus.FAILED) {
-      statuses.addAll(Arrays.asList(HostRoleStatus.ABORTED,
-          HostRoleStatus.FAILED, HostRoleStatus.TIMEDOUT));
+    if (null == status) {
+      return requestDAO.findAllRequestIds(maxResults, ascOrder);
     }
-    return hostRoleCommandDAO.getRequestsByTaskStatus(statuses, match,
-      checkAllTasks, maxResults, ascOrder);
+
+    EnumSet<HostRoleStatus> taskStatuses = null;
+    switch( status ){
+      case IN_PROGRESS:
+        taskStatuses = HostRoleStatus.IN_PROGRESS_STATUSES;
+        break;
+      case FAILED:
+        taskStatuses = HostRoleStatus.FAILED_STATUSES;
+        break;
+      case COMPLETED:
+        // !!! COMPLETED is special as all tasks in the request must be
+        // completed
+        return hostRoleCommandDAO.getCompletedRequests(maxResults, ascOrder);
+    }
+
+    return hostRoleCommandDAO.getRequestsByTaskStatus(taskStatuses, maxResults,
+        ascOrder);
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/ambari/blob/1774c9f7/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleStatus.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleStatus.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleStatus.java
index 7c7e8cd..39cbabc 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleStatus.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleStatus.java
@@ -65,11 +65,17 @@ public enum HostRoleStatus {
   ABORTED;
 
   private static List<HostRoleStatus> COMPLETED_STATES = Arrays.asList(FAILED, 
TIMEDOUT, ABORTED, COMPLETED);
-  private static List<HostRoleStatus> FAILED_STATES = Arrays.asList(FAILED, 
TIMEDOUT, ABORTED);
   private static List<HostRoleStatus> HOLDING_STATES = Arrays.asList(HOLDING, 
HOLDING_FAILED, HOLDING_TIMEDOUT);
 
   /**
    * The {@link HostRoleStatus}s that represent any commands which are
+   * considered to be "Failed".
+   */
+  public static EnumSet<HostRoleStatus> FAILED_STATUSES = EnumSet.of(FAILED,
+      TIMEDOUT, ABORTED);
+
+  /**
+   * The {@link HostRoleStatus}s that represent any commands which are
    * considered to be "In Progress".
    */
   public static final EnumSet<HostRoleStatus> IN_PROGRESS_STATUSES = 
EnumSet.of(
@@ -78,12 +84,17 @@ public enum HostRoleStatus {
       HostRoleStatus.HOLDING_FAILED, HostRoleStatus.HOLDING_TIMEDOUT);
 
   /**
+   * The {@link HostRoleStatus}s that represent all non-completed states.
+   */
+  public static final EnumSet<HostRoleStatus> NOT_COMPLETED_STATUSES = 
EnumSet.complementOf(EnumSet.of(COMPLETED));
+
+  /**
    * Indicates whether or not it is a valid failure state.
    *
    * @return true if this is a valid failure state.
    */
   public boolean isFailedState() {
-    return FAILED_STATES.contains(this);
+    return FAILED_STATUSES.contains(this);
   }
 
   /**
@@ -118,14 +129,6 @@ public enum HostRoleStatus {
   }
 
   /**
-   *
-   * @return list of failed states
-   */
-  public static List<HostRoleStatus> getFailedStates() {
-    return Collections.unmodifiableList(FAILED_STATES);
-  }
-
-  /**
    * @return {@code true} if this is a status that is in progress
    */
   public boolean isInProgress() {

http://git-wip-us.apache.org/repos/asf/ambari/blob/1774c9f7/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
index 3a74bda..9b340ee 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
@@ -396,11 +396,14 @@ public class AmbariServer {
                     "org.apache.ambari.server.api.AmbariCsrfProtectionFilter");
       }
 
-      //Set jetty thread pool
-      serverForAgent.setThreadPool(
-          new QueuedThreadPool(configs.getAgentThreadPoolSize()));
-      server.setThreadPool(
-          new QueuedThreadPool(configs.getClientThreadPoolSize()));
+      // Set jetty thread pool
+      QueuedThreadPool qtp = new 
QueuedThreadPool(configs.getAgentThreadPoolSize());
+      qtp.setName("qtp-ambari-agent");
+      serverForAgent.setThreadPool(qtp);
+
+      qtp = new QueuedThreadPool(configs.getClientThreadPoolSize());
+      qtp.setName("qtp-client");
+      server.setThreadPool(qtp);
 
       /* Configure the API server to use the NIO connectors */
       SelectChannelConnector apiConnector;

http://git-wip-us.apache.org/repos/asf/ambari/blob/1774c9f7/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java
index f9d21ad..f927197 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java
@@ -21,6 +21,7 @@ package org.apache.ambari.server.orm.dao;
 import static org.apache.ambari.server.orm.DBAccessor.DbType.ORACLE;
 import static org.apache.ambari.server.orm.dao.DaoUtils.ORACLE_LIST_LIMIT;
 
+import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -66,6 +67,18 @@ public class HostRoleCommandDAO {
       " GROUP BY hrc.requestId, hrc.stageId HAVING hrc.requestId = :requestId",
       HostRoleCommandStatusSummaryDTO.class.getName());
 
+  /**
+   * SQL template to get requests that have at least one task in any of the
+   * specified statuses.
+   */
+  private static final String REQUESTS_BY_TASK_STATUS_SQL = "SELECT DISTINCT 
task.requestId FROM HostRoleCommandEntity task WHERE task.status IN 
:taskStatuses ORDER BY task.requestId {0}";
+
+  /**
+   * SQL template to get all requests which have had all of their tasks
+   * COMPLETED
+   */
+  private static final String COMPLETED_REQUESTS_SQL = "SELECT DISTINCT 
task.requestId FROM HostRoleCommandEntity task WHERE NOT EXISTS (SELECT 
task.requestId FROM HostRoleCommandEntity task WHERE task.status IN 
:notCompletedStatuses) ORDER BY task.requestId {0}";
+
   @Inject
   Provider<EntityManager> entityManagerProvider;
   @Inject
@@ -278,47 +291,45 @@ public class HostRoleCommandDAO {
     return daoUtils.selectAll(entityManagerProvider.get(), 
HostRoleCommandEntity.class);
   }
 
+  /**
+   * Gets requests that have tasks in any of the specified statuses.
+   *
+   * @param statuses
+   * @param maxResults
+   * @param ascOrder
+   * @return
+   */
   @RequiresSession
-  public List<Long> getRequestsByTaskStatus(Collection<HostRoleStatus> 
statuses,
-    boolean match, boolean checkAllTasks, int maxResults, boolean ascOrder) {
-
-    List<Long> results;
-    StringBuilder queryStr = new StringBuilder();
-
-    queryStr.append("SELECT DISTINCT command.requestId ").append(
-      "FROM HostRoleCommandEntity command ");
-    if (statuses != null && !statuses.isEmpty()) {
-      queryStr.append("WHERE ");
-
-      if (checkAllTasks) {
-        queryStr.append("command.requestId ");
-        if (!match) {
-          queryStr.append("NOT ");
-        }
-        queryStr.append("IN (").append("SELECT c.requestId ")
-          .append("FROM HostRoleCommandEntity c ")
-          .append("WHERE c.requestId = command.requestId ")
-          .append("AND c.status IN ?1) ");
-      } else {
-        queryStr.append("command.status ");
-        if (!match) {
-          queryStr.append("NOT ");
-        }
-        queryStr.append("IN ?1 ");
-      }
+  public List<Long> getRequestsByTaskStatus(
+      Collection<HostRoleStatus> statuses, int maxResults, boolean ascOrder) {
+    String sortOrder = "ASC";
+    if (!ascOrder) {
+      sortOrder = "DESC";
     }
 
-    queryStr.append("ORDER BY command.requestId ").append(ascOrder ? "ASC" : 
"DESC");
-    TypedQuery<Long> query = 
entityManagerProvider.get().createQuery(queryStr.toString(),
-      Long.class);
-    query.setMaxResults(maxResults);
+    String sql = MessageFormat.format(REQUESTS_BY_TASK_STATUS_SQL, sortOrder);
+    TypedQuery<Long> query = entityManagerProvider.get().createQuery(sql,
+        Long.class);
 
-    if (statuses != null && !statuses.isEmpty()) {
-      results = daoUtils.selectList(query, statuses);
-    } else {
-      results = daoUtils.selectList(query);
+    query.setParameter("taskStatuses", statuses);
+    return daoUtils.selectList(query);
+  }
+
+  @RequiresSession
+  public List<Long> getCompletedRequests(int maxResults, boolean ascOrder) {
+    String sortOrder = "ASC";
+    if (!ascOrder) {
+      sortOrder = "DESC";
     }
-    return results;
+
+    String sql = MessageFormat.format(COMPLETED_REQUESTS_SQL, sortOrder);
+    TypedQuery<Long> query = entityManagerProvider.get().createQuery(sql,
+        Long.class);
+
+    query.setParameter("notCompletedStatuses",
+        HostRoleStatus.NOT_COMPLETED_STATUSES);
+
+    return daoUtils.selectList(query);
   }
 
   @Transactional

http://git-wip-us.apache.org/repos/asf/ambari/blob/1774c9f7/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RequestDAO.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RequestDAO.java 
b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RequestDAO.java
index 9d538f7..96510b7 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RequestDAO.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RequestDAO.java
@@ -18,27 +18,34 @@
 
 package org.apache.ambari.server.orm.dao;
 
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-import com.google.inject.Singleton;
-import com.google.inject.persist.Transactional;
+import java.text.MessageFormat;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+import javax.persistence.EntityManager;
+import javax.persistence.TypedQuery;
 
 import org.apache.ambari.server.actionmanager.HostRoleStatus;
 import org.apache.ambari.server.orm.RequiresSession;
 import org.apache.ambari.server.orm.entities.RequestEntity;
 import org.apache.ambari.server.orm.entities.RequestResourceFilterEntity;
 
-import javax.persistence.EntityManager;
-import javax.persistence.TypedQuery;
-
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.google.inject.persist.Transactional;
 
 @Singleton
 public class RequestDAO {
+  /**
+   * SQL template to retrieve all request IDs, sorted by the ID.
+   */
+  private final static String REQUEST_IDS_SORTED_SQL = "SELECT 
request.requestId FROM RequestEntity request ORDER BY request.requestId {0}";
+
   @Inject
   Provider<EntityManager> entityManagerProvider;
+
   @Inject
   DaoUtils daoUtils;
 
@@ -49,9 +56,10 @@ public class RequestDAO {
 
   @RequiresSession
   public List<RequestEntity> findByPks(Collection<Long> requestIds) {
-    if (null == requestIds || 0 == requestIds.size())
+    if (null == requestIds || 0 == requestIds.size()) {
       return Collections.emptyList();
-    
+    }
+
     TypedQuery<RequestEntity> query = 
entityManagerProvider.get().createQuery("SELECT request FROM RequestEntity 
request " +
         "WHERE request.requestId IN ?1", RequestEntity.class);
     return daoUtils.selectList(query, requestIds);
@@ -63,6 +71,22 @@ public class RequestDAO {
   }
 
   @RequiresSession
+  public List<Long> findAllRequestIds(int limit, boolean ascending) {
+    String sort = "ASC";
+    if (!ascending) {
+      sort = "DESC";
+    }
+
+    String sql = MessageFormat.format(REQUEST_IDS_SORTED_SQL, sort);
+    TypedQuery<Long> query = entityManagerProvider.get().createQuery(sql,
+        Long.class);
+
+    query.setMaxResults(limit);
+
+    return daoUtils.selectList(query);
+  }
+
+  @RequiresSession
   public List<RequestResourceFilterEntity> findAllResourceFilters() {
     return daoUtils.selectAll(entityManagerProvider.get(), 
RequestResourceFilterEntity.class);
   }

http://git-wip-us.apache.org/repos/asf/ambari/blob/1774c9f7/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
index 1c16ce3..9cf1f5a 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
@@ -279,7 +279,6 @@ public class ClustersImpl implements Clusters {
   }
 
   @Override
-  @Transactional
   public List<Host> getHosts() {
     checkLoaded();
     r.lock();
@@ -419,7 +418,6 @@ public class ClustersImpl implements Clusters {
     }
   }
 
-  @Transactional
   private Map<String, Host> getHostsMap(Collection<String> hostSet) throws
       HostNotFoundException {
     checkLoaded();
@@ -439,7 +437,6 @@ public class ClustersImpl implements Clusters {
     return hostMap;
   }
 
-  @Transactional
   private Map<String, Cluster> getClustersMap(Collection<String> clusterSet) 
throws
       ClusterNotFoundException {
     checkLoaded();
@@ -579,7 +576,6 @@ public class ClustersImpl implements Clusters {
   }
 
   @Override
-  @Transactional
   public Map<String, Cluster> getClusters() {
     checkLoaded();
     r.lock();

http://git-wip-us.apache.org/repos/asf/ambari/blob/1774c9f7/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/RequestDAOTest.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/RequestDAOTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/RequestDAOTest.java
index bae6fd5..8ca53f7 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/RequestDAOTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/RequestDAOTest.java
@@ -81,11 +81,15 @@ public class RequestDAOTest {
   @Test
   public void testFindAll() throws Exception {
     RequestDAO dao = injector.getInstance(RequestDAO.class);
-
     Set<Long> set = Collections.emptySet();
-
     List<RequestEntity> list = dao.findByPks(set);
+    Assert.assertEquals(0, list.size());
+  }
 
+  @Test
+  public void testFindAllRequestIds() throws Exception {
+    RequestDAO dao = injector.getInstance(RequestDAO.class);
+    List<Long> list = dao.findAllRequestIds(0, true);
     Assert.assertEquals(0, list.size());
   }
 

Reply via email to