AMBARI-19967 - Ambari Server Unit Test Failures (jonathanhurley)

ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAO.java

@@ -321,16 +321,15 @@ public class AlertDefinitionDAO {
     EntityManager entityManager = entityManagerProvider.get();
     entityManager.persist(alertDefinition);
 
-    AlertGroupEntity group = dispatchDao.findDefaultServiceGroup(
-        alertDefinition.getClusterId(), alertDefinition.getServiceName());
+    AlertGroupEntity group = dispatchDao.findDefaultServiceGroup(alertDefinition.getClusterId(),
+        alertDefinition.getServiceName());
 
     if (null == group) {
       // create the default alert group for the new service; this MUST be done
       // before adding definitions so that they are properly added to the
       // default group
       String serviceName = alertDefinition.getServiceName();
-      group = dispatchDao.createDefaultGroup(alertDefinition.getClusterId(),
-          serviceName);
+      group = dispatchDao.createDefaultGroup(alertDefinition.getClusterId(), serviceName);
     }
 
     group.addAlertDefinition(alertDefinition);

ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java

@@ -22,7 +22,6 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
 
 import javax.persistence.EntityManager;
 import javax.persistence.TypedQuery;
@@ -52,6 +51,7 @@ import org.eclipse.persistence.config.QueryHints;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.util.concurrent.Striped;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
@@ -83,10 +83,11 @@ public class AlertDispatchDAO {
   private Provider<Clusters> m_clusters;
 
   /**
-   * A lock that ensures that group writes are protected. This is useful since
-   * groups can be created through different events/threads in the system.
+   * Used for ensuring that the concurrent nature of the event handler methods
+   * don't collide when attempting to creation alert groups for the same
+   * service.
    */
-  private final Lock m_groupLock = new ReentrantLock();
+  private Striped<Lock> m_locksByService = Striped.lazyWeakLock(20);
 
   private static final Logger LOG = LoggerFactory.getLogger(AlertDispatchDAO.class);
 
@@ -194,24 +195,6 @@ public class AlertDispatchDAO {
     return daoUtils.selectList(query);
   }
 
-  /**
-   * Gets an alert group with the specified name across all clusters. Alert
-   * group names are unique within a cluster.
-   *
-   * @param groupName
-   *          the name of the group (not {@code null}).
-   * @return the alert group or {@code null} if none exists.
-   */
-  @RequiresSession
-  public AlertGroupEntity findGroupByName(String groupName) {
-    TypedQuery<AlertGroupEntity> query = entityManagerProvider.get().createNamedQuery(
-        "AlertGroupEntity.findByName", AlertGroupEntity.class);
-
-    query.setParameter("groupName", groupName);
-
-    return daoUtils.selectSingle(query);
-  }
-
   /**
    * Gets an alert group with the specified name for the given cluster. Alert
    * group names are unique within a cluster.
@@ -390,7 +373,7 @@ public class AlertDispatchDAO {
     }
 
     // sorting
-    JpaSortBuilder<AlertNoticeEntity> sortBuilder = new JpaSortBuilder<AlertNoticeEntity>();
+    JpaSortBuilder<AlertNoticeEntity> sortBuilder = new JpaSortBuilder<>();
     List<Order> sortOrders = sortBuilder.buildSortOrders(request.Sort, visitor);
     query.orderBy(sortOrders);
 
@@ -466,6 +449,7 @@ public class AlertDispatchDAO {
   @Transactional
   public AlertGroupEntity createDefaultGroup(long clusterId, String serviceName)
       throws AmbariException {
+
     // AMBARI is a special service that we let through, otherwise we need to
     // verify that the service exists before we create the default group
     String ambariServiceName = Services.AMBARI.name();
@@ -481,21 +465,26 @@ public class AlertDispatchDAO {
       }
     }
 
-    AlertGroupEntity group = new AlertGroupEntity();
+    Lock lock = m_locksByService.get(serviceName);
+    lock.lock();
 
-    m_groupLock.lock();
     try {
+      AlertGroupEntity group = findDefaultServiceGroup(clusterId, serviceName);
+      if (null != group) {
+        return group;
+      }
+
+      group = new AlertGroupEntity();
       group.setClusterId(clusterId);
       group.setDefault(true);
       group.setGroupName(serviceName);
       group.setServiceName(serviceName);
 
       create(group);
+      return group;
     } finally {
-      m_groupLock.unlock();
+      lock.unlock();
     }
-
-    return group;
   }
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertGroupEntity.java

@@ -61,9 +61,6 @@ import javax.persistence.UniqueConstraint;
     @NamedQuery(
         name = "AlertGroupEntity.findAllInCluster",
         query = "SELECT alertGroup FROM AlertGroupEntity alertGroup WHERE alertGroup.clusterId = :clusterId"),
-    @NamedQuery(
-        name = "AlertGroupEntity.findByName",
-        query = "SELECT alertGroup FROM AlertGroupEntity alertGroup WHERE alertGroup.groupName = :groupName"),
     @NamedQuery(
         name = "AlertGroupEntity.findByNameInCluster",
         query = "SELECT alertGroup FROM AlertGroupEntity alertGroup WHERE alertGroup.groupName = :groupName AND alertGroup.clusterId = :clusterId"),
@@ -226,7 +223,7 @@ public class AlertGroupEntity {
    */
   public Set<AlertDefinitionEntity> getAlertDefinitions() {
     if (null == alertDefinitions) {
-      alertDefinitions = new HashSet<AlertDefinitionEntity>();
+      alertDefinitions = new HashSet<>();
     }
 
     return Collections.unmodifiableSet(alertDefinitions);
@@ -263,7 +260,7 @@ public class AlertGroupEntity {
    */
   public void addAlertDefinition(AlertDefinitionEntity definition) {
     if (null == alertDefinitions) {
-      alertDefinitions = new HashSet<AlertDefinitionEntity>();
+      alertDefinitions = new HashSet<>();
     }
 
     alertDefinitions.add(definition);
@@ -308,7 +305,7 @@ public class AlertGroupEntity {
    */
   public void addAlertTarget(AlertTargetEntity alertTarget) {
     if (null == alertTargets) {
-      alertTargets = new HashSet<AlertTargetEntity>();
+      alertTargets = new HashSet<>();
     }
 
     alertTargets.add(alertTarget);

ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java

@@ -158,7 +158,7 @@ public class AlertDispatchDAOTest {
     assertEquals(1, targets.size());
 
     // find by ids
-    List<Long> ids = new ArrayList<Long>();
+    List<Long> ids = new ArrayList<>();
     ids.add(targets.get(0).getTargetId());
     ids.add(99999L);
 
@@ -213,7 +213,7 @@ public class AlertDispatchDAOTest {
     assertEquals(group, actual);
 
     //find by id
-    List<Long> ids = new ArrayList<Long>();
+    List<Long> ids = new ArrayList<>();
     ids.add(groups.get(0).getGroupId());
     ids.add(groups.get(1).getGroupId());
     ids.add(99999L);
@@ -243,7 +243,7 @@ public class AlertDispatchDAOTest {
   public void testCreateUpdateRemoveGroup() throws Exception {
     // create group
     AlertTargetEntity target = m_helper.createAlertTarget();
-    Set<AlertTargetEntity> targets = new HashSet<AlertTargetEntity>();
+    Set<AlertTargetEntity> targets = new HashSet<>();
     targets.add(target);
 
     AlertGroupEntity group = m_helper.createAlertGroup(
@@ -299,7 +299,7 @@ public class AlertDispatchDAOTest {
     int targetCount = m_dao.findAllTargets().size();
 
     AlertTargetEntity target = m_helper.createAlertTarget();
-    Set<AlertTargetEntity> targets = new HashSet<AlertTargetEntity>();
+    Set<AlertTargetEntity> targets = new HashSet<>();
     targets.add(target);
 
     AlertGroupEntity group = m_helper.createAlertGroup(
@@ -430,7 +430,7 @@ public class AlertDispatchDAOTest {
   @Test
   public void testDeleteAssociatedTarget() throws Exception {
     AlertTargetEntity target = m_helper.createAlertTarget();
-    Set<AlertTargetEntity> targets = new HashSet<AlertTargetEntity>();
+    Set<AlertTargetEntity> targets = new HashSet<>();
     targets.add(target);
 
     AlertGroupEntity group = m_helper.createAlertGroup(
@@ -473,7 +473,7 @@ public class AlertDispatchDAOTest {
 
     m_dao.merge(group);
 
-    group = m_dao.findGroupByName(group.getGroupName());
+    group = m_dao.findGroupByName(m_cluster.getClusterId(), group.getGroupName());
     assertEquals(definitions.size(), group.getAlertDefinitions().size());
 
     // assert that the definition is now part of 2 groups (the default group
@@ -690,7 +690,7 @@ public class AlertDispatchDAOTest {
 
     m_alertHelper.populateData(m_cluster);
 
-    List<SortRequestProperty> sortProperties = new ArrayList<SortRequestProperty>();
+    List<SortRequestProperty> sortProperties = new ArrayList<>();
     SortRequest sortRequest = new SortRequestImpl(sortProperties);
 
     AlertNoticeRequest request = new AlertNoticeRequest();
@@ -850,7 +850,7 @@ public class AlertDispatchDAOTest {
    * @throws Exception
    */
   private Set<AlertTargetEntity> createTargets(int numberOfTargets) throws Exception {
-    Set<AlertTargetEntity> targets = new HashSet<AlertTargetEntity>();
+    Set<AlertTargetEntity> targets = new HashSet<>();
     for (int i = 0; i < numberOfTargets; i++) {
       AlertTargetEntity target = new AlertTargetEntity();
       target.setDescription("Target Description " + i);
@@ -883,7 +883,7 @@ public class AlertDispatchDAOTest {
 
     m_dao.merge(group);
 
-    group = m_dao.findGroupByName(group.getGroupName());
+    group = m_dao.findGroupByName(m_cluster.getClusterId(), group.getGroupName());
     assertEquals(definitions.size(), group.getAlertDefinitions().size());
 
     for (AlertDefinitionEntity definition : definitions) {
@@ -894,7 +894,7 @@ public class AlertDispatchDAOTest {
     m_definitionDao.remove(definitions.get(0));
     definitions.remove(0);
 
-    group = m_dao.findGroupByName(group.getGroupName());
+    group = m_dao.findGroupByName(m_cluster.getClusterId(), group.getGroupName());
     assertEquals(definitions.size(), group.getAlertDefinitions().size());
 
     for (AlertDefinitionEntity definition : definitions) {

ambari-server/src/test/java/org/apache/ambari/server/state/cluster/AlertDataManagerTest.java

@@ -113,6 +113,7 @@ public class AlertDataManagerTest {
   public void setup() throws Exception {
     m_injector = Guice.createInjector(new InMemoryDefaultTestModule());
     EventBusSynchronizer.synchronizeAlertEventPublisher(m_injector);
+    EventBusSynchronizer.synchronizeAmbariEventPublisher(m_injector);
     m_injector.getInstance(GuiceJpaInitializer.class);
     m_injector.getInstance(UnitOfWork.class).begin();
 
@@ -311,7 +312,7 @@ public class AlertDataManagerTest {
     m_dao.create(currentAlert);
 
     AlertTargetEntity target = m_helper.createAlertTarget();
-    Set<AlertTargetEntity> targets = new HashSet<AlertTargetEntity>();
+    Set<AlertTargetEntity> targets = new HashSet<>();
     targets.add(target);
 
     AlertGroupEntity group = m_helper.createAlertGroup(
@@ -419,7 +420,7 @@ public class AlertDataManagerTest {
     AlertEventPublisher publisher = m_injector.getInstance(AlertEventPublisher.class);
     EventBusSynchronizer.synchronizeAlertEventPublisher(m_injector);
 
-    final AtomicReference<Alert> ref = new AtomicReference<Alert>();
+    final AtomicReference<Alert> ref = new AtomicReference<>();
     publisher.register(new TestListener() {
       @Override
       @Subscribe