From: Chi-Vien Ly Date: Thu, 25 Jul 2013 05:51:03 +0000 (-0700) Subject: Out of Memory when static Routes are configured with unresolved next hop X-Git-Tag: releasepom-0.1.0~262^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=a2e03732c76b35b28622be16bd3c1c0c5bd7c2b5 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 --- 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 4afd4fb8e4..bccdea4162 100644 --- 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 staticRoutingAware = Collections .synchronizedSet(new HashSet()); + 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 { 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 future = hostTracker - .discoverHost(staticRoute.getNextHopAddress()); + log.debug("Next hop {} is not present, try to discover it", nh.getHostAddress()); + Future 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(); 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 6b7bac03e9..5aa8f10706 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 @@ -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 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 06311a5206..303308270d 100644 --- 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 { 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); }