Gracefully stop HT threads when the bundle is being stopped (cache terminated exception) 55/1055/1
authorAlessandro Boch <aboch@cisco.com>
Thu, 29 Aug 2013 23:58:31 +0000 (16:58 -0700)
committerAlessandro Boch <aboch@cisco.com>
Fri, 30 Aug 2013 00:05:14 +0000 (17:05 -0700)
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 <aboch@cisco.com>
opendaylight/hosttracker/api/src/main/java/org/opendaylight/controller/hosttracker/hostAware/HostNodeConnector.java
opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/Activator.java
opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTracker.java
opendaylight/hosttracker/implementation/src/test/java/org/opendaylight/controller/hosttracker/internal/HostTrackerTest.java

index 206124b93bf67a2fc995e960e36bc0bdae21942a..eb9a436ed55db7171a833c67b1bf9451e6bff28b 100644 (file)
@@ -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);
     }
 
index e60b02d83b881a06f215644d7af8216369921357..3af9826b7e38efe8306a452f0ca2406a6e7d1698 100644 (file)
@@ -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<String, Object> props = new Hashtable<String, Object>();
@@ -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)) {
-        }
-    }
 }
index 367df5ebb05e87e7b93737f9a9db03bd43075e9f..a6e1a32c54c257aecce985219f9f1aaa035a71d4 100644 (file)
@@ -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<InetAddress, HostNodeConnector> hostsDB;
+    protected IHostFinder hostFinder;
+    protected ConcurrentMap<InetAddress, HostNodeConnector> 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<IfNewHostNotify> newHostNotify = Collections.synchronizedSet(new HashSet<IfNewHostNotify>());
 
     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<InetAddress, ARPPending> 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<InetAddress, ARPPending>();
     }
 
-
     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 <InetAddress, ARPPending> entry : ARPPendingList.entrySet()) {
+        for (Entry<InetAddress, ARPPending> 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<Node, Set<Edge>> 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<Node> nodes = null;
         if (switchManager == null) {
@@ -721,8 +725,9 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw
     @Override
     public List<List<String>> getHostNetworkHierarchy(InetAddress hostAddress) {
         HostNodeConnector host = hostQuery(hostAddress);
-        if (host == null)
+        if (host == null) {
             return null;
+        }
 
         List<List<String>> hierarchies = new ArrayList<List<String>>();
         ArrayList<String> currHierarchy = new ArrayList<String>();
@@ -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 <InetAddress, ARPPending> entry : failedARPReqList.entrySet()) {
+            for (Entry<InetAddress, ARPPending> 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 <InetAddress, ARPPending> entry : ARPPendingList.entrySet()) {
+            for (Entry<InetAddress, ARPPending> 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<String, Property> 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<String, Property> 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 <InetAddress, ARPPending> entry : failedARPReqList.entrySet()) {
+        for (Entry<InetAddress, ARPPending> 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 <InetAddress, ARPPending> entry : ARPPendingList.entrySet()) {
+        for (Entry<InetAddress, ARPPending> 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 <InetAddress, ARPPending> entry : failedARPReqList.entrySet()) {
+        for (Entry<InetAddress, ARPPending> entry : failedARPReqList.entrySet()) {
             arphost = entry.getValue();
             ci.println(arphost.getHostIP().toString());
         }
index 25de544f52e8d0dd79da9afff83e2eda1e28a145..dc15f96f6efadb2f727f1a469cbb59d9a921f1b9 100644 (file)
@@ -10,13 +10,10 @@ package org.opendaylight.controller.hosttracker.internal;
 \r
 import java.net.InetAddress;\r
 import java.net.UnknownHostException;\r
-import java.util.concurrent.Future;\r
-\r
 import junit.framework.TestCase;\r
 \r
 import org.junit.Assert;\r
 import org.junit.Test;\r
-import org.opendaylight.controller.hosttracker.hostAware.HostNodeConnector;\r
 \r
 public class HostTrackerTest extends TestCase {\r
 \r
@@ -36,7 +33,7 @@ public class HostTrackerTest extends TestCase {
 \r
         long count = htCallable.latch.getCount();\r
         htCallable.wakeup();\r
-        Assert.assertTrue(htCallable.latch.getCount() == --count);\r
+        Assert.assertTrue(htCallable.latch.getCount() == (count - 1));\r
     }\r
 \r
     @Test\r
@@ -47,8 +44,8 @@ public class HostTrackerTest extends TestCase {
 \r
         InetAddress hostIP_1 = InetAddress.getByName("192.168.0.8");\r
         InetAddress hostIP_2 = InetAddress.getByName("192.168.0.18");\r
-        Future<HostNodeConnector> dschost = hostTracker.discoverHost(hostIP_1);\r
-        dschost = hostTracker.discoverHost(hostIP_2);\r
+        hostTracker.discoverHost(hostIP_1);\r
+        hostTracker.discoverHost(hostIP_2);\r
         hostTracker.nonClusterObjectCreate();\r
     }\r
 \r