From 5d05491a829a3ad4d9986989878e2a11e0c176f1 Mon Sep 17 00:00:00 2001 From: Alejandro Galue Date: Fri, 25 Jul 2014 11:54:40 -0400 Subject: [PATCH 1/4] Fix for Bug NMS-6757 High CPU usage due to DataCollectionConfigDao.getConfiguredResourceTypes() while Collectd starts. --- .../DefaultDataCollectionConfigDao.java | 77 ++++++------------- .../config/DataCollectionConfigDaoTest.java | 13 +++- 2 files changed, 35 insertions(+), 55 deletions(-) diff --git a/opennms-config/src/main/java/org/opennms/netmgt/config/DefaultDataCollectionConfigDao.java b/opennms-config/src/main/java/org/opennms/netmgt/config/DefaultDataCollectionConfigDao.java index 0578c1a9ba46..ff05ece557cd 100644 --- a/opennms-config/src/main/java/org/opennms/netmgt/config/DefaultDataCollectionConfigDao.java +++ b/opennms-config/src/main/java/org/opennms/netmgt/config/DefaultDataCollectionConfigDao.java @@ -53,7 +53,6 @@ import org.opennms.netmgt.config.datacollection.SystemDefChoice; import org.opennms.netmgt.config.datacollection.Systems; import org.opennms.netmgt.model.RrdRepository; -import org.springframework.core.io.Resource; /** * DefaultDataCollectionConfigDao @@ -68,23 +67,13 @@ public class DefaultDataCollectionConfigDao extends AbstractJaxbConfigDao dataCollectionGroups = new ArrayList(); + private Map resourceTypes = new HashMap(); public DefaultDataCollectionConfigDao() { super(DatacollectionConfig.class, "data-collection"); } - @Override - protected DatacollectionConfig loadConfig(final Resource resource) { - m_validated = false; - m_validationException = null; - return super.loadConfig(resource); - } - @Override protected DatacollectionConfig translateConfig(final DatacollectionConfig config) { final DataCollectionConfigParser parser = new DataCollectionConfigParser(getConfigDirectory()); @@ -97,14 +86,19 @@ protected DatacollectionConfig translateConfig(final DatacollectionConfig config // Create a special collection to hold all resource types, because they should be defined only once. final SnmpCollection resourceTypeCollection = new SnmpCollection(); resourceTypeCollection.setName("__resource_type_collection"); + resourceTypes.clear(); for (final ResourceType rt : parser.getAllResourceTypes()) { resourceTypeCollection.addResourceType(rt); + resourceTypes.put(rt.getName(), rt); } resourceTypeCollection.setGroups(new Groups()); resourceTypeCollection.setSystems(new Systems()); config.getSnmpCollectionCollection().add(0, resourceTypeCollection); dataCollectionGroups.clear(); dataCollectionGroups.addAll(parser.getExternalGroupMap().keySet()); + + validateResourceTypes(config.getSnmpCollectionCollection(), resourceTypes.keySet()); + return config; } @@ -283,30 +277,7 @@ public List getMibObjectList(final String cName, final String aSysoid @Override public Map getConfiguredResourceTypes() { - final Map map = new HashMap(); - - final Collection snmpCollections = getContainer().getObject().getSnmpCollectionCollection(); - for (final SnmpCollection collection : snmpCollections) { - for (final ResourceType resourceType : collection.getResourceTypeCollection()) { - map.put(resourceType.getName(), resourceType); - } - } - - // FIXME: I guarantee there's a cleaner way to do this, but I didn't want to refactor everything - // that calls this just to optimize out validation. - if (!m_validated) { - try { - validateResourceTypes(getContainer(), map.keySet()); - } catch (final RuntimeException e) { - m_validationException = e; - throw e; - } - } else { - if (m_validationException != null) { - throw m_validationException; - } - } - return Collections.unmodifiableMap(map); + return Collections.unmodifiableMap(resourceTypes); } @Override @@ -571,7 +542,7 @@ private static Map> getCollectionGroupMap(FileReloadCon return Collections.unmodifiableMap(collectionGroupMap); } - private static void validateResourceTypes(final FileReloadContainer container, final Set allowedResourceTypes) { + private void validateResourceTypes(final Collection snmpCollections, final Set allowedResourceTypes) { final String configuredString; if (allowedResourceTypes.size() == 0) { configuredString = "(none)"; @@ -580,26 +551,26 @@ private static void validateResourceTypes(final FileReloadContainer= 0) { - continue; - } - } catch (NumberFormatException e) {} - - // XXX this should be a better exception - throw new IllegalArgumentException("instance '" + instance + "' invalid in mibObj definition for OID '" + mibObj.getOid() + "' in collection '" + collection.getName() + "' for group '" + group.getName() + "'. Allowable instance values: " + allowableValues); - } - } + try { + // Check to see if the value is a non-negative integer + if (Integer.parseInt(instance.trim()) >= 0) { + continue; + } + } catch (NumberFormatException e) {} + + // XXX this should be a better exception + throw new IllegalArgumentException("instance '" + instance + "' invalid in mibObj definition for OID '" + mibObj.getOid() + "' in collection '" + collection.getName() + "' for group '" + group.getName() + "'. Allowable instance values: " + allowableValues); + } + } } } } diff --git a/opennms-dao/src/test/java/org/opennms/netmgt/config/DataCollectionConfigDaoTest.java b/opennms-dao/src/test/java/org/opennms/netmgt/config/DataCollectionConfigDaoTest.java index 41bcf3d53094..a81ab6211136 100644 --- a/opennms-dao/src/test/java/org/opennms/netmgt/config/DataCollectionConfigDaoTest.java +++ b/opennms-dao/src/test/java/org/opennms/netmgt/config/DataCollectionConfigDaoTest.java @@ -31,11 +31,9 @@ import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; - import org.opennms.core.test.OpenNMSJUnit4ClassRunner; import org.opennms.core.test.db.annotations.JUnitTemporaryDatabase; import org.opennms.test.JUnitConfigurationEnvironment; - import org.springframework.test.context.ContextConfiguration; @RunWith(OpenNMSJUnit4ClassRunner.class) @@ -54,4 +52,15 @@ public void testDefaultReloadInterval() { Assert.assertTrue(config instanceof DefaultDataCollectionConfigDao); Assert.assertEquals(new Long(60000), ((DefaultDataCollectionConfigDao) config).getReloadCheckInterval()); } + + @Test + public void testResourceTypes() { + long start = System.nanoTime(); + DataCollectionConfigDao config = DataCollectionConfigFactory.getInstance(); + for (int i=0; i<1000; i++) { + Assert.assertNotNull(config.getConfiguredResourceTypes().get("hrStorageIndex")); + } + long end = System.nanoTime(); + System.err.println("Test took " + ((end - start)/1000) + " us"); + } } From c5a2f013cad17dcf71141ffb736eeecb811149de Mon Sep 17 00:00:00 2001 From: Alejandro Galue Date: Fri, 25 Jul 2014 14:36:42 -0400 Subject: [PATCH 2/4] Fix for Bug NMS-6757 - Part II It turns out that the current implementation of the DefaultDataCollectionConfigDao class was not adding the resource types defined inside the snmp-collection (like the old format of the files), it was checking only the resource types defined on the datacolelction-group files. --- .../config/DefaultDataCollectionConfigDao.java | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/opennms-config/src/main/java/org/opennms/netmgt/config/DefaultDataCollectionConfigDao.java b/opennms-config/src/main/java/org/opennms/netmgt/config/DefaultDataCollectionConfigDao.java index ff05ece557cd..1b8af6ebfbd3 100644 --- a/opennms-config/src/main/java/org/opennms/netmgt/config/DefaultDataCollectionConfigDao.java +++ b/opennms-config/src/main/java/org/opennms/netmgt/config/DefaultDataCollectionConfigDao.java @@ -77,20 +77,30 @@ public DefaultDataCollectionConfigDao() { @Override protected DatacollectionConfig translateConfig(final DatacollectionConfig config) { final DataCollectionConfigParser parser = new DataCollectionConfigParser(getConfigDirectory()); + resourceTypes.clear(); + + // Create a special collection to hold all resource types, because they should be defined only once. + final SnmpCollection resourceTypeCollection = new SnmpCollection(); + resourceTypeCollection.setName("__resource_type_collection"); // Updating Configured Collections for (final SnmpCollection collection : config.getSnmpCollectionCollection()) { parser.parseCollection(collection); + // Save local resource types + for (final ResourceType rt : collection.getResourceTypeCollection()) { + resourceTypeCollection.addResourceType(rt); + resourceTypes.put(rt.getName(), rt); + } + // Remove local resource types + collection.getResourceTypeCollection().clear(); } - // Create a special collection to hold all resource types, because they should be defined only once. - final SnmpCollection resourceTypeCollection = new SnmpCollection(); - resourceTypeCollection.setName("__resource_type_collection"); - resourceTypes.clear(); + // Save external Resource Types for (final ResourceType rt : parser.getAllResourceTypes()) { resourceTypeCollection.addResourceType(rt); resourceTypes.put(rt.getName(), rt); } + resourceTypeCollection.setGroups(new Groups()); resourceTypeCollection.setSystems(new Systems()); config.getSnmpCollectionCollection().add(0, resourceTypeCollection); From 4ff91688ce19fc2e3a5ee5f613dfb1c3c42905f8 Mon Sep 17 00:00:00 2001 From: Alejandro Galue Date: Fri, 25 Jul 2014 14:44:08 -0400 Subject: [PATCH 3/4] NMS-6757 - Part II - Fixing compiling issues --- .../opennms/netmgt/config/DefaultDataCollectionConfigDao.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opennms-config/src/main/java/org/opennms/netmgt/config/DefaultDataCollectionConfigDao.java b/opennms-config/src/main/java/org/opennms/netmgt/config/DefaultDataCollectionConfigDao.java index 96f5d84b794d..f9aa96f92a3e 100644 --- a/opennms-config/src/main/java/org/opennms/netmgt/config/DefaultDataCollectionConfigDao.java +++ b/opennms-config/src/main/java/org/opennms/netmgt/config/DefaultDataCollectionConfigDao.java @@ -91,12 +91,12 @@ protected DatacollectionConfig translateConfig(final DatacollectionConfig config for (final SnmpCollection collection : config.getSnmpCollections()) { parser.parseCollection(collection); // Save local resource types - for (final ResourceType rt : collection.getResourceTypeCollection()) { + for (final ResourceType rt : collection.getResourceTypes()) { resourceTypeCollection.addResourceType(rt); resourceTypes.put(rt.getName(), rt); } // Remove local resource types - collection.getResourceTypeCollection().clear(); + collection.setResourceTypes(new ArrayList()); } // Save external Resource Types From caa3decb7f6ea3e62b9ecb44ebe6bf77019a794b Mon Sep 17 00:00:00 2001 From: Jeff Gehlbach Date: Sat, 26 Jul 2014 13:37:24 -0400 Subject: [PATCH 4/4] Squashed commit of the following: commit 61b93196340e0d65ba79e54375fa0e0ce2985daf Merge: 8a0242d c5a2f01 Author: Jeff Gehlbach Date: Sat Jul 26 09:06:53 2014 -0400 Merge branch '1.12' of github.com:OpenNMS/opennms into feature/iplike-search-placeholder commit 8a0242d9a9381bff9077eeace009843687eb502d Author: Jeff Gehlbach Date: Mon Jul 21 13:55:47 2014 -0400 Change IPLIKE search boxes to have empty value and "*.*.*.*" in placeholder --- opennms-webapp/src/main/webapp/element/index.jsp | 2 +- opennms-webapp/src/main/webapp/includes/quicksearch-box.jsp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/opennms-webapp/src/main/webapp/element/index.jsp b/opennms-webapp/src/main/webapp/element/index.jsp index 1c7e5f3c2a99..e61e222a132e 100644 --- a/opennms-webapp/src/main/webapp/element/index.jsp +++ b/opennms-webapp/src/main/webapp/element/index.jsp @@ -75,7 +75,7 @@

TCP/IP Address like: - +

diff --git a/opennms-webapp/src/main/webapp/includes/quicksearch-box.jsp b/opennms-webapp/src/main/webapp/includes/quicksearch-box.jsp index bc2d4f6c5b05..2023b7112683 100644 --- a/opennms-webapp/src/main/webapp/includes/quicksearch-box.jsp +++ b/opennms-webapp/src/main/webapp/includes/quicksearch-box.jsp @@ -60,7 +60,7 @@
TCP/IP Address like:
- +