From 78405e6699eb9c85e8b0c3e2fd85a798f0bcbd55 Mon Sep 17 00:00:00 2001 From: Yevgeny Khodorkovsky Date: Mon, 10 Mar 2014 19:15:42 -0700 Subject: [PATCH] When node disconnects from master controller, no node takes over - Node connections caches in connection manager were allocated after configureInstance call, thus failing CacheUpdateAware registration. - Fix couple of NPEs in devices.web Change-Id: I51975a4a984606ecfad8db0941315164eedc83cb Signed-off-by: Yevgeny Khodorkovsky --- .../internal/ClusterGlobalManager.java | 5 +- .../internal/ClusterManagerCommon.java | 10 ++-- .../internal/ConnectionManager.java | 38 ++++++------- .../scheme/AbstractScheme.java | 54 ++++++++++++++++++- .../controller/devices/web/Devices.java | 14 ++--- 5 files changed, 84 insertions(+), 37 deletions(-) diff --git a/opendaylight/clustering/services_implementation/src/main/java/org/opendaylight/controller/clustering/services_implementation/internal/ClusterGlobalManager.java b/opendaylight/clustering/services_implementation/src/main/java/org/opendaylight/controller/clustering/services_implementation/internal/ClusterGlobalManager.java index 34ddb7a207..e05f9dfe16 100644 --- a/opendaylight/clustering/services_implementation/src/main/java/org/opendaylight/controller/clustering/services_implementation/internal/ClusterGlobalManager.java +++ b/opendaylight/clustering/services_implementation/src/main/java/org/opendaylight/controller/clustering/services_implementation/internal/ClusterGlobalManager.java @@ -10,6 +10,7 @@ package org.opendaylight.controller.clustering.services_implementation.internal; import java.util.Map; + import org.opendaylight.controller.clustering.services.ICacheUpdateAware; import org.opendaylight.controller.clustering.services.IClusterGlobalServices; import org.slf4j.Logger; @@ -22,7 +23,7 @@ public class ClusterGlobalManager @Override void setCacheUpdateAware(Map props, ICacheUpdateAware s) { - logger.trace("setCacheUpdateAware"); + logger.trace("setCacheUpdateAware: {}",s); if (props.get("containerName") != null) { // If we got a reference with the containerName property // that is not what we are looking for, so filter it out. @@ -33,7 +34,7 @@ public class ClusterGlobalManager @Override void unsetCacheUpdateAware(Map props, ICacheUpdateAware s) { - logger.trace("unsetCacheUpdateAware"); + logger.trace("unsetCacheUpdateAware: {}",s); if (props.get("containerName") != null) { // If we got a reference with the containerName property // that is not what we are looking for, so filter it out. diff --git a/opendaylight/clustering/services_implementation/src/main/java/org/opendaylight/controller/clustering/services_implementation/internal/ClusterManagerCommon.java b/opendaylight/clustering/services_implementation/src/main/java/org/opendaylight/controller/clustering/services_implementation/internal/ClusterManagerCommon.java index 97d9ded6c8..06e5bc5b61 100644 --- a/opendaylight/clustering/services_implementation/src/main/java/org/opendaylight/controller/clustering/services_implementation/internal/ClusterManagerCommon.java +++ b/opendaylight/clustering/services_implementation/src/main/java/org/opendaylight/controller/clustering/services_implementation/internal/ClusterManagerCommon.java @@ -58,6 +58,7 @@ public abstract class ClusterManagerCommon implements IClusterServicesCommon { * export the interface ICoordinatorChangeAware */ class ListenCoordinatorChange implements IListenRoleChange { + @Override public void newActiveAvailable() { if (coordinatorChangeAware != null) { // Make sure to look the set while walking it @@ -93,13 +94,9 @@ public abstract class ClusterManagerCommon implements IClusterServicesCommon { logger.trace("cachenames provided below:"); for (String cache : caches) { if (this.cacheUpdateAware.get(cache) != null) { - logger.error("cachename:{} on container:{} has " + - "already a listener", cache, - this.containerName); + logger.error("cachename:{} on container:{} has already a listener", cache, this.containerName); } else { - GetUpdatesContainer up = - new GetUpdatesContainer(s, this.containerName, - cache); + GetUpdatesContainer up = new GetUpdatesContainer(s, this.containerName, cache); if (up != null) { try { this.clusterService.addListener(this.containerName, @@ -109,6 +106,7 @@ public abstract class ClusterManagerCommon implements IClusterServicesCommon { "been registered", cache, this.containerName); } catch (CacheListenerAddException exc) { + logger.debug("Cache {} didn't exist when {} tried to register to its updates", cache, s); // Do nothing, the important is that // we don't register the listener in // the shadow, and we are not doing diff --git a/opendaylight/connectionmanager/implementation/src/main/java/org/opendaylight/controller/connectionmanager/internal/ConnectionManager.java b/opendaylight/connectionmanager/implementation/src/main/java/org/opendaylight/controller/connectionmanager/internal/ConnectionManager.java index 773c6cac50..ebc56928a2 100644 --- a/opendaylight/connectionmanager/implementation/src/main/java/org/opendaylight/controller/connectionmanager/internal/ConnectionManager.java +++ b/opendaylight/connectionmanager/implementation/src/main/java/org/opendaylight/controller/connectionmanager/internal/ConnectionManager.java @@ -127,17 +127,6 @@ public class ConnectionManager implements IConnectionManager, } public void started() { - String schemeStr = System.getProperty("connection.scheme"); - for (ConnectionMgmtScheme scheme : ConnectionMgmtScheme.values()) { - AbstractScheme schemeImpl = SchemeFactory.getScheme(scheme, - clusterServices); - if (schemeImpl != null) { - schemes.put(scheme, schemeImpl); - if (scheme.name().equalsIgnoreCase(schemeStr)) { - activeScheme = scheme; - } - } - } connectionEventThread.start(); @@ -152,6 +141,17 @@ public class ConnectionManager implements IConnectionManager, "ConnectionEvent Thread"); this.connectionEvents = new LinkedBlockingQueue(); schemes = new ConcurrentHashMap(); + + String schemeStr = System.getProperty("connection.scheme"); + for (ConnectionMgmtScheme scheme : ConnectionMgmtScheme.values()) { + AbstractScheme schemeImpl = SchemeFactory.getScheme(scheme, clusterServices); + if (schemeImpl != null) { + schemes.put(scheme, schemeImpl); + if (scheme.name().equalsIgnoreCase(schemeStr)) { + activeScheme = scheme; + } + } + } } public void stopping() { @@ -290,7 +290,7 @@ public class ConnectionManager implements IConnectionManager, } /* - * Clustering Services' doesnt provide the existing states in the cache + * Clustering Services doesn't provide the existing states in the cache * update callbacks. Hence, using a scratch local cache to maintain the * existing state. */ @@ -303,21 +303,17 @@ public class ConnectionManager implements IConnectionManager, return; Set existingControllers = existingConnections.get(node); if (existingControllers != null) { - logger.debug( - "Processing Update for : {} NewControllers : {} existingControllers : {}", - node, newControllers.toString(), - existingControllers.toString()); + logger.debug("Processing Update for : {} NewControllers : {} existingControllers : {}", node, + newControllers.toString(), existingControllers.toString()); if (newControllers.size() < existingControllers.size()) { - Set removed = new HashSet( - existingControllers); + Set removed = new HashSet(existingControllers); if (removed.removeAll(newControllers)) { logger.debug("notifyNodeDisconnectFromMaster({})", node); notifyNodeDisconnectedEvent(node); } } } else { - logger.debug("Ignoring the Update for : {} NewControllers : {}", - node, newControllers.toString()); + logger.debug("Ignoring the Update for : {} NewControllers : {}", node, newControllers.toString()); } existingConnections.put(node, newControllers); } @@ -326,7 +322,7 @@ public class ConnectionManager implements IConnectionManager, public void entryDeleted(Node key, String cacheName, boolean originLocal) { if (originLocal) return; - logger.debug("Deleted : {} cache : {}", key, cacheName); + logger.debug("Deleted entry {} from cache : {}", key, cacheName); notifyNodeDisconnectedEvent(key); } diff --git a/opendaylight/connectionmanager/implementation/src/main/java/org/opendaylight/controller/connectionmanager/scheme/AbstractScheme.java b/opendaylight/connectionmanager/implementation/src/main/java/org/opendaylight/controller/connectionmanager/scheme/AbstractScheme.java index 1d0e86ecd1..68d1b233b2 100644 --- a/opendaylight/connectionmanager/implementation/src/main/java/org/opendaylight/controller/connectionmanager/scheme/AbstractScheme.java +++ b/opendaylight/connectionmanager/implementation/src/main/java/org/opendaylight/controller/connectionmanager/scheme/AbstractScheme.java @@ -54,7 +54,7 @@ public abstract class AbstractScheme { allocateCaches(); retrieveCaches(); } else { - log.error("Couldn't retrieve caches for scheme %s. Clustering service unavailable", name); + log.error("Couldn't retrieve caches for scheme {}. Clustering service unavailable", name); } } @@ -335,4 +335,56 @@ public abstract class AbstractScheme { log.error("An error occured",e); } } + + /* (non-Javadoc) + * @see java.lang.Object#hashCode() + */ + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((name == null) ? 0 : name.hashCode()); + result = prime * result + ((nodeConnections == null) ? 0 : nodeConnections.hashCode()); + result = prime * result + ((nodeConnectionsCacheName == null) ? 0 : nodeConnectionsCacheName.hashCode()); + return result; + } + + /* (non-Javadoc) + * @see java.lang.Object#equals(java.lang.Object) + */ + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (!(obj instanceof AbstractScheme)) { + return false; + } + AbstractScheme other = (AbstractScheme) obj; + if (name == null) { + if (other.name != null) { + return false; + } + } else if (!name.equals(other.name)) { + return false; + } + if (nodeConnections == null) { + if (other.nodeConnections != null) { + return false; + } + } else if (!nodeConnections.equals(other.nodeConnections)) { + return false; + } + if (nodeConnectionsCacheName == null) { + if (other.nodeConnectionsCacheName != null) { + return false; + } + } else if (!nodeConnectionsCacheName.equals(other.nodeConnectionsCacheName)) { + return false; + } + return true; + } } diff --git a/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/Devices.java b/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/Devices.java index a118ccfbba..e4bb790676 100644 --- a/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/Devices.java +++ b/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/Devices.java @@ -23,7 +23,6 @@ import java.util.concurrent.ConcurrentMap; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import com.fasterxml.jackson.databind.ObjectMapper; import org.opendaylight.controller.connectionmanager.IConnectionManager; import org.opendaylight.controller.forwarding.staticrouting.IForwardingStaticRouting; import org.opendaylight.controller.forwarding.staticrouting.StaticRouteConfig; @@ -60,6 +59,7 @@ import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.ResponseBody; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; @@ -145,21 +145,21 @@ public class Devices implements IDaylightWeb { for (NodeConnector nodeConnector : nodeConnectorSet) { String nodeConnectorNumberToStr = nodeConnector.getID().toString(); Name ncName = ((Name) switchManager.getNodeConnectorProp(nodeConnector, Name.NamePropName)); - Config portStatus = ((Config) switchManager.getNodeConnectorProp(nodeConnector, + Config portConfig = ((Config) switchManager.getNodeConnectorProp(nodeConnector, Config.ConfigPropName)); State portState = ((State) switchManager.getNodeConnectorProp(nodeConnector, State.StatePropName)); String nodeConnectorName = (ncName != null) ? ncName.getValue() : ""; nodeConnectorName += " (" + nodeConnector.getID() + ")"; - if (portStatus != null) { - if (portStatus.getValue() == Config.ADMIN_UP) { - if (portState.getValue() == State.EDGE_UP) { + if (portConfig != null) { + if (portConfig.getValue() == Config.ADMIN_UP) { + if (portState != null && portState.getValue() == State.EDGE_UP) { nodeConnectorName = "" + nodeConnectorName + ""; - } else if (portState.getValue() == State.EDGE_DOWN) { + } else if (portState == null || portState.getValue() == State.EDGE_DOWN) { nodeConnectorName = "" + nodeConnectorName + ""; } - } else if (portStatus.getValue() == Config.ADMIN_DOWN) { + } else if (portConfig.getValue() == Config.ADMIN_DOWN) { nodeConnectorName = "" + nodeConnectorName + ""; } } -- 2.36.6