AMBARI-19967 - Ambari Server Unit Test Failures (jonathanhurley)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/fa32fec6 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/fa32fec6 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/fa32fec6 Branch: refs/heads/branch-feature-AMBARI-12556 Commit: fa32fec6d891745885b976cc2dc13512706dfed9 Parents: 6c4cbc4 Author: Jonathan Hurley <[email protected]> Authored: Fri Feb 10 12:09:38 2017 -0500 Committer: Jonathan Hurley <[email protected]> Committed: Mon Feb 13 11:23:33 2017 -0500 ---------------------------------------------------------------------- .../server/orm/dao/AlertDefinitionDAO.java | 7 ++- .../ambari/server/orm/dao/AlertDispatchDAO.java | 45 ++++++++------------ .../server/orm/entities/AlertGroupEntity.java | 9 ++-- .../server/orm/dao/AlertDispatchDAOTest.java | 20 ++++----- .../state/cluster/AlertDataManagerTest.java | 5 ++- 5 files changed, 36 insertions(+), 50 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/fa32fec6/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAO.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAO.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAO.java index 703ff58..c3e3a9f 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAO.java +++ b/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); http://git-wip-us.apache.org/repos/asf/ambari/blob/fa32fec6/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java index 3b9c97a..5bd84ad 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java +++ b/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); @@ -195,24 +196,6 @@ public class AlertDispatchDAO { } /** - * 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; } /** http://git-wip-us.apache.org/repos/asf/ambari/blob/fa32fec6/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertGroupEntity.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertGroupEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertGroupEntity.java index b660631..7ca26e6 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertGroupEntity.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertGroupEntity.java @@ -62,9 +62,6 @@ import javax.persistence.UniqueConstraint; 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"), @NamedQuery( @@ -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); http://git-wip-us.apache.org/repos/asf/ambari/blob/fa32fec6/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java b/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java index 1ec6d40..0bdd5b2 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java +++ b/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) { http://git-wip-us.apache.org/repos/asf/ambari/blob/fa32fec6/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/AlertDataManagerTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/AlertDataManagerTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/AlertDataManagerTest.java index 4ad93e6..1e74658 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/AlertDataManagerTest.java +++ b/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
