Improved thread-safety of the PathChildrenCaches used to supply registry information to REST
diff --git a/src/main/java/net/floodlightcontroller/onoslistener/OnosPublisher.java b/src/main/java/net/floodlightcontroller/onoslistener/OnosPublisher.java
index 36cde95..964eff1 100644
--- a/src/main/java/net/floodlightcontroller/onoslistener/OnosPublisher.java
+++ b/src/main/java/net/floodlightcontroller/onoslistener/OnosPublisher.java
@@ -102,7 +102,7 @@
// TODO Auto-generated catch block
e.printStackTrace();
} catch (RegistryException e) {
- // TODO Auto-generated catch block
+ log.debug("Caught RegistryException trying to requestControl in cleanup thread");
e.printStackTrace();
}
}
diff --git a/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java b/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java
index b666db7..acb6e52 100644
--- a/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java
+++ b/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java
@@ -96,7 +96,8 @@
HexString.toLong(dpid), latch.hasLeadership());
}
else {
- log.debug("Latch for {} has changed", dpid);
+ log.debug("Latch for {} has changed: old latch {} - new latch {}",
+ new Object[]{dpid, latch, swData.getLatch()});
}
}
}
@@ -123,21 +124,28 @@
case CHILD_ADDED:
case CHILD_UPDATED:
//Check we have a PathChildrenCache for this child, add one if not
- if (switchPathCaches.get(strSwitch) == null){
- PathChildrenCache pc = new PathChildrenCache(client,
- event.getData().getPath(), true);
- pc.start(StartMode.NORMAL);
- switchPathCaches.put(strSwitch, pc);
+ synchronized (switchPathCaches){
+ if (switchPathCaches.get(strSwitch) == null){
+ PathChildrenCache pc = new PathChildrenCache(client,
+ event.getData().getPath(), true);
+ pc.start(StartMode.NORMAL);
+ switchPathCaches.put(strSwitch, pc);
+ }
}
break;
case CHILD_REMOVED:
//Remove our PathChildrenCache for this child
- PathChildrenCache pc = switchPathCaches.remove(strSwitch);
- pc.close();
+ PathChildrenCache pc = null;
+ synchronized(switchPathCaches){
+ pc = switchPathCaches.remove(strSwitch);
+ }
+ if (pc != null){
+ pc.close();
+ }
break;
default:
- //All other events are connection status events. We need to do anything
- //as the path cache handles these on its own.
+ //All other events are connection status events. We don't need to
+ //do anything as the path cache handles these on its own.
break;
}
@@ -161,6 +169,8 @@
throw new RegistryException("Already contesting control for " + dpidStr);
}
+ log.debug("No latch for switch {}", dpidStr);
+
LeaderLatch latch = new LeaderLatch(client, latchPath, controllerId);
latch.addListener(new SwitchLeaderListener(dpidStr, latch));
@@ -203,9 +213,13 @@
log.debug("Trying to release control of a switch we are not contesting");
return;
}
+
+ log.debug("swData for {} was not null: {}", dpidStr, swData);
LeaderLatch latch = swData.getLatch();
+ latch.removeAllListeners();
+
try {
latch.close();
} catch (IOException e) {
@@ -421,7 +435,8 @@
restApi = context.getServiceImpl(IRestApiService.class);
switches = new ConcurrentHashMap<String, SwitchLeadershipData>();
- switchPathCaches = new HashMap<String, PathChildrenCache>();
+ //switchPathCaches = new HashMap<String, PathChildrenCache>();
+ switchPathCaches = new ConcurrentHashMap<String, PathChildrenCache>();
RetryPolicy retryPolicy = new ExponentialBackoffRetry(1000, 3);
client = CuratorFrameworkFactory.newClient(this.connectionString,