[rhq] modules/enterprise

Jay Shaughnessy jshaughn at fedoraproject.org
Tue Aug 14 21:17:41 UTC 2012


 modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/configuration/metadata/ConfigurationMetadataManagerBean.java |   50 ++++++----
 modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/metadata/ResourceMetadataManagerBean.java           |   32 ++----
 modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/metadata/ResourceMetadataManagerLocal.java          |    3 
 modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/resource/metadata/test/UpdateConfigurationSubsystemTest.java |    2 
 4 files changed, 52 insertions(+), 35 deletions(-)

New commits:
commit c9eb53bb8dab7393d9b5383fbe5bab99088487ed
Author: Jay Shaughnessy <jshaughn at redhat.com>
Date:   Tue Aug 14 17:13:42 2012 -0400

    [Bug 846353 - ORA-02049 during upgrade from JON-3.1.0.GA to JON-3.1.1.ER1 with Oracle]
    After a lot of investigation it seems that the problem occurs occasionally
    when we remove obsolete properties from resource configuration.  It does
    not happen every time, on fact it's fairly rare, although for the same DB,
    and the same plugin update, it is repeatable.  This makes it seem like the
    locking issue is due mainly to unpredictable locking at the db level, and
    likely the fact that we occasionally hit a page lock due to some other
    prior update to the config table.  Since the config table stores so many
    different types of data it's not obvious how we would identify the conflict.
    
    The approach taken was to try and reduce possible conflict by increasing the
    granularity of metadata update transactions.  Prior to this change we used a
    single encompassing transaction for a plugin update, that means all types
    were updated under one umbrella transaction.  Not one transaction, because
    we already use nested transactions in several places, but using one umbrella
    transaction increases the chance of that transaction holding a lock that
    could affect a nested transaction.
    
    We still maintain the umbrella transaction but this commit breaks it up
    such that a nested transaction is used for the update of each resource
    type in the plugin.
    
    That means each type update will not hold any locks when it has completed.
    This change seems to be working as the AS7 plugin now updates successfully.
    
    Additionally:
    - added some more INFO level logging to give some basic progress during a
      plugin update.
    - added some more debug logging as well
    - removed a bunch of unnecessary em.flush calls
    - used the return value of some em.merge calls to ensure using the up to
      date entity.

diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/configuration/metadata/ConfigurationMetadataManagerBean.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/configuration/metadata/ConfigurationMetadataManagerBean.java
index dcc1a0b..b343f79 100644
--- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/configuration/metadata/ConfigurationMetadataManagerBean.java
+++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/configuration/metadata/ConfigurationMetadataManagerBean.java
@@ -73,24 +73,27 @@ public class ConfigurationMetadataManagerBean implements ConfigurationMetadataMa
             for (PropertyDefinition newProperty : newPropertyDefinitions) {
                 PropertyDefinition existingProp = existingDefinition.get(newProperty.getName());
                 if (existingProp != null) {
+                    log.debug("Updating nonGrouped property [" + existingProp + "]");
+
                     updatePropertyDefinition(existingProp, newProperty);
                     updateReport.addUpdatedPropertyDefinition(newProperty);
                 } else {
+                    log.debug("Adding nonGrouped property [" + newProperty + "]");
+
                     existingDefinition.put(newProperty);
                     updateReport.addNewPropertyDefinition(newProperty);
                 }
             }
 
-            // delete outdated properties
-            removeNoLongerUsedProperties(newDefinition, existingDefinition, existingPropertyDefinitions);
+            existingDefinition = removeNoLongerUsedProperties(newDefinition, existingDefinition,
+                existingPropertyDefinitions);
+
         } else {
             // TODO what if existingDefinitions is null?
             // we probably don't run in here, as the initial persisting is done
             // somewhere else.
         }
 
-        //        entityManager.flush();
-
         /*
          * Now update / delete contained groups We need to be careful here, as groups are present in PropertyDefinition
          * as "backlink" from PropertyDefinition to group
@@ -108,6 +111,8 @@ public class ConfigurationMetadataManagerBean implements ConfigurationMetadataMa
 
             // first look for contained stuff
             for (PropertyDefinition def : groupedDefinitions) {
+                log.debug("Removing property [" + def + "] from group [" + group + "]");
+
                 existingPropertyDefinitions.remove(def);
                 existingDefinition.getPropertyDefinitions().remove(def.getName());
                 def.setPropertyGroupDefinition(null);
@@ -115,41 +120,44 @@ public class ConfigurationMetadataManagerBean implements ConfigurationMetadataMa
             }
 
             // then remove the definition itself
+            log.debug("Removing group [" + group + "]");
+
             existingGroups.remove(group);
             entityManager.remove(group);
         }
 
-        //        entityManager.flush();
-
         // update existing groups that stay
         for (PropertyGroupDefinition group : toUpdate) {
             String groupName = group.getName();
 
-            //         System.out.println("Group to update: " + groupName + ", id=" + group.getId());
             List<PropertyDefinition> newGroupedDefinitions = newDefinition.getPropertiesInGroup(groupName);
             for (PropertyDefinition nDef : newGroupedDefinitions) {
                 PropertyDefinition existingProperty = existingDefinition.getPropertyDefinitions().get(nDef.getName());
                 if (existingProperty != null) {
+                    log.debug("Updating property [" + nDef + "] in group [" + group + "]");
+
                     updatePropertyDefinition(existingProperty, nDef);
                     updateReport.addUpdatedPropertyDefinition(nDef);
+
                 } else {
+                    log.debug("Adding property [" + nDef + "] to group [" + group + "]");
+
                     existingDefinition.put(nDef);
                     updateReport.addNewPropertyDefinition(nDef);
                 }
             }
 
             // delete outdated properties of this group
-            removeNoLongerUsedProperties(newDefinition, existingDefinition,
+            existingDefinition = removeNoLongerUsedProperties(newDefinition, existingDefinition,
                 existingDefinition.getPropertiesInGroup(groupName));
         }
 
-        //        entityManager.flush();
-
         // persist new groups
         for (PropertyGroupDefinition group : toPersist) {
-            /*
-             * First persist a new group definition and then link the properties to it
-             */
+
+            // First persist a new group definition and then link the properties to it
+            log.debug("Persisting new group [" + group + "]");
+
             entityManager.persist(group);
             existingGroups.add(group); // iterating over this does not update the underlying crap
 
@@ -185,17 +193,22 @@ public class ConfigurationMetadataManagerBean implements ConfigurationMetadataMa
         }
 
         for (String name : toRemove) {
+            log.debug("Removing template [" + name + "]");
+
             ConfigurationTemplate template = existingTemplates.remove(name);
             entityManager.remove(template);
         }
 
         for (String name : templatesToUpdate) {
+            log.debug("Updating template [" + name + "]");
+
             updateTemplate(existingDefinition.getTemplate(name), newTemplates.get(name));
         }
 
         for (String name : newTemplates.keySet()) {
             // add completely new templates
             if (!existingTemplates.containsKey(name)) {
+                log.debug("Adding template [" + name + "]");
 
                 ConfigurationTemplate newTemplate = newTemplates.get(name);
 
@@ -279,8 +292,9 @@ public class ConfigurationMetadataManagerBean implements ConfigurationMetadataMa
      * @param existingConfigDef  existing persisted configuration
      * @param existingProperties list of existing properties to inspect for potential removal
      */
-    private void removeNoLongerUsedProperties(ConfigurationDefinition newConfigDef,
+    private ConfigurationDefinition removeNoLongerUsedProperties(ConfigurationDefinition newConfigDef,
         ConfigurationDefinition existingConfigDef, List<PropertyDefinition> existingProperties) {
+
         List<PropertyDefinition> propDefsToDelete = new ArrayList<PropertyDefinition>();
         for (PropertyDefinition existingPropDef : existingProperties) {
             PropertyDefinition newPropDef = newConfigDef.get(existingPropDef.getName());
@@ -296,8 +310,10 @@ public class ConfigurationMetadataManagerBean implements ConfigurationMetadataMa
                 existingConfigDef.getPropertyDefinitions().remove(propDef.getName());
                 existingProperties.remove(propDef); // does not operate on original list!!
             }
-            entityManager.merge(existingConfigDef);
+            existingConfigDef = entityManager.merge(existingConfigDef);
         }
+
+        return existingConfigDef;
     }
 
     /**
@@ -360,7 +376,7 @@ public class ConfigurationMetadataManagerBean implements ConfigurationMetadataMa
                     }
                 }
 
-                entityManager.merge(existingProperty);
+                existingProperty = entityManager.merge(existingProperty);
 
             } else { // different type
 
@@ -412,7 +428,7 @@ public class ConfigurationMetadataManagerBean implements ConfigurationMetadataMa
                         }
                     }
                 }
-                entityManager.merge(existingPDS);
+                existingPDS = entityManager.merge(existingPDS);
 
                 // handle <constraint> [0..*]
 
diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/metadata/ResourceMetadataManagerBean.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/metadata/ResourceMetadataManagerBean.java
index 032deb3..2c8a925 100644
--- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/metadata/ResourceMetadataManagerBean.java
+++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/metadata/ResourceMetadataManagerBean.java
@@ -153,7 +153,7 @@ public class ResourceMetadataManagerBean implements ResourceMetadataManagerLocal
         Set<ResourceType> legitimateChildren = new HashSet<ResourceType>();
         for (ResourceType resourceType : nonRunsInsideResourceTypes) {
             long startTime = System.currentTimeMillis();
-            updateType(resourceType);
+            resourceType = resourceMetadataManager.updateType(resourceType);
             long endTime = System.currentTimeMillis();
             log.debug("Updated resource type [" + toConciseString(resourceType) + "] in " + (endTime - startTime)
                 + " ms");
@@ -181,7 +181,7 @@ public class ResourceMetadataManagerBean implements ResourceMetadataManagerLocal
             resourceMetadataManager.getPluginTypes(subject, pluginName, legitTypes, obsoleteTypes, metadataCache);
 
             if (!obsoleteTypes.isEmpty()) {
-                log.debug("Removing " + obsoleteTypes.size() + " obsolete types: " + obsoleteTypes + "...");
+                log.info("Removing " + obsoleteTypes.size() + " obsolete types: " + obsoleteTypes + "...");
                 removeResourceTypes(subject, obsoleteTypes, new HashSet<ResourceType>(obsoleteTypes));
             }
 
@@ -361,21 +361,19 @@ public class ResourceMetadataManagerBean implements ResourceMetadataManagerLocal
         }
     }
 
-    private void updateType(ResourceType resourceType) {
-        entityManager.flush();
+    @TransactionAttribute(TransactionAttributeType.REQUIRES_NEW)
+    public ResourceType updateType(ResourceType resourceType) {
 
         // see if there is already an existing type that we need to update
-        if (log.isDebugEnabled()) {
-            log.debug("Updating resource type [" + resourceType.getName() + "] from plugin ["
-                + resourceType.getPlugin() + "]...");
-        }
+        log.info("Updating resource type [" + toConciseString(resourceType) + "]...");
 
         ResourceType existingType;
         try {
             existingType = resourceTypeManager.getResourceTypeByNameAndPlugin(resourceType.getName(),
                 resourceType.getPlugin());
+
         } catch (NonUniqueResultException nure) {
-            log.debug("Found more than one existing ResourceType for " + resourceType);
+            log.info("Found more than one existing ResourceType for " + resourceType);
             // TODO: Delete the redundant ResourceTypes to get the DB into a valid state.
             throw new IllegalStateException(nure);
         }
@@ -390,6 +388,8 @@ public class ResourceMetadataManagerBean implements ResourceMetadataManagerLocal
         } else {
             mergeExistingType(resourceType, existingType);
         }
+
+        return resourceType;
     }
 
     private void mergeExistingType(ResourceType resourceType, ResourceType existingType) {
@@ -427,7 +427,7 @@ public class ResourceMetadataManagerBean implements ResourceMetadataManagerLocal
         // Update the type itself
         if (resourceType.getCategory() != existingType.getCategory()) {
             log.info("Changing category of Resource type [" + resourceType + "] from " + existingType.getCategory()
-                    + " to " + resourceType.getCategory() + "...");
+                + " to " + resourceType.getCategory() + "...");
             existingType.setCategory(resourceType.getCategory());
         }
 
@@ -447,7 +447,7 @@ public class ResourceMetadataManagerBean implements ResourceMetadataManagerLocal
             // above already took care of any modifications to the ResourceSubCategories themselves).
         } else if (newSubCat == null) {
             if (oldSubCat != null) {
-                log.debug("Metadata update: Subcategory of ResourceType [" + resourceType.getName() + "] changed from "
+                log.info("Metadata update: Subcategory of ResourceType [" + resourceType.getName() + "] changed from "
                     + oldSubCat + " to " + newSubCat);
                 existingType.setSubCategory(null);
             }
@@ -459,13 +459,12 @@ public class ResourceMetadataManagerBean implements ResourceMetadataManagerLocal
                 throw new IllegalStateException("Resource type [" + resourceType.getName() + "] in plugin ["
                     + resourceType.getPlugin() + "] has a subcategory (" + newSubCat.getName()
                     + ") which was not defined as a child subcategory of one of its ancestor resource types.");
-            log.debug("Metadata update: Subcategory of ResourceType [" + resourceType.getName() + "] changed from "
+            log.info("Metadata update: Subcategory of ResourceType [" + resourceType.getName() + "] changed from "
                 + oldSubCat + " to " + existingSubCat);
             existingType.setSubCategory(existingSubCat);
         }
 
         existingType = entityManager.merge(existingType);
-        entityManager.flush();
     }
 
     @Override
@@ -600,7 +599,6 @@ public class ResourceMetadataManagerBean implements ResourceMetadataManagerLocal
         MeasurementMetadataManagerBean.getMetricDefinitions(resourceType);
 
         entityManager.persist(resourceType);
-        entityManager.flush();
     }
 
     private void linkSubCategoryToParents(ResourceType resourceType) {
@@ -665,8 +663,6 @@ public class ResourceMetadataManagerBean implements ResourceMetadataManagerLocal
             obsoleteParentType.removeChildResourceType(existingType);
             moveResourcesToNewParent(existingType, obsoleteParentType, newParentTypes);
         }
-
-        entityManager.flush();
     }
 
     private static String toConciseString(ResourceType type) {
@@ -711,7 +707,7 @@ public class ResourceMetadataManagerBean implements ResourceMetadataManagerLocal
                 // children are not handled in this method, update their ancestry now.
                 resourceManager.updateAncestry(subjectManager.getOverlord(), resource.getId());
             } else {
-                log.debug("We were unable to move " + resource + " from invalid parent " + resource.getParentResource()
+                log.info("We were unable to move " + resource + " from invalid parent " + resource.getParentResource()
                     + " to a new valid parent with one of the following types: " + newParentTypes);
             }
         }
@@ -842,7 +838,7 @@ public class ResourceMetadataManagerBean implements ResourceMetadataManagerLocal
         // simply use the ones from the new type
         if ((existingSubCat.getChildSubCategories() == null) || existingSubCat.getChildSubCategories().isEmpty()) {
             for (ResourceSubCategory newChildSubCategory : newSubCategory.getChildSubCategories()) {
-                log.debug("Metadata update: Adding new child SubCategory [" + newChildSubCategory.getName()
+                log.info("Metadata update: Adding new child SubCategory [" + newChildSubCategory.getName()
                     + "] to SubCategory [" + existingSubCat.getName() + "]...");
                 existingSubCat.addChildSubCategory(newChildSubCategory);
                 entityManager.persist(newChildSubCategory);
diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/metadata/ResourceMetadataManagerLocal.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/metadata/ResourceMetadataManagerLocal.java
index ae96d22..b40bf7a 100644
--- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/metadata/ResourceMetadataManagerLocal.java
+++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/metadata/ResourceMetadataManagerLocal.java
@@ -46,6 +46,9 @@ public interface ResourceMetadataManagerLocal {
 
     void updateTypes(Set<ResourceType> resourceTypes) throws Exception;
 
+    /** Exists only to for transactional boundary reasons. Not for general consumption. */
+    ResourceType updateType(ResourceType resourceType) throws Exception;
+
     /** TODO: do we want to create a driftMetadataManager SLSB and put this in there */
     void updateDriftMetadata(ResourceType existingType, ResourceType resourceType);
 }
\ No newline at end of file
diff --git a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/resource/metadata/test/UpdateConfigurationSubsystemTest.java b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/resource/metadata/test/UpdateConfigurationSubsystemTest.java
index 20f3b9e..4714f57 100644
--- a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/resource/metadata/test/UpdateConfigurationSubsystemTest.java
+++ b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/resource/metadata/test/UpdateConfigurationSubsystemTest.java
@@ -176,6 +176,8 @@ public class UpdateConfigurationSubsystemTest extends UpdatePluginMetadataTestBa
         }
     }
 
+    // This test fails but I'm not sure why.  It may be a bad test or it may be a real problem. It should
+    // be investigated and either enabled or deleted.
     @Test(enabled = false)
     public void testBZ_573034() throws Exception {
         // Note, plugins are registered in new transactions. for tests, this means




More information about the rhq-commits mailing list