From: Alessandro Boch Date: Thu, 29 Aug 2013 23:58:31 +0000 (-0700) Subject: Gracefully stop HT threads when the bundle is being stopped (cache terminated exception) X-Git-Tag: releasepom-0.1.0~145^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=6c927a9467f3b38ba6acd4e4d1631e902fbf9557 Gracefully stop HT threads when the bundle is being stopped (cache terminated exception) ISSUE: When HT bundle is being stopped because container is being destroyed, infinispan cache terminated exceptions are thrown because HT treads during their last run are accessing terminated HT caches. CHANGE: - Have a stopping flag which gets set when dep mgr calls the HT stopping() method, so that pending threads and timer tasks can be stopped before Cluster Mgr terminates the HT caches. - Fix other static analysis issues in HT code Change-Id: I3eedb81f9db31c8419502d2eba02aa421616f06a Signed-off-by: Alessandro Boch --- diff --git a/opendaylight/hosttracker/api/src/main/java/org/opendaylight/controller/hosttracker/hostAware/HostNodeConnector.java b/opendaylight/hosttracker/api/src/main/java/org/opendaylight/controller/hosttracker/hostAware/HostNodeConnector.java index 206124b93b..eb9a436ed5 100644 --- a/opendaylight/hosttracker/api/src/main/java/org/opendaylight/controller/hosttracker/hostAware/HostNodeConnector.java +++ b/opendaylight/hosttracker/api/src/main/java/org/opendaylight/controller/hosttracker/hostAware/HostNodeConnector.java @@ -8,7 +8,6 @@ package org.opendaylight.controller.hosttracker.hostAware; -import java.io.Serializable; import java.net.Inet4Address; import java.net.Inet6Address; import java.net.InetAddress; @@ -27,7 +26,7 @@ import org.opendaylight.controller.sal.packet.address.EthernetAddress; @XmlRootElement(name = "host") @XmlAccessorType(XmlAccessType.NONE) -public class HostNodeConnector extends Host implements Serializable { +public class HostNodeConnector extends Host { private static final long serialVersionUID = 1L; @XmlElement private NodeConnector nodeConnector; @@ -136,22 +135,29 @@ public class HostNodeConnector extends Host implements Serializable { @Override public boolean equals(Object obj) { - if (this == obj) + if (this == obj) { return true; - if (!super.equals(obj)) + } + if (!super.equals(obj)) { return false; - if (getClass() != obj.getClass()) + } + if (getClass() != obj.getClass()) { return false; + } HostNodeConnector other = (HostNodeConnector) obj; if (nodeConnector == null) { - if (other.nodeConnector != null) + if (other.nodeConnector != null) { return false; - } else if (!nodeConnector.equals(other.nodeConnector)) + } + } else if (!nodeConnector.equals(other.nodeConnector)) { return false; - if (staticHost != other.staticHost) + } + if (staticHost != other.staticHost) { return false; - if (vlan != other.vlan) + } + if (vlan != other.vlan) { return false; + } return true; } @@ -167,8 +173,9 @@ public class HostNodeConnector extends Host implements Serializable { EthernetAddress e = (EthernetAddress) getDataLayerAddress(); macaddr = e.getValue(); } - if (macaddr == null) + if (macaddr == null) { return false; + } return !Arrays.equals(emptyArray, macaddr); } diff --git a/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/Activator.java b/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/Activator.java index e60b02d83b..3af9826b7e 100644 --- a/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/Activator.java +++ b/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/Activator.java @@ -38,6 +38,7 @@ public class Activator extends ComponentActivatorAbstractBase { * are done by the ComponentActivatorAbstractBase. * */ + @Override public void init() { } @@ -46,6 +47,7 @@ public class Activator extends ComponentActivatorAbstractBase { * ComponentActivatorAbstractBase * */ + @Override public void destroy() { } @@ -58,6 +60,7 @@ public class Activator extends ComponentActivatorAbstractBase { * instantiated in order to get an fully working implementation * Object */ + @Override public Object[] getImplementations() { Object[] res = { HostTracker.class }; return res; @@ -78,6 +81,7 @@ public class Activator extends ComponentActivatorAbstractBase { * per-container different behavior if needed, usually should not * be the case though. */ + @Override public void configureInstance(Component c, Object imp, String containerName) { if (imp.equals(HostTracker.class)) { Dictionary props = new Hashtable(); @@ -116,36 +120,4 @@ public class Activator extends ComponentActivatorAbstractBase { .setRequired(false)); } } - - /** - * Method which tells how many Global implementations are supported by the - * bundle. This way we can tune the number of components created. This - * components will be created ONLY at the time of bundle startup and will be - * destroyed only at time of bundle destruction, this is the major - * difference with the implementation retrieved via getImplementations where - * all of them are assumed to be in a container ! - * - * - * @return The list of implementations the bundle will support, in Global - * version - */ - protected Object[] getGlobalImplementations() { - return null; - } - - /** - * Configure the dependency for a given instance Global - * - * @param c - * Component assigned for this instance, this will be what will - * be used for configuration - * @param imp - * implementation to be configured - * @param containerName - * container on which the configuration happens - */ - protected void configureGlobalInstance(Component c, Object imp) { - if (imp.equals(HostTracker.class)) { - } - } } diff --git a/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTracker.java b/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTracker.java index 367df5ebb0..a6e1a32c54 100644 --- a/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTracker.java +++ b/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTracker.java @@ -88,8 +88,8 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw static final String ACTIVE_HOST_CACHE = "hosttracker.ActiveHosts"; static final String INACTIVE_HOST_CACHE = "hosttracker.InactiveHosts"; private static final Logger logger = LoggerFactory.getLogger(HostTracker.class); - private IHostFinder hostFinder; - private ConcurrentMap hostsDB; + protected IHostFinder hostFinder; + protected ConcurrentMap hostsDB; /* * Following is a list of hosts which have been requested by NB APIs to be * added, but either the switch or the port is not sup, so they will be @@ -99,12 +99,13 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw private final Set newHostNotify = Collections.synchronizedSet(new HashSet()); private ITopologyManager topologyManager; - private IClusterContainerServices clusterContainerService = null; - private ISwitchManager switchManager = null; + protected IClusterContainerServices clusterContainerService = null; + protected ISwitchManager switchManager = null; private Timer timer; private Timer arpRefreshTimer; private String containerName = null; private ExecutorService executor; + protected boolean stopping; private static class ARPPending { protected InetAddress hostIP; protected short sent_count; @@ -134,6 +135,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw hostTrackerCallable = callable; } } + // This list contains the hosts for which ARP requests are being sent // periodically ConcurrentMap ARPPendingList; @@ -162,6 +164,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw nonClusterObjectCreate(); allocateCache(); retrieveCache(); + stopping = false; timer = new Timer(); timer.schedule(new OutStandingARPHandler(), 4000, 4000); @@ -172,7 +175,6 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw logger.debug("startUp: Caches created, timers started"); } - @SuppressWarnings("deprecation") private void allocateCache() { if (this.clusterContainerService == null) { logger.error("un-initialized clusterContainerService, can't create cache"); @@ -192,7 +194,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw logger.debug("Cache successfully created for HostTracker"); } - @SuppressWarnings({ "unchecked", "deprecation" }) + @SuppressWarnings({ "unchecked" }) private void retrieveCache() { if (this.clusterContainerService == null) { logger.error("un-initialized clusterContainerService, can't retrieve cache"); @@ -221,7 +223,6 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw failedARPReqList = new ConcurrentHashMap(); } - public void shutDown() { } @@ -335,7 +336,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw /* Add this host to ARPPending List for any potential retries */ - AddtoARPPendingList(networkAddress); + addToARPPendingList(networkAddress); logger.debug("hostFind(): Host Not Found for IP: {}, Inititated Host Discovery ...", networkAddress.getHostAddress()); @@ -379,7 +380,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw return list; } - private void AddtoARPPendingList(InetAddress networkAddr) { + private void addToARPPendingList(InetAddress networkAddr) { ARPPending arphost = new ARPPending(); arphost.setHostIP(networkAddr); @@ -390,7 +391,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw public void setCallableOnPendingARP(InetAddress networkAddr, HostTrackerCallable callable) { ARPPending arphost; - for (Entry entry : ARPPendingList.entrySet()) { + for (Entry entry : ARPPendingList.entrySet()) { arphost = entry.getValue(); if (arphost.getHostIP().equals(networkAddr)) { arphost.setHostTrackerCallable(callable); @@ -405,8 +406,9 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw // Remove the arphost from ARPPendingList as it has been learned now logger.debug("Host Removed from ARPPending List, IP: {}", networkAddr); HostTrackerCallable htCallable = arphost.getHostTrackerCallable(); - if (htCallable != null) + if (htCallable != null) { htCallable.wakeup(); + } return; } @@ -414,7 +416,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw * It could have been a host from the FailedARPReqList */ - if (failedARPReqList.containsKey(networkAddr)) { + if (failedARPReqList.containsKey(networkAddr)) { failedARPReqList.remove(networkAddr); logger.debug("Host Removed from FailedARPReqList List, IP: {}", networkAddr); } @@ -503,14 +505,12 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw replaceHost(networkAddr, removedHost, host); return; } else { - logger.error("Host to be removed not found in hostsDB. Host {}", removedHost); + logger.error("Host to be removed not found in hostsDB"); } } - if (removedHost == null) { - // It is a new host - learnNewHost(host); - } + // It is a new host + learnNewHost(host); /* check if there is an outstanding request for this host */ processPendingARPReqs(networkAddr); @@ -600,6 +600,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw * @param currentTier * The Tier on which n belongs */ + @SuppressWarnings("unused") private void updateSwitchTiers(Node n, int currentTier) { Map> ndlinks = topologyManager.getNodeEdges(); if (ndlinks == null) { @@ -661,12 +662,14 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw } // This is the case where Tier was never set for this node Tier t = (Tier) switchManager.getNodeProp(n, Tier.TierPropName); - if (t == null) + if (t == null) { return true; - if (t.getValue() == 0) + } + if (t.getValue() == 0) { return true; - else if (t.getValue() > tier) + } else if (t.getValue() > tier) { return true; + } return false; } @@ -675,6 +678,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw * cleanup is performed during cases such as Topology Change where the * existing Tier values might become incorrect */ + @SuppressWarnings("unused") private void clearTiers() { Set nodes = null; if (switchManager == null) { @@ -721,8 +725,9 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw @Override public List> getHostNetworkHierarchy(InetAddress hostAddress) { HostNodeConnector host = hostQuery(hostAddress); - if (host == null) + if (host == null) { return null; + } List> hierarchies = new ArrayList>(); ArrayList currHierarchy = new ArrayList(); @@ -750,10 +755,12 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw int result = 0; for (int i = 0; i < hex.length(); i++) { result = (int) ((dpid >> (i * 8)) & 0xff); - if (result == 0) + if (result == 0) { continue; - if (result < 0x30) + } + if (result < 0x30) { result += 0x40; + } sb.append(String.format("%c", result)); } return sb.reverse().toString(); @@ -900,15 +907,15 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw public void subnetNotify(Subnet sub, boolean add) { logger.debug("Received subnet notification: {} add={}", sub, add); if (add) { - for (Entry entry : failedARPReqList.entrySet()) { + for (Entry entry : failedARPReqList.entrySet()) { ARPPending arphost; arphost = entry.getValue(); if (hostFinder == null) { logger.warn("ARPHandler Services are not available on subnet addition"); continue; } - logger.debug("Sending the ARP from FailedARPReqList fors IP: {}", arphost.getHostIP().getHostAddress()); - hostFinder.find(arphost.getHostIP()); + logger.debug("Sending the ARP from FailedARPReqList fors IP: {}", arphost.getHostIP().getHostAddress()); + hostFinder.find(arphost.getHostIP()); } } } @@ -916,16 +923,19 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw class OutStandingARPHandler extends TimerTask { @Override public void run() { + if (stopping) { + return; + } ARPPending arphost; - /* This routine runs every 4 seconds */ logger.trace("Number of Entries in ARP Pending/Failed Lists: ARPPendingList = {}, failedARPReqList = {}", ARPPendingList.size(), failedARPReqList.size()); - for (Entry entry : ARPPendingList.entrySet()) { + for (Entry entry : ARPPendingList.entrySet()) { arphost = entry.getValue(); if (hostsDB.containsKey(arphost.getHostIP())) { - // this host is already learned, shouldn't be in ARPPendingList + // this host is already learned, shouldn't be in + // ARPPendingList // Remove it and continue logger.warn("Learned Host {} found in ARPPendingList", arphost.getHostIP()); ARPPendingList.remove(entry.getKey()); @@ -934,8 +944,8 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw if (arphost.getSent_count() < switchManager.getHostRetryCount()) { /* * No reply has been received of first ARP Req, send the - * next one. Before sending the ARP, check if ARPHandler - * is available or not + * next one. Before sending the ARP, check if ARPHandler is + * available or not */ if (hostFinder == null) { logger.warn("ARPHandler Services are not available for Outstanding ARPs"); @@ -946,8 +956,8 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw logger.debug("ARP Sent from ARPPending List, IP: {}", arphost.getHostIP().getHostAddress()); } else if (arphost.getSent_count() >= switchManager.getHostRetryCount()) { /* - * ARP requests have been sent without receiving a - * reply, remove this from the pending list + * ARP requests have been sent without receiving a reply, + * remove this from the pending list */ ARPPendingList.remove(entry.getKey()); logger.debug("ARP reply not received after multiple attempts, removing from Pending List IP: {}", @@ -968,8 +978,10 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw private class ARPRefreshHandler extends TimerTask { @Override - @SuppressWarnings("deprecation") public void run() { + if (stopping) { + return; + } if ((clusterContainerService != null) && !clusterContainerService.amICoordinator()) { return; } @@ -1222,8 +1234,9 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw @Override public void notifyNode(Node node, UpdateType type, Map propMap) { - if (node == null) + if (node == null) { return; + } switch (type) { case REMOVED: @@ -1244,8 +1257,9 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw @Override public void notifyNodeConnector(NodeConnector nodeConnector, UpdateType type, Map propMap) { - if (nodeConnector == null) + if (nodeConnector == null) { return; + } boolean up = false; switch (type) { @@ -1303,7 +1317,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw logger.debug("handleNodeConnectorStatusUp {}", nodeConnector); - for (Entry entry : failedARPReqList.entrySet()) { + for (Entry entry : failedARPReqList.entrySet()) { arphost = entry.getValue(); logger.debug("Sending the ARP from FailedARPReqList fors IP: {}", arphost.getHostIP().getHostAddress()); if (hostFinder == null) { @@ -1374,8 +1388,9 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw } public String getContainerName() { - if (containerName == null) + if (containerName == null) { return GlobalConstants.DEFAULT.toString(); + } return containerName; } @@ -1420,42 +1435,40 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw * calls * */ - void stop(){ + void stop() { } void stopping() { + stopping = true; arpRefreshTimer.cancel(); timer.cancel(); - executor.shutdown(); + executor.shutdownNow(); } @Override public void edgeOverUtilized(Edge edge) { - // TODO Auto-generated method stub } @Override public void edgeUtilBackToNormal(Edge edge) { - // TODO Auto-generated method stub } @Override - public void entryCreated(InetAddress key, String cacheName, - boolean originLocal) { - if (originLocal) return; + public void entryCreated(InetAddress key, String cacheName, boolean originLocal) { + if (originLocal) { + return; + } processPendingARPReqs(key); } @Override - public void entryUpdated(InetAddress key, HostNodeConnector new_value, - String cacheName, boolean originLocal) { + public void entryUpdated(InetAddress key, HostNodeConnector new_value, String cacheName, boolean originLocal) { } @Override - public void entryDeleted(InetAddress key, String cacheName, - boolean originLocal) { + public void entryDeleted(InetAddress key, String cacheName, boolean originLocal) { } private void registerWithOSGIConsole() { @@ -1465,13 +1478,12 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw @Override public String getHelp() { - // TODO Auto-generated method stub return null; } public void _dumpPendingARPReqList(CommandInterpreter ci) { ARPPending arphost; - for (Entry entry : ARPPendingList.entrySet()) { + for (Entry entry : ARPPendingList.entrySet()) { arphost = entry.getValue(); ci.println(arphost.getHostIP().toString()); } @@ -1479,7 +1491,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw public void _dumpFailedARPReqList(CommandInterpreter ci) { ARPPending arphost; - for (Entry entry : failedARPReqList.entrySet()) { + for (Entry entry : failedARPReqList.entrySet()) { arphost = entry.getValue(); ci.println(arphost.getHostIP().toString()); } diff --git a/opendaylight/hosttracker/implementation/src/test/java/org/opendaylight/controller/hosttracker/internal/HostTrackerTest.java b/opendaylight/hosttracker/implementation/src/test/java/org/opendaylight/controller/hosttracker/internal/HostTrackerTest.java index 25de544f52..dc15f96f6e 100644 --- a/opendaylight/hosttracker/implementation/src/test/java/org/opendaylight/controller/hosttracker/internal/HostTrackerTest.java +++ b/opendaylight/hosttracker/implementation/src/test/java/org/opendaylight/controller/hosttracker/internal/HostTrackerTest.java @@ -10,13 +10,10 @@ package org.opendaylight.controller.hosttracker.internal; import java.net.InetAddress; import java.net.UnknownHostException; -import java.util.concurrent.Future; - import junit.framework.TestCase; import org.junit.Assert; import org.junit.Test; -import org.opendaylight.controller.hosttracker.hostAware.HostNodeConnector; public class HostTrackerTest extends TestCase { @@ -36,7 +33,7 @@ public class HostTrackerTest extends TestCase { long count = htCallable.latch.getCount(); htCallable.wakeup(); - Assert.assertTrue(htCallable.latch.getCount() == --count); + Assert.assertTrue(htCallable.latch.getCount() == (count - 1)); } @Test @@ -47,8 +44,8 @@ public class HostTrackerTest extends TestCase { InetAddress hostIP_1 = InetAddress.getByName("192.168.0.8"); InetAddress hostIP_2 = InetAddress.getByName("192.168.0.18"); - Future dschost = hostTracker.discoverHost(hostIP_1); - dschost = hostTracker.discoverHost(hostIP_2); + hostTracker.discoverHost(hostIP_1); + hostTracker.discoverHost(hostIP_2); hostTracker.nonClusterObjectCreate(); }