modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/PluginMetadataManager.java
| 6
modules/core/domain/src/main/java/org/rhq/core/domain/configuration/Configuration.java
| 2
modules/core/domain/src/main/java/org/rhq/core/domain/resource/Resource.java
| 4
modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java
| 115 +++++++---
modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/RuntimeDiscoveryExecutor.java
| 39 +--
5 files changed, 105 insertions(+), 61 deletions(-)
New commits:
commit 98149c5c59ee5cb6932240fc291f8db69c8b2580
Author: Ian Springer <ian.springer(a)redhat.com>
Date: Wed Feb 29 22:43:34 2012 -0500
[BZ 797999] prevent ConcurrentModificationExceptions during inventory sync of unknown
Resources (
https://bugzilla.redhat.com/show_bug.cgi?id=797999)
diff --git
a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java
b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java
index 6f2d55e..c583f4b 100644
---
a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java
+++
b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java
@@ -860,6 +860,28 @@ public class InventoryManager extends AgentService implements
ContainerService,
return resource;
}
+ private Resource cloneResourceWithoutChildren(Resource resourceFromServer) {
+ // Use a ConcurrentHashMap-based Set for childResources to allow the field to be
concurrently accessed safely
+ // (i.e. to avoid ConcurrentModificationExceptions).
+ Set<Resource> childResources = Collections.newSetFromMap(new
ConcurrentHashMap<Resource, Boolean>());
+ Resource resource = new Resource(childResources);
+
+ resource.setId(resourceFromServer.getId());
+ resource.setUuid(resourceFromServer.getUuid());
+ resource.setResourceKey(resourceFromServer.getResourceKey());
+ resource.setResourceType(resourceFromServer.getResourceType());
+ resource.setMtime(resourceFromServer.getMtime());
+ resource.setInventoryStatus(resourceFromServer.getInventoryStatus());
+ resource.setPluginConfiguration(resourceFromServer.getPluginConfiguration());
+ resource.setVersion(resourceFromServer.getVersion());
+
+ resource.setName(resourceFromServer.getName());
+ resource.setDescription(resourceFromServer.getDescription());
+ resource.setLocation(resourceFromServer.getLocation());
+
+ return resource;
+ }
+
/**
* Returns the known availability for the resource. If the availability is not known,
<code>null</code> is returned.
*
@@ -2635,81 +2657,104 @@ public class InventoryManager extends AgentService implements
ContainerService,
// }
// }
- private void mergeResource(Resource resource) {
+ private void mergeResource(Resource resourceFromServer) {
if (log.isDebugEnabled()) {
- log.debug("Merging [" + resource + "] into local
inventory...");
+ log.debug("Merging " + resourceFromServer + " into local
inventory...");
}
- Resource passedResource = resource; // always keep a reference to the passed
resource
+
+ // Replace the stripped-down ResourceType that came from the Server with the full
ResourceType - it's critical
+ // to do this before merging the Resource, because the plugin container and
plugins rely on the type being fully
+ // initialized.
+ if (!hydrateResourceType(resourceFromServer)) { return; }
+
+ // Find the Resource's parent in our inventory.
Resource parentResource;
- if (resource.getParentResource() != null) {
- ResourceContainer parentResourceContainer =
getResourceContainer(resource.getParentResource());
+ Resource parentResourceFromServer = resourceFromServer.getParentResource();
+ if (parentResourceFromServer != null) {
+ ResourceContainer parentResourceContainer =
getResourceContainer(parentResourceFromServer);
if (parentResourceContainer == null) {
- parentResourceContainer =
getResourceContainer(resource.getParentResource().getId());
+ parentResourceContainer =
getResourceContainer(parentResourceFromServer.getId());
}
- if (parentResourceContainer != null)
+ if (parentResourceContainer != null) {
parentResource = parentResourceContainer.getResource();
- else
+ } else {
parentResource = null; // TODO right thing to do? Or directly return?
+ }
} else {
parentResource = null;
}
- Resource existingResource = findMatchingChildResource(resource, parentResource);
+
+ // See if the Resource already exists in our inventory.
+ Resource existingResource = findMatchingChildResource(resourceFromServer,
parentResource);
if (parentResource == null && existingResource == null) {
// This should never happen, but add a check so we'll know if it ever
does.
log.error("Existing platform [" + this.platform + "] has
different Resource type and/or Resource key than "
- + "platform in Server inventory: " + resource);
- }
+ + "platform in Server inventory: " + resourceFromServer);
+ }
+ boolean pluginConfigUpdated;
+
+ Resource mergedResource;
ResourceContainer resourceContainer;
- boolean pluginConfigUpdated = false;
this.inventoryLock.writeLock().lock();
try {
- if (existingResource != null) {
+ if (existingResource != null) { // modified Resource
+ log.debug("Modifying " + existingResource + " in local
inventory - Resource from Server is "
+ + resourceFromServer + ".");
+
// First grab the existing Resource's container, so we can reuse it.
resourceContainer =
this.resourceContainers.remove(existingResource.getUuid());
if (resourceContainer != null) {
- this.resourceContainers.put(resource.getUuid(), resourceContainer);
+ this.resourceContainers.put(resourceFromServer.getUuid(),
resourceContainer);
+ } else {
+ log.error("No ResourceContainer found for existing " +
existingResource + ".");
+ return;
}
if (parentResource != null) {
// It's critical to remove the existing Resource from the
parent's child Set if the UUID has
// changed (i.e. altering the hashCode of an item in a Set == BAD),
so just always remove it.
parentResource.removeChildResource(existingResource);
}
- // Now merge the new Resource into the existing Resource...
- pluginConfigUpdated = mergeResource(resource, existingResource);
- resource = existingResource;
+ // Now merge the Resource from the Server into the existing Resource...
+ pluginConfigUpdated = mergeResource(resourceFromServer,
existingResource);
+ mergedResource = existingResource;
+ } else { // unknown Resource
+ log.debug("Adding unknown " + resourceFromServer + " to
local inventory.");
+ pluginConfigUpdated = false;
+ mergedResource = cloneResourceWithoutChildren(resourceFromServer);
}
if (parentResource != null) {
- parentResource.addChildResource(resource);
+ parentResource.addChildResource(mergedResource);
} else {
- this.platform = resource;
- }
-
- // Replace the stripped-down ResourceType that came from the Server with the
full ResourceType - it's
- // critical to do this before refreshing the state (i.e. calling start on the
ResourceComponent)
- // and critical to do this before initializing the container (since its
needed for the classloader creation).
- ResourceType fullResourceType =
this.pluginManager.getMetadataManager().getType(resource.getResourceType());
- if (fullResourceType == null) {
- log.error("Unable to merge Resource " + resource + " - its
type is unknown - perhaps the ["
- + resource.getResourceType().getPlugin()
- + "] plugin jar was manually removed from the Server's
rhq-plugins dir?");
- return;
+ this.platform = mergedResource;
}
- resource.setResourceType(fullResourceType);
-
- resourceContainer = initResourceContainer(resource);
+ resourceContainer = initResourceContainer(mergedResource);
} finally {
this.inventoryLock.writeLock().unlock();
}
refreshResourceComponentState(resourceContainer, pluginConfigUpdated);
- for (Resource childResource : passedResource.getChildResources()) {
+ // Recursively merge the children.
+ for (Resource childResource : resourceFromServer.getChildResources()) {
mergeResource(childResource);
}
}
+ private boolean hydrateResourceType(Resource resourceFromServer) {
+ ResourceType fullResourceType =
+
this.pluginManager.getMetadataManager().getType(resourceFromServer.getResourceType());
+ if (fullResourceType == null) {
+ log.error(resourceFromServer + " being synced from Server has an unknown
type ["
+ + resourceFromServer.getResourceType() + "] - the [" +
resourceFromServer.getResourceType().getPlugin()
+ + "] plugin is most likely not up to date in the Agent - try
updating the Agent's plugins.");
+ return false;
+ }
+ resourceFromServer.setResourceType(fullResourceType);
+ return true;
+ }
+
private boolean mergeResource(Resource sourceResource, Resource targetResource) {
if (targetResource.getId() != 0 && targetResource.getId() !=
sourceResource.getId()) {
log.warn("Id for " + targetResource + " changed from [" +
targetResource.getId() + "] to ["
@@ -2728,9 +2773,11 @@ public class InventoryManager extends AgentService implements
ContainerService,
boolean pluginConfigUpdated = (!targetResource.getPluginConfiguration().equals(
sourceResource.getPluginConfiguration()));
targetResource.setPluginConfiguration(sourceResource.getPluginConfiguration());
+
targetResource.setName(sourceResource.getName());
targetResource.setDescription(sourceResource.getDescription());
targetResource.setLocation(sourceResource.getLocation());
+
return pluginConfigUpdated;
}
diff --git
a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/RuntimeDiscoveryExecutor.java
b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/RuntimeDiscoveryExecutor.java
index 0f2257f..1500eb2 100644
---
a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/RuntimeDiscoveryExecutor.java
+++
b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/RuntimeDiscoveryExecutor.java
@@ -26,7 +26,6 @@ import java.io.ByteArrayOutputStream;
import java.io.ObjectOutputStream;
import java.util.Collections;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Callable;
@@ -147,25 +146,20 @@ public class RuntimeDiscoveryExecutor implements Runnable,
Callable<InventoryRep
}
private void discoverForResourceRecursive(Resource parent, InventoryReport report)
throws PluginContainerException {
- Set<Resource> children = parent.getChildResources();
- if (!children.isEmpty()) {
- Set<Resource> childrenCopy = new HashSet<Resource>(children); //
prevent concurrent mod
- for (Resource child : childrenCopy) {
- // See if the child has new children itself. Then we check those children
to see if there are grandchildren.
- // Note that if the child has already been added to the report, there is
no need to process it again, so skip it.
- boolean alreadyProcessed = report.getAddedRoots().contains(child);
- if (!alreadyProcessed) {
- discoverForResource(child, report, alreadyProcessed);
- // We need to recurse here even though discoverForResource recurses
over child, too.
- // This is because that discovery above only goes over newly
discovered resources.
- // It is possible this child has already existing children (e.g.
previously manually added)
- // that they themselves might have additional new children that need
discovering.
- discoverForResourceRecursive(child, report);
- }
+ for (Resource child : parent.getChildResources()) {
+ // See if the child has new children itself. Then we check those children to
see if there are grandchildren.
+ // Note that if the child has already been added to the report, there is no
need to process it again, so skip it.
+ boolean alreadyProcessed = report.getAddedRoots().contains(child);
+ if (!alreadyProcessed) {
+ discoverForResource(child, report, alreadyProcessed);
+ // We need to recurse here even though discoverForResource recurses over
child, too.
+ // This is because that discovery above only goes over newly discovered
resources.
+ // It is possible this child has already existing children (e.g.
previously manually added)
+ // that they themselves might have additional new children that need
discovering.
+ discoverForResourceRecursive(child, report);
}
- childrenCopy.clear(); // help GC
- childrenCopy = null;
}
+
return;
}
@@ -180,7 +174,9 @@ public class RuntimeDiscoveryExecutor implements Runnable,
Callable<InventoryRep
private void discoverForResource(Resource parent, InventoryReport report, boolean
parentReported)
throws PluginContainerException {
// TODO GH: If resource.isRuntimeDiscoveryEnabled
- // TODO GH: If resoure.isInventoryStatusCommitted
+ // TODO GH: If resource.isInventoryStatusCommitted
+
+ log.debug("Discovering child Resources for " + parent +
"...");
ResourceContainer parentContainer =
this.inventoryManager.getResourceContainer(parent);
if (parentContainer == null) {
@@ -199,7 +195,7 @@ public class RuntimeDiscoveryExecutor implements Runnable,
Callable<InventoryRep
if (parent.getInventoryStatus() != InventoryStatus.COMMITTED) {
if (log.isDebugEnabled()) {
- log.debug("Parent [" + parent + "] must be
imported/commited before service scan can run.");
+ log.debug("Parent [" + parent + "] must be
imported/committed before service scan can run.");
}
return;
}
@@ -236,7 +232,8 @@ public class RuntimeDiscoveryExecutor implements Runnable,
Callable<InventoryRep
// For each child resource type of the server, do a discovery for resources of
that type
PluginComponentFactory factory =
PluginContainer.getInstance().getPluginComponentFactory();
- for (ResourceType childResourceType :
parent.getResourceType().getChildResourceTypes()) {
+ Set<ResourceType> childResourceTypes =
parent.getResourceType().getChildResourceTypes();
+ for (ResourceType childResourceType : childResourceTypes) {
try {
// Make sure we have a discovery component for that type, otherwise there
is nothing to do
ResourceDiscoveryComponent discoveryComponent = null;
commit 463f433dc0c5f87a6af028732fb8f495bc881fce
Author: Ian Springer <ian.springer(a)redhat.com>
Date: Wed Feb 29 22:40:26 2012 -0500
minor: remove unused imports
diff --git
a/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/Configuration.java
b/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/Configuration.java
index a0d5561..3ad8e95 100644
---
a/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/Configuration.java
+++
b/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/Configuration.java
@@ -47,9 +47,7 @@ import javax.persistence.PreUpdate;
import javax.persistence.SequenceGenerator;
import javax.persistence.Table;
import javax.xml.bind.annotation.XmlAccessType;
-import javax.xml.bind.annotation.XmlAccessorOrder;
import javax.xml.bind.annotation.XmlAccessorType;
-import javax.xml.bind.annotation.XmlElement;
import javax.xml.bind.annotation.XmlElementRef;
import javax.xml.bind.annotation.XmlElementRefs;
import javax.xml.bind.annotation.XmlRootElement;
commit 6538928e21b0884054fe2eba060a84dc0001e7fd
Author: Ian Springer <ian.springer(a)redhat.com>
Date: Wed Feb 29 22:40:02 2012 -0500
minor: improve variable name
diff --git
a/modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/PluginMetadataManager.java
b/modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/PluginMetadataManager.java
index 0c5e0b6..ce621ff 100644
---
a/modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/PluginMetadataManager.java
+++
b/modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/PluginMetadataManager.java
@@ -166,9 +166,9 @@ public class PluginMetadataManager {
public ResourceType getType(String resourceTypeName, String pluginName) {
ResourceType searchType = new ResourceType(resourceTypeName, pluginName, null,
null);
synchronized (this.typesLock) {
- for (ResourceType t : types) {
- if (t.equals(searchType)) {
- return t;
+ for (ResourceType type : types) {
+ if (type.equals(searchType)) {
+ return type;
}
}
}
commit 199d19d6670eced1485ffd1b59127ae36e7b4b4c
Author: Ian Springer <ian.springer(a)redhat.com>
Date: Wed Feb 29 22:38:12 2012 -0500
include uuid and type's plugin in toString()
diff --git a/modules/core/domain/src/main/java/org/rhq/core/domain/resource/Resource.java
b/modules/core/domain/src/main/java/org/rhq/core/domain/resource/Resource.java
index d0f49cd..9247850 100644
--- a/modules/core/domain/src/main/java/org/rhq/core/domain/resource/Resource.java
+++ b/modules/core/domain/src/main/java/org/rhq/core/domain/resource/Resource.java
@@ -1855,7 +1855,9 @@ public class Resource implements Comparable<Resource>,
Serializable {
StringBuilder buffer = new StringBuilder();
buffer.append("Resource").append("[");
buffer.append("id=").append(this.id);
- String typeName = (this.resourceType != null) ? this.resourceType.getName() :
"<null>";
+ buffer.append(", uuid=").append(this.uuid);
+ String typeName = (this.resourceType != null) ?
+ '{' + this.resourceType.getPlugin() + '}' +
this.resourceType.getName() : "<null>";
buffer.append(", type=").append(typeName);
buffer.append(", key=").append(this.resourceKey);
buffer.append(", name=").append(this.name);