Out of Memory when static Routes are configured with unresolved next hop

Root cause: This problem happens when there are staticRoutes configured but their
next hop IP are unresolved (ie. not responding to ARP Request).
StaticRouteImpl.java spawns a thread requesting hostTracker to discover the host,
this thread is waiting forever due to a bug in the thread (HostTrackerCallable)
that handles the discovery which never returns since the host is not responding.

Solution:
- Instead of creating threads dynamically, we create a threadpool of 2 and reuse
  the threads.
- Fix the bug in HostTrackerCallable to sleep(2000) and returns.

Change-Id: Ic04d1dbaf17d5cd6672646050d71abb9885695e9
Signed-off-by: Chi-Vien Ly <chivly@cisco.com>

diff --git a/opendaylight/forwarding/staticrouting/src/main/java/org/opendaylight/controller/forwarding/staticrouting/internal/StaticRoutingImplementation.java b/opendaylight/forwarding/staticrouting/src/main/java/org/opendaylight/controller/forwarding/staticrouting/internal/StaticRoutingImplementation.java
index 4afd4fb8e444136562b2d59fff79728c74bc763f..bccdea416208ef27d30294b9f7ddcebf83a77ba2 100644 (file)
--- a/opendaylight/forwarding/staticrouting/src/main/java/org/opendaylight/controller/forwarding/staticrouting/internal/StaticRoutingImplementation.java
+++ b/opendaylight/forwarding/staticrouting/src/main/java/org/opendaylight/controller/forwarding/staticrouting/internal/StaticRoutingImplementation.java
@@ -24,8 +24,11 @@ import java.util.Map;
 import java.util.Set;
 import java.util.Timer;
 import java.util.TimerTask;
+import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -55,9 +58,6 @@ import org.slf4j.LoggerFactory;
 
 /**
  * Static Routing feature provides the bridge between SDN and Non-SDN networks.
- *
- *
- *
  */
 public class StaticRoutingImplementation implements IfNewHostNotify,
         IForwardingStaticRouting, IObjectReader, IConfigurationContainerAware,
@@ -75,6 +75,7 @@ public class StaticRoutingImplementation implements IfNewHostNotify,
     private IClusterContainerServices clusterContainerService = null;
     private Set<IStaticRoutingAware> staticRoutingAware = Collections
             .synchronizedSet(new HashSet<IStaticRoutingAware>());
+    private ExecutorService executor;
 
     void setStaticRoutingAware(IStaticRoutingAware s) {
         if (this.staticRoutingAware != null) {
@@ -235,43 +236,53 @@ public class StaticRoutingImplementation implements IfNewHostNotify,
         }
     }
 
-    private class NotifyStaticRouteThread extends Thread {
+    private class NotifyStaticRouteWorker implements Callable<Object> {
         private StaticRoute staticRoute;
         private boolean added;
 
-        public NotifyStaticRouteThread(StaticRoute s, boolean update) {
+        public NotifyStaticRouteWorker(StaticRoute s, boolean update) {
             this.staticRoute = s;
             this.added = update;
         }
 
-        public void run() {
+        @Override
+        public Object call() throws Exception {
             if (!added
                     || (staticRoute.getType() == StaticRoute.NextHopType.SWITCHPORT)) {
                 notifyStaticRouteUpdate(staticRoute, added);
             } else {
-                HostNodeConnector host = hostTracker.hostQuery(staticRoute
-                        .getNextHopAddress());
+                InetAddress nh = staticRoute.getNextHopAddress();
+                HostNodeConnector host = hostTracker.hostQuery(nh);
                 if (host == null) {
-                    Future<HostNodeConnector> future = hostTracker
-                            .discoverHost(staticRoute.getNextHopAddress());
+                    log.debug("Next hop {}  is not present, try to discover it", nh.getHostAddress());
+                    Future<HostNodeConnector> future = hostTracker.discoverHost(nh);
                     if (future != null) {
                         try {
                             host = future.get();
                         } catch (Exception e) {
-                            log.error("",e);
+                            log.error("", e);
                         }
                     }
                 }
                 if (host != null) {
+                    log.debug("Next hop {} is found", nh.getHostAddress());
                     staticRoute.setHost(host);
                     notifyStaticRouteUpdate(staticRoute, added);
+                } else {
+                    log.debug("Next hop {}  is still not present, try again later", nh.getHostAddress());
                 }
             }
+            return null;
         }
     }
 
     private void checkAndUpdateListeners(StaticRoute staticRoute, boolean added) {
-        new NotifyStaticRouteThread(staticRoute, added).start();
+        NotifyStaticRouteWorker worker = new NotifyStaticRouteWorker(staticRoute, added);
+        try {
+            executor.submit(worker);
+        } catch (Exception e) {
+            log.error("got Exception ", e);
+        }
     }
 
     private void notifyHostUpdate(HostNodeConnector host, boolean added) {
@@ -440,7 +451,7 @@ public class StaticRoutingImplementation implements IfNewHostNotify,
         //staticRoutes = new ConcurrentHashMap<String, StaticRoute>();
         allocateCaches();
         retrieveCaches();
-
+        this.executor = Executors.newFixedThreadPool(1);
         if (staticRouteConfigs.isEmpty())
             loadConfiguration();
 
@@ -494,10 +505,12 @@ public class StaticRoutingImplementation implements IfNewHostNotify,
      *
      */
     void stop() {
+        executor.shutdown();
     }
 
     @Override
     public Status saveConfiguration() {
         return this.saveConfig();
     }
+
 }

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 6b7bac03e9bdcfc472ad3dce7cc2c10d1f8dd096..5aa8f10706cae4e774455895fc30642a1fa8049d 100644 (file)
--- 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
@@ -97,7 +97,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw
     private Timer timer;
     private Timer arp_refresh_timer;
     private String containerName = null;
-
+    private ExecutorService executor;
     private static class ARPPending {
         protected InetAddress hostIP;
         protected short sent_count;
@@ -158,7 +158,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw
 
         timer = new Timer();
         timer.schedule(new OutStandingARPHandler(), 4000, 4000);
-
+        executor = Executors.newFixedThreadPool(2);
         /* ARP Refresh Timer to go off every 5 seconds to implement ARP aging */
         arp_refresh_timer = new Timer();
         arp_refresh_timer.schedule(new ARPRefreshHandler(), 5000, 5000);
@@ -296,7 +296,6 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw
 
     @Override
     public Future<HostNodeConnector> discoverHost(InetAddress networkAddress) {
-        ExecutorService executor = Executors.newFixedThreadPool(1);
         if (executor == null) {
             logger.error("discoverHost: Null executor");
             return null;
@@ -1349,6 +1348,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw
      *
      */
     void stop() {
+        executor.shutdown();
     }
 
     @Override

diff --git a/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTrackerCallable.java b/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTrackerCallable.java
index 06311a5206268ae79fe8ebe32970a395423cf162..303308270d77d36eb803b42feca3096bbfeed0fb 100644 (file)
--- a/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTrackerCallable.java
+++ b/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTrackerCallable.java
@@ -42,7 +42,7 @@ public class HostTrackerCallable implements Callable<HostNodeConnector> {
         if (h != null)
             return h;
         hostTracker.setCallableOnPendingARP(trackedHost, this);
-        latch.await();
+        Thread.sleep(2000); // wait 2sec to see if the host responds
         return hostTracker.hostQuery(trackedHost);
     }