Repository: ambari Updated Branches: refs/heads/trunk 516690891 -> 0887e8e3d
AMBARI-15363 - Postgres And c3p0 Queries Can Hang Ambari On Large Queries (jonathanhurley) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/0887e8e3 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/0887e8e3 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/0887e8e3 Branch: refs/heads/trunk Commit: 0887e8e3d6b6030143a8f3b38475e72875f899f1 Parents: 5166908 Author: Jonathan Hurley <jhur...@hortonworks.com> Authored: Wed Mar 9 17:29:48 2016 -0500 Committer: Jonathan Hurley <jhur...@hortonworks.com> Committed: Wed Mar 9 23:40:14 2016 -0500 ---------------------------------------------------------------------- ambari-project/pom.xml | 2 +- ambari-server/pom.xml | 16 +- .../server/api/query/JpaPredicateVisitor.java | 10 +- .../ambari/server/api/query/JpaSortBuilder.java | 31 +++- .../server/api/query/JpaSortBuilderTest.java | 153 +++++++++++++++++++ 5 files changed, 205 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/0887e8e3/ambari-project/pom.xml ---------------------------------------------------------------------- diff --git a/ambari-project/pom.xml b/ambari-project/pom.xml index ed94004..b3d9ca2 100644 --- a/ambari-project/pom.xml +++ b/ambari-project/pom.xml @@ -210,7 +210,7 @@ <dependency> <groupId>org.eclipse.persistence</groupId> <artifactId>eclipselink</artifactId> - <version>2.5.2</version> + <version>2.6.2</version> </dependency> <dependency> <groupId>org.postgresql</groupId> http://git-wip-us.apache.org/repos/asf/ambari/blob/0887e8e3/ambari-server/pom.xml ---------------------------------------------------------------------- diff --git a/ambari-server/pom.xml b/ambari-server/pom.xml index f691fad..83424c2 100644 --- a/ambari-server/pom.xml +++ b/ambari-server/pom.xml @@ -327,7 +327,7 @@ <dependency> <groupId>org.eclipse.persistence</groupId> <artifactId>eclipselink</artifactId> - <version>2.4.2</version> + <version>2.6.2</version> </dependency> </dependencies> </plugin> @@ -399,7 +399,7 @@ </source> </sources> </mapping> - <mapping> + <mapping> <directory>/usr</directory> <username>root</username> <groupname>root</groupname> @@ -1194,6 +1194,12 @@ <groupId>org.quartz-scheduler</groupId> <artifactId>quartz</artifactId> <version>2.2.1</version> + <exclusions> + <exclusion> + <groupId>c3p0</groupId> + <artifactId>c3p0</artifactId> + </exclusion> + </exclusions> </dependency> <dependency> <groupId>org.quartz-scheduler</groupId> @@ -1267,6 +1273,12 @@ <artifactId>commons-cli</artifactId> <version>1.3.1</version> </dependency> + <dependency> + <groupId>com.mchange</groupId> + <artifactId>c3p0</artifactId> + <version>[0.9.5.2]</version> + <scope>compile</scope> + </dependency> </dependencies> <pluginRepositories> http://git-wip-us.apache.org/repos/asf/ambari/blob/0887e8e3/ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java b/ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java index 3a8a631..bed0e5a 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java @@ -55,12 +55,17 @@ public abstract class JpaPredicateVisitor<T> implements PredicateVisitor { /** * The root that the {@code from} clause requests from. */ - private Root<T> m_root; + final private Root<T> m_root; /** * The query to submit to JPA. */ - private CriteriaQuery<T> m_query; + final private CriteriaQuery<T> m_query; + + /** + * The entity class that the root of the query is built from. + */ + final private Class<T> m_entityClass; /** * The last calculated predicate. @@ -87,6 +92,7 @@ public abstract class JpaPredicateVisitor<T> implements PredicateVisitor { public JpaPredicateVisitor(EntityManager entityManager, Class<T> entityClass) { m_entityManager = entityManager; m_builder = m_entityManager.getCriteriaBuilder(); + m_entityClass = entityClass; m_query = m_builder.createQuery(entityClass); m_root = m_query.from(entityClass); } http://git-wip-us.apache.org/repos/asf/ambari/blob/0887e8e3/ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java b/ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java index 8021346..5161e83 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java @@ -19,16 +19,20 @@ package org.apache.ambari.server.api.query; import java.util.ArrayList; import java.util.Collections; +import java.util.Iterator; import java.util.List; +import java.util.Set; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; import javax.persistence.criteria.Order; import javax.persistence.criteria.Path; +import javax.persistence.criteria.Root; import javax.persistence.metamodel.SingularAttribute; import org.apache.ambari.server.controller.spi.SortRequest; import org.apache.ambari.server.controller.spi.SortRequestProperty; +import org.apache.commons.lang.ObjectUtils; /** * The {@link JpaSortBuilder} class is used to convert and Ambari @@ -84,9 +88,32 @@ public class JpaSortBuilder<T> { Path<?> path = null; for (SingularAttribute<?, ?> singularAttribute : singularAttributes) { if (null == path) { + CriteriaQuery<T> query = visitor.getCriteriaQuery(); - path = query.from(visitor.getEntityClass()).get( - singularAttribute.getName()); + Set<Root<?>> roots = query.getRoots(); + + // if there are existing roots; use the existing roots to prevent more + // roots from being added potentially causing a cartesian product + // where we don't want one + if (null != roots && !roots.isEmpty()) { + Iterator<Root<?>> iterator = roots.iterator(); + while (iterator.hasNext()) { + Root<?> root = iterator.next(); + + Class<?> visitorEntityClass = visitor.getEntityClass(); + if (ObjectUtils.equals(visitorEntityClass, root.getJavaType()) + || ObjectUtils.equals(visitorEntityClass, root.getModel().getJavaType())) { + path = root.get(singularAttribute.getName()); + break; + } + } + } + + // no roots exist already which match this entity class, create a new + // path + if (null == path) { + path = query.from(visitor.getEntityClass()).get(singularAttribute.getName()); + } } else { path = path.get(singularAttribute.getName()); } http://git-wip-us.apache.org/repos/asf/ambari/blob/0887e8e3/ambari-server/src/test/java/org/apache/ambari/server/api/query/JpaSortBuilderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/api/query/JpaSortBuilderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/api/query/JpaSortBuilderTest.java new file mode 100644 index 0000000..b9bfc50 --- /dev/null +++ b/ambari-server/src/test/java/org/apache/ambari/server/api/query/JpaSortBuilderTest.java @@ -0,0 +1,153 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ambari.server.api.query; + +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + +import javax.persistence.EntityManager; +import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.criteria.Order; +import javax.persistence.criteria.Root; +import javax.persistence.metamodel.SingularAttribute; + +import org.apache.ambari.server.controller.internal.AlertHistoryResourceProvider; +import org.apache.ambari.server.controller.internal.SortRequestImpl; +import org.apache.ambari.server.controller.spi.Predicate; +import org.apache.ambari.server.controller.spi.SortRequest; +import org.apache.ambari.server.controller.spi.SortRequestProperty; +import org.apache.ambari.server.controller.utilities.PredicateBuilder; +import org.apache.ambari.server.controller.utilities.PredicateHelper; +import org.apache.ambari.server.orm.GuiceJpaInitializer; +import org.apache.ambari.server.orm.InMemoryDefaultTestModule; +import org.apache.ambari.server.orm.entities.AlertHistoryEntity; +import org.apache.ambari.server.orm.entities.AlertHistoryEntity_; +import org.junit.Before; +import org.junit.Test; + +import com.google.inject.Guice; +import com.google.inject.Injector; + +import junit.framework.Assert; + +/** + * Tests the {@link JpaSortBuilder}. + */ +public class JpaSortBuilderTest { + + private Injector m_injector; + + @Before + public void before() { + m_injector = Guice.createInjector(new InMemoryDefaultTestModule()); + m_injector.getInstance(GuiceJpaInitializer.class); + m_injector.injectMembers(this); + } + + /** + * Tests that adding a sort does not create another {@link Root} in the + * {@link CriteriaQuery}. A duplicate root will cause a cartesian product + * similar to: + * + * <pre> + * SELECT t0.alert_id, + * t0.alert_instance, + * t0.alert_label, + * t0.alert_state, + * t0.alert_text, + * t0.alert_timestamp, + * t0.cluster_id, + * t0.component_name, + * t0.host_name, + * t0.service_name, + * t0.alert_definition_id + * FROM alert_history t0, + * alert_history t2, + * alert_definition t1 + * WHERE ( ( t1.definition_name = ? ) + * AND ( t1.definition_id = t2.alert_definition_id ) ) + * ORDER BY t0.alert_timestamp DESC + * </pre> + * + * where the root for {@code alert_history} is added twice. + * + * @throws Exception + */ + @Test + public void testSortDoesNotAddExtraRootPaths() throws Exception { + // create a sort request against the entity directly + List<SortRequestProperty> sortRequestProperties = new ArrayList<>(); + + sortRequestProperties.add( + new SortRequestProperty(AlertHistoryResourceProvider.ALERT_HISTORY_TIMESTAMP, + org.apache.ambari.server.controller.spi.SortRequest.Order.ASC)); + + SortRequest sortRequest = new SortRequestImpl(sortRequestProperties); + + // create a complex, cross-entity predicate + Predicate predicate = new PredicateBuilder().property( + AlertHistoryResourceProvider.ALERT_HISTORY_DEFINITION_NAME).equals("foo").toPredicate(); + + MockAlertHistoryredicateVisitor visitor = new MockAlertHistoryredicateVisitor(); + PredicateHelper.visit(predicate, visitor); + + JpaSortBuilder<AlertHistoryEntity> sortBuilder = new JpaSortBuilder<AlertHistoryEntity>(); + List<Order> sortOrders = sortBuilder.buildSortOrders(sortRequest, visitor); + + Assert.assertEquals(sortOrders.size(), 1); + + // verify the CriteriaQuery has the correct roots + // it should have one for the main query predicate + CriteriaQuery<AlertHistoryEntity> query = visitor.getCriteriaQuery(); + Set<Root<?>> roots = query.getRoots(); + Assert.assertEquals(1, roots.size()); + } + + /** + * The {@link HistoryPredicateVisitor} is used to convert an Ambari + * {@link Predicate} into a JPA {@link javax.persistence.criteria.Predicate}. + */ + private final class MockAlertHistoryredicateVisitor + extends JpaPredicateVisitor<AlertHistoryEntity> { + + /** + * Constructor. + * + */ + public MockAlertHistoryredicateVisitor() { + super(m_injector.getInstance(EntityManager.class), AlertHistoryEntity.class); + } + + /** + * {@inheritDoc} + */ + @Override + public Class<AlertHistoryEntity> getEntityClass() { + return AlertHistoryEntity.class; + } + + /** + * {@inheritDoc} + */ + @Override + public List<? extends SingularAttribute<?, ?>> getPredicateMapping(String propertyId) { + return AlertHistoryEntity_.getPredicateMapping().get(propertyId); + } + } +}