modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/ConfigurationMetadataParser.java | 22 ++++--- modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/PluginMetadataManager.java | 11 +-- modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/PluginMetadataParser.java | 18 ++++- modules/core/domain/src/main/java/org/rhq/core/domain/configuration/Configuration.java | 5 + modules/core/plugin-container/src/main/java/org/rhq/core/pc/configuration/ConfigurationCheckExecutor.java | 2 modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/AvailabilityExecutor.java | 31 +++++----- modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/ForceAvailabilityExecutor.java | 4 - modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java | 13 ++-- modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/RuntimeDiscoveryExecutor.java | 2 9 files changed, 66 insertions(+), 42 deletions(-)
New commits: commit e460fc848e588afa2d4f0c005424db908be70f9c Author: Heiko W. Rupp hwr@redhat.com Date: Mon Dec 16 12:15:35 2013 +0100
Tune down logging. Obtain resource containers by numeric resource id instead of uuid String.
diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/configuration/ConfigurationCheckExecutor.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/configuration/ConfigurationCheckExecutor.java index abd0ae1..1dce1d9 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/configuration/ConfigurationCheckExecutor.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/configuration/ConfigurationCheckExecutor.java @@ -77,7 +77,7 @@ public class ConfigurationCheckExecutor implements Runnable, Callable { }
public CountTime checkConfigurations(Resource resource, boolean checkChildren) { - ResourceContainer resourceContainer = this.inventoryManager.getResourceContainer(resource); + ResourceContainer resourceContainer = this.inventoryManager.getResourceContainer(resource.getId()); ConfigurationFacet resourceComponent = null; ResourceType resourceType = resource.getResourceType();
diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/AvailabilityExecutor.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/AvailabilityExecutor.java index c23809d..d21505e 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/AvailabilityExecutor.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/AvailabilityExecutor.java @@ -355,7 +355,7 @@ public class AvailabilityExecutor implements Runnable, Callable<AvailabilityRepo t.getLocalizedMessage(), ThrowableUtil.getStackAsString(t), System.currentTimeMillis()); this.inventoryManager.sendResourceErrorToServer(resourceError); LOG.warn("Availability collection failed with exception on " + resource - + ", availability will be reported as " + DOWN.name(), t); + + ", availability will be reported as " + DOWN.name() + ", reason=" + t.getMessage()); current = DOWN; } } else { 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 27bd7ef..fd5713c 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 @@ -1031,7 +1031,7 @@ public class InventoryManager extends AgentService implements ContainerService, */ @Nullable public Availability getAvailabilityIfKnown(Resource resource) { - ResourceContainer resourceContainer = getResourceContainer(resource); + ResourceContainer resourceContainer = getResourceContainer(resource.getId());
if (resourceContainer != null) { if (ResourceComponentState.STARTED == resourceContainer.getResourceComponentState()) { @@ -1926,6 +1926,12 @@ public class InventoryManager extends AgentService implements ContainerService, // because we're not actually STARTED container.setResourceComponentState(ResourceComponentState.STOPPED);
+ String message = "Failed to start component for resource " + resource + + "."; + if (t.getCause()!=null) { + message += " Cause: " + t.getCause().getMessage(); + } + if (updatedPluginConfig || (t instanceof InvalidPluginConfigurationException)) { if (log.isDebugEnabled()) { log.debug("Resource has a bad config, waiting for this to go away: " + resource); @@ -1933,11 +1939,10 @@ public class InventoryManager extends AgentService implements ContainerService, InventoryEventListener iel = new ResourceGotActivatedListener(); addInventoryEventListener(iel);
- throw new InvalidPluginConfigurationException("Failed to start component for resource " + resource - + ".", t); + throw new InvalidPluginConfigurationException(message); }
- throw new PluginContainerException("Failed to start component for resource " + resource + ".", t); + throw new PluginContainerException(message); }
// We purposefully do not get availability of this resource yet
commit 2c79b84224f0e25264935edfbcdb4a09f2528266 Author: Heiko W. Rupp hwr@redhat.com Date: Mon Dec 16 11:54:46 2013 +0100
Do some string interning.
diff --git a/modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/ConfigurationMetadataParser.java b/modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/ConfigurationMetadataParser.java index 35ef00a..d40d82d 100644 --- a/modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/ConfigurationMetadataParser.java +++ b/modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/ConfigurationMetadataParser.java @@ -85,7 +85,7 @@ public class ConfigurationMetadataParser { return null; }
- ConfigurationDefinition configurationDefinition = new ConfigurationDefinition(configurationName, + ConfigurationDefinition configurationDefinition = new ConfigurationDefinition(configurationName.intern(), descriptor.getNotes()); configurationDefinition.setConfigurationFormat(getConfigurationFormat(descriptor));
@@ -181,14 +181,14 @@ public class ConfigurationMetadataParser { } }
- private static ConfigurationTemplate parseTemplate(ConfigurationTemplateDescriptor templateDescripter) + private static ConfigurationTemplate parseTemplate(ConfigurationTemplateDescriptor templateDescriptor) throws InvalidPluginDescriptorException { - ConfigurationTemplate template = new ConfigurationTemplate(templateDescripter.getName(), - templateDescripter.getDescription()); + ConfigurationTemplate template = new ConfigurationTemplate(templateDescriptor.getName(), + templateDescriptor.getDescription()); Configuration templateConfiguration = new Configuration(); template.setConfiguration(templateConfiguration);
- parseProperties(templateDescripter, templateConfiguration, null); + parseProperties(templateDescriptor, templateConfiguration, null); return template; }
@@ -313,12 +313,14 @@ public class ConfigurationMetadataParser { PropertyDefinition memberDefinition = (memberProperty != null) ? parseProperty(memberProperty.getValue(), 0) : null;
- PropertyDefinitionList list = new PropertyDefinitionList(listProperty.getName(), description, + PropertyDefinitionList list = new PropertyDefinitionList(listProperty.getName().intern(), description, listProperty.isRequired(), memberDefinition);
String displayName = (listProperty.getDisplayName() != null) ? listProperty.getDisplayName() : StringUtils .deCamelCase(listProperty.getName()); - list.setDisplayName(displayName); + if (displayName!=null) { + list.setDisplayName(displayName.intern()); + } list.setReadOnly(listProperty.isReadOnly()); list.setSummary(listProperty.isSummary());
@@ -336,12 +338,14 @@ public class ConfigurationMetadataParser { AbstractPropertyMap defaultConfigurationParentMap) throws InvalidPluginDescriptorException { String description = parseMultiValue(mapProperty.getDescription(), mapProperty.getLongDescription());
- PropertyDefinitionMap propDefMap = new PropertyDefinitionMap(mapProperty.getName(), description, + PropertyDefinitionMap propDefMap = new PropertyDefinitionMap(mapProperty.getName().intern(), description, mapProperty.isRequired());
String displayName = (mapProperty.getDisplayName() != null) ? mapProperty.getDisplayName() : StringUtils .deCamelCase(mapProperty.getName()); - propDefMap.setDisplayName(displayName); + if (displayName!=null) { + propDefMap.setDisplayName(displayName.intern()); + } propDefMap.setReadOnly(mapProperty.isReadOnly()); propDefMap.setSummary(mapProperty.isSummary());
diff --git a/modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/PluginMetadataParser.java b/modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/PluginMetadataParser.java index 7e7e574..e5d58b2 100644 --- a/modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/PluginMetadataParser.java +++ b/modules/core/client-api/src/main/java/org/rhq/core/clientapi/agent/metadata/PluginMetadataParser.java @@ -602,7 +602,7 @@ public class PluginMetadataParser { }
try { - resourceType.setPlugin(pluginDescriptor.getName()); + resourceType.setPlugin(pluginDescriptor.getName().intern());
if (resourceDescriptor.getPluginConfiguration() != null) { resourceType.setPluginConfigurationDefinition(ConfigurationMetadataParser.parse(resourceType.getName(), @@ -744,7 +744,11 @@ public class PluginMetadataParser { * @return the resource discovery component class name */ public String getDiscoveryComponentClass(ResourceType resourceType) { - return this.discoveryClasses.get(resourceType); + String s = this.discoveryClasses.get(resourceType); + if (s!=null) { + s = s.intern(); + } + return s; }
/** @@ -755,15 +759,19 @@ public class PluginMetadataParser { * @return the resource component class name */ public String getComponentClass(ResourceType resourceType) { - return this.componentClasses.get(resourceType); + String s = this.componentClasses.get(resourceType); + if (s!=null) { + s=s.intern(); + } + return s; }
private void registerResourceTypeAndComponentClasses(ResourceType resourceType, String discoveryClass, String componentClass) { this.resourceTypes.add(resourceType); - this.componentClasses.put(resourceType, componentClass); + this.componentClasses.put(resourceType, componentClass.intern()); if (discoveryClass != null) { - this.discoveryClasses.put(resourceType, discoveryClass); + this.discoveryClasses.put(resourceType, discoveryClass.intern()); } }
commit e7cc75bfa28c5cb28c9f55ab91dfa005b6dd2d53 Author: Heiko W. Rupp hwr@redhat.com Date: Mon Dec 16 11:38:27 2013 +0100
Synchronize on final fields
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 5b22e3e..6c0703c 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 @@ -1,6 +1,6 @@ /* * RHQ Management Platform - * Copyright (C) 2005-2008 Red Hat, Inc. + * Copyright (C) 2005-2013 Red Hat, Inc. * All rights reserved. * * This program is free software; you can redistribute it and/or modify @@ -69,7 +69,7 @@ public class PluginMetadataManager {
private Map<ResourceCategory, LinkedHashSet<ResourceType>> typesByCategory = new HashMap<ResourceCategory, LinkedHashSet<ResourceType>>(); private Set<ResourceType> types = new HashSet<ResourceType>(); - private Object typesLock = new Object(); + private final Object typesLock = new Object();
private Map<String, PluginMetadataParser> parsersByPlugin = new HashMap<String, PluginMetadataParser>();
@@ -80,12 +80,13 @@ public class PluginMetadataManager { private List<String> disabledResourceTypesAsStrings = null; private Map<ResourceType, String> disabledResourceTypes = null; private Set<ResourceType> ignoredResourceTypes = null; - private Object disabledIgnoredTypesLock = new Object(); // used when accessing disabled and ignored collections + private final Object disabledIgnoredTypesLock = new Object(); // used when accessing disabled and ignored collections
// these define the discovery callbacks per resource type. The key is the resource type whose discovered details // need to be funneled through callbacks. The value is a map whose key is plugin names and whose values are // discovery callback implementation classes defined in the plugins. private Map<ResourceType, Map<String, List<String>>> discoveryCallbacks = new HashMap<ResourceType, Map<String, List<String>>>(); + private final Object discoveryCallbacksLock = new Object();
public PluginMetadataManager() { } @@ -516,14 +517,14 @@ public class PluginMetadataManager { * @return the collection of callbacks, grouped by the plugins that defined them (may be null) */ public Map<String, List<String>> getDiscoveryCallbacks(ResourceType resourceType) { - synchronized (discoveryCallbacks) { + synchronized (discoveryCallbacksLock) { Map<String, List<String>> map = discoveryCallbacks.get(resourceType); return map; } }
private void addDiscoveryCallbackClassName(ResourceType resourceType, String pluginName, String className) { - synchronized (discoveryCallbacks) { + synchronized (discoveryCallbacksLock) { Map<String, List<String>> map = discoveryCallbacks.get(resourceType); if (map == null) { map = new HashMap<String, List<String>>(1);
commit 80c42d099588802c7b12af7a51baf1640f0e4374 Author: Heiko W. Rupp hwr@redhat.com Date: Mon Dec 16 11:33:37 2013 +0100
When cloning, directly allocate a collection of the right size.
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 27e9dc3..64dc441 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 @@ -953,6 +953,11 @@ public class Configuration implements Serializable, Cloneable, AbstractPropertyM if (properties==null) { return; } + + if (copy.properties==null) { + copy.properties=new LinkedHashMap<String, Property>(this.properties.size()); + } + for (Property property : this.properties.values()) { copy.put(property.deepCopy(keepId)); }
commit 2fb20f60d6acdc2108ecbedb9091849a5aed0847 Author: Heiko W. Rupp hwr@redhat.com Date: Mon Dec 16 09:34:59 2013 +0100
Pull out determination of log.trace
diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/AvailabilityExecutor.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/AvailabilityExecutor.java index e278062..c23809d 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/AvailabilityExecutor.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/AvailabilityExecutor.java @@ -169,8 +169,9 @@ public class AvailabilityExecutor implements Runnable, Callable<AvailabilityRepo parentAvailabilityType = UP; }
+ boolean traceEnabled = LOG.isTraceEnabled(); try { - checkInventory(scanRoot, availabilityReport, parentAvailabilityType, false, scan); + checkInventory(scanRoot, availabilityReport, parentAvailabilityType, false, scan, traceEnabled); } catch (InterruptedException e) { LOG.debug("Availability check was interrupted", e); return; @@ -217,14 +218,14 @@ public class AvailabilityExecutor implements Runnable, Callable<AvailabilityRepo * @throws InterruptedException if this checking thread was interrupted */ protected void checkInventory(Resource resource, AvailabilityReport availabilityReport, - AvailabilityType parentAvailType, boolean isForced, Scan scan) throws InterruptedException { + AvailabilityType parentAvailType, boolean isForced, Scan scan, boolean traceEnabled) throws InterruptedException {
// Only report avail for committed Resources - that's all the Server cares about. if (resource.getId() == 0 || resource.getInventoryStatus() != InventoryStatus.COMMITTED) { return; }
- ResourceContainer resourceContainer = this.inventoryManager.getResourceContainer(resource); + ResourceContainer resourceContainer = this.inventoryManager.getResourceContainer(resource.getId()); // Only report avail for synchronized Resources, otherwise the Server will likely know nothing of the Resource. if (resourceContainer == null || resourceContainer.getSynchronizationState() != ResourceContainer.SynchronizationState.SYNCHRONIZED) { @@ -248,7 +249,7 @@ public class AvailabilityExecutor implements Runnable, Callable<AvailabilityRepo // if there is no availability schedule (platform) then just perform the avail check // (note, platforms always return UP anyway). if (null == availScheduleRequest) { - if (LOG.isTraceEnabled()) { + if (traceEnabled) { LOG.trace("No availScheduleRequest for " + resource + ". checkAvail set to true"); } checkAvail = true; @@ -263,14 +264,14 @@ public class AvailabilityExecutor implements Runnable, Callable<AvailabilityRepo availabilityScheduleTime = scan.startTime + RANDOM.nextInt(interval + 1); resourceContainer.setAvailabilityScheduleTime(availabilityScheduleTime);
- if (LOG.isTraceEnabled()) { + if (traceEnabled) { LOG.trace("Forced availabilityScheduleTime to " + new Date(availabilityScheduleTime) + " for " + resource); } ++scan.numScheduledRandomly;
} else { - if (LOG.isTraceEnabled()) { + if (traceEnabled) { LOG.trace("Deferred availability to parent for " + resource); } deferToParent = true; @@ -281,14 +282,14 @@ public class AvailabilityExecutor implements Runnable, Callable<AvailabilityRepo checkAvail = scan.startTime >= availabilityScheduleTime;
if (checkAvail) { - if (LOG.isTraceEnabled()) { + if (traceEnabled) { LOG.trace("Scheduled time has been reached for " + resource); } long interval = availScheduleRequest.getInterval(); // intervals are short enough for safe cast resourceContainer.setAvailabilityScheduleTime(scan.startTime + interval); ++scan.numPushedByInterval; } else { - if (LOG.isTraceEnabled()) { + if (traceEnabled) { LOG.trace("Scheduled time has not been reached for " + resource); } } @@ -311,7 +312,7 @@ public class AvailabilityExecutor implements Runnable, Callable<AvailabilityRepo // the child. For now, we'll leave it alone and let the next check happen according to the // schedule already established.
- if (LOG.isTraceEnabled()) { + if (traceEnabled) { LOG.trace("Gave parent availability " + parentAvailType + " to " + resource); } } else { @@ -322,7 +323,7 @@ public class AvailabilityExecutor implements Runnable, Callable<AvailabilityRepo }
if (checkAvail) { - if (LOG.isTraceEnabled()) { + if (traceEnabled) { LOG.trace("Now checking availability for " + resource); }
@@ -346,7 +347,7 @@ public class AvailabilityExecutor implements Runnable, Callable<AvailabilityRepo current = DOWN; } } - if (LOG.isTraceEnabled()) { + if (traceEnabled) { LOG.trace("Current availability is " + current + " for " + resource); } } catch (Throwable t) { @@ -370,7 +371,7 @@ public class AvailabilityExecutor implements Runnable, Callable<AvailabilityRepo Availability availability;
if (availChanged) { - if (LOG.isTraceEnabled()) { + if (traceEnabled) { LOG.trace("Availability changed for " + resource); } ++scan.numAvailabilityChanges; @@ -381,7 +382,7 @@ public class AvailabilityExecutor implements Runnable, Callable<AvailabilityRepo // children, to ensure their avails are up to date. Note that if it changed to NOT UP // then the children will just get the parent avail type and there is no avail check anyway. if (!isForced && (UP == current)) { - if (LOG.isTraceEnabled()) { + if (traceEnabled) { LOG.trace("Forcing availability check for children of " + resource); } isForced = true; @@ -396,7 +397,7 @@ public class AvailabilityExecutor implements Runnable, Callable<AvailabilityRepo }
for (Resource child : this.inventoryManager.getContainerChildren(resource, resourceContainer)) { - checkInventory(child, availabilityReport, current, isForced, scan); + checkInventory(child, availabilityReport, current, isForced, scan, traceEnabled); }
} diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/ForceAvailabilityExecutor.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/ForceAvailabilityExecutor.java index 53475fd..74a4901 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/ForceAvailabilityExecutor.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/ForceAvailabilityExecutor.java @@ -37,9 +37,9 @@ public class ForceAvailabilityExecutor extends AvailabilityExecutor {
@Override protected void checkInventory(Resource resource, AvailabilityReport availabilityReport, - AvailabilityType parentAvailType, boolean forceCheck, Scan scan) throws InterruptedException { + AvailabilityType parentAvailType, boolean forceCheck, Scan scan, boolean traceEnabled) throws InterruptedException {
scan.setForced(true); - super.checkInventory(resource, availabilityReport, parentAvailType, true, scan); + super.checkInventory(resource, availabilityReport, parentAvailType, true, scan, traceEnabled); } } 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 5265977..fd423a7 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 @@ -161,7 +161,7 @@ public class RuntimeDiscoveryExecutor implements Runnable, Callable<InventoryRep
log.debug("Discovering child Resources for " + parent + "...");
- ResourceContainer parentContainer = this.inventoryManager.getResourceContainer(parent); + ResourceContainer parentContainer = this.inventoryManager.getResourceContainer(parent.getId()); if (parentContainer == null) { if (log.isDebugEnabled()) { log.debug("Cannot perform service scan on parent [" + parent + "] without a container");
rhq-commits@lists.fedorahosted.org