modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryBossBean.java | 90 ++++++----
1 file changed, 57 insertions(+), 33 deletions(-)
New commits:
commit 4b370f3059fdd74fa62b19b36e2315e73ecc5025
Author: Jay Shaughnessy <jshaughn(a)jshaughn.csb>
Date: Tue Jan 29 11:40:54 2013 -0500
More mergeInventoryReport speed
- add map cache of attached parent entities to avoid redundant DB fetches
- replace named query fetch with simple em.find where possible
- trivial, rename some private methods that were named inappropriately
CP master: dac4ee6d56378f759f6e61abafa6999a9c281aba
diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryBossBean.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryBossBean.java
index 2a2a80f..ceff0ea 100644
--- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryBossBean.java
+++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryBossBean.java
@@ -37,7 +37,6 @@ import javax.ejb.Stateless;
import javax.ejb.TransactionAttribute;
import javax.ejb.TransactionAttributeType;
import javax.persistence.EntityManager;
-import javax.persistence.NoResultException;
import javax.persistence.PersistenceContext;
import javax.persistence.Query;
import javax.security.auth.login.LoginException;
@@ -499,7 +498,7 @@ public class DiscoveryBossBean implements DiscoveryBossLocal, DiscoveryBossRemot
+ resource.getResourceType());
}
- Resource existingResource = findExistingResource(resource);
+ Resource existingResource = findExistingResource(resource, null);
if (existingResource != null) {
mergeResourceResponse = new MergeResourceResponse(existingResource.getId(), true);
} else {
@@ -781,20 +780,22 @@ public class DiscoveryBossBean implements DiscoveryBossLocal, DiscoveryBossRemot
long batchStart = System.currentTimeMillis();
boolean isDebugEnabled = log.isDebugEnabled();
+ // Cache parent resources we've already fetched from the DB, many resources will have the same parent
+ Map<Integer, Resource> parentMap = new HashMap<Integer, Resource>();
for (Resource resource : resourceBatch) {
Resource existingResource = null;
long start = System.currentTimeMillis();
- existingResource = findExistingResource(resource);
+ existingResource = findExistingResource(resource, parentMap);
// Does this resource already exist in inventory? If so, update, otherwise add
if (null != existingResource) {
- updateExistingResourceInNewTransaction(resource, existingResource);
+ updateExistingResource(resource, existingResource);
} else {
presetAgent(resource, agent);
- persistResourceInNewTransaction(resource);
+ persistResource(resource, parentMap);
}
if (isDebugEnabled) {
@@ -803,6 +804,9 @@ public class DiscoveryBossBean implements DiscoveryBossLocal, DiscoveryBossRemot
}
}
+ // Help out the GC
+ parentMap.clear();
+
if (isDebugEnabled) {
long delta = (System.currentTimeMillis() - batchStart);
log.debug("Resource Batch merged: size/average/millis=" + resourceBatch.size() + "/" + delta
@@ -824,15 +828,19 @@ public class DiscoveryBossBean implements DiscoveryBossLocal, DiscoveryBossRemot
}
/**
+ * <p>Requires A Transaction</p>
+ *
* Given a resource, will attempt to find it in the server's inventory (that is, finds it in the database). If the
* given resource's ID does not exist in the database, it will be looked up by its resource key. If the resource
* cannot be found either via ID or resource key then SIDE EFFECT: the given resource's ID will be reset to 0 and null
* will be returned.
*
- * @param resource Pojo containing resourceId, key, and parentResoure (if applicable)
+ * @param resource Pojo containing resourceId, key, and parentResoure (if applicable)
+ * @param parentMap, if supplied, holds previously fetched parent pojos. useful when the calling code does many
+ * finds for a few parents. If not found in the map the db will be searched, the map will be updated if possible.
* @return the Resource entity found in the database and matching the given resource.
*/
- private Resource findExistingResource(Resource resource) {
+ private Resource findExistingResource(Resource resource, Map<Integer, Resource> parentMap) {
boolean isDebugEnabled = log.isDebugEnabled();
@@ -840,27 +848,20 @@ public class DiscoveryBossBean implements DiscoveryBossLocal, DiscoveryBossRemot
log.debug("getExistingResource processing for [" + resource + "]");
}
- // We perform this query a lot during a large inventory merge, so minimize overhead by using it directly
- Query query = entityManager.createNamedQuery(Resource.QUERY_FIND_BY_ID);
+ // We perform this query a lot during a large inventory merge, so minimize overhead by using it directly
Resource existingResource = null;
- Subject overlord = subjectManager.getOverlord();
if (resource.getId() != 0) {
if (isDebugEnabled) {
log.debug("Agent claims resource is already in inventory. Id=" + resource.getId());
}
- try {
- query.setParameter("resourceId", resource.getId());
- existingResource = (Resource) query.getSingleResult();
- if (isDebugEnabled) {
+ existingResource = entityManager.find(Resource.class, resource.getId());
+ if (isDebugEnabled) {
+ if (null != existingResource) {
log.debug("Found resource already in inventory. Id=" + resource.getId());
- }
- } catch (NoResultException e) {
- existingResource = null;
-
- // agent lied - agent's copy of JON server inventory must be stale.
- if (isDebugEnabled) {
+ } else {
+ // agent lied - agent's copy of JON server inventory must be stale.
log.debug("However, no resource exists with the specified id. Id=" + resource.getId());
}
}
@@ -882,6 +883,7 @@ public class DiscoveryBossBean implements DiscoveryBossLocal, DiscoveryBossRemot
// relocate the parent. Anyway, I'm not going to touch it even though it slows things down.
ResourceType resourceType = resource.getResourceType();
Resource parent = resource;
+ Subject overlord = subjectManager.getOverlord();
while (null != parent && null == existingResource) {
parent = parent.getParentResource();
@@ -889,18 +891,28 @@ public class DiscoveryBossBean implements DiscoveryBossLocal, DiscoveryBossRemot
// check if the parent is in inventory. This might not be the case during initial sync-up for resource upgrade.
Resource existingParent = null;
if (null != parent) {
- if (parent.getId() <= 0) {
+ int parentId = parent.getId();
+
+ if (parentId <= 0) {
log.warn("Expected potential parent resource to have a valid ID. Parent=" + parent + ", Child="
+ resource);
}
- try {
- query.setParameter("resourceId", parent.getId());
- existingParent = (Resource) query.getSingleResult();
+ // See if we already fetched this parent and it's in our cache map, if so, use it.
+ if (null != parentMap) {
+ existingParent = parentMap.get(parentId);
+ }
- } catch (NoResultException e) {
- // this parent is not known to the server, so there's no point in trying to find a child of it...
- continue;
+ if (null == existingParent) {
+ existingParent = entityManager.find(Resource.class, parentId);
+ if (null != existingParent) {
+ if (null != parentMap) {
+ parentMap.put(parentId, existingParent);
+ }
+ } else {
+ // this parent is not known to the server, so there's no point in trying to find a child of it...
+ continue;
+ }
}
}
@@ -935,7 +947,7 @@ public class DiscoveryBossBean implements DiscoveryBossLocal, DiscoveryBossRemot
return existingResource;
}
- private void updateExistingResourceInNewTransaction(Resource updatedResource, Resource existingResource)
+ private void updateExistingResource(Resource updatedResource, Resource existingResource)
throws InvalidInventoryReportException {
/*
* there exists a small window of time after the synchronous part of the uninventory and before the async
@@ -1078,13 +1090,25 @@ public class DiscoveryBossBean implements DiscoveryBossLocal, DiscoveryBossRemot
return true;
}
- private void persistResourceInNewTransaction(Resource resource) {
+ private void persistResource(Resource resource, Map<Integer, Resource> parentMap) {
- Resource parentResource = resource.getParentResource();
+ // Id of detached parent resource
+ Integer parentId = (null != resource.getParentResource()) ? resource.getParentResource().getId() : null;
- if (null != parentResource) {
- // Find the parent resource entity and create the parent-child relationship
- parentResource = findExistingResource(parentResource);
+ // attached parentResource
+ Resource parentResource = null;
+
+ if (null != parentId) {
+ // look in our map cache first
+ if (null != parentMap) {
+ parentResource = parentMap.get(parentId);
+ }
+ // if not in cache, try the DB
+ if (null == parentResource) {
+ // Find the parent resource entity
+ parentResource = entityManager.find(Resource.class, parentId);
+ }
+ // if the parent exists, create the parent-child relationship
if (null != parentResource) {
parentResource.addChildResource(resource);
}
