Refactored NetworkGraphDatastore to use ForceCreate/ForceDelete
Change-Id: I0f9f5a083ea7cc7b7a507a9fa9e9533c59742ae4
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphDatastore.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphDatastore.java
index be0c3c4..27454a8 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphDatastore.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphDatastore.java
@@ -7,15 +7,12 @@
import net.onrc.onos.datastore.RCObject.WriteOp;
import net.onrc.onos.datastore.topology.RCLink;
import net.onrc.onos.datastore.topology.RCPort;
+import net.onrc.onos.datastore.topology.RCPort.STATUS;
import net.onrc.onos.datastore.topology.RCSwitch;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import edu.stanford.ramcloud.JRamCloud.ObjectDoesntExistException;
-import edu.stanford.ramcloud.JRamCloud.ObjectExistsException;
-import edu.stanford.ramcloud.JRamCloud.WrongVersionException;
-
/**
* The southbound interface to the network graph which allows clients to
* mutate the graph. This class will maintain the invariants of the network
@@ -35,14 +32,6 @@
public class NetworkGraphDatastore {
private static final Logger log = LoggerFactory.getLogger(NetworkGraphDatastore.class);
- private static final int NUM_RETRIES = 10;
-
- private final TopologyManager graph;
-
- public NetworkGraphDatastore(TopologyManager graph) {
- this.graph = graph;
- }
-
/**
* Add a switch to the database.
*
@@ -61,11 +50,9 @@
// to assure that DPID is unique cluster-wide, etc.
groupOp.add(WriteOp.ForceCreate(rcSwitch));
- //for (Port port : sw.getPorts()) {
for (PortEvent portEvent : sw.getPorts()) {
RCPort rcPort = new RCPort(sw.getDpid(), portEvent.getNumber());
rcPort.setStatus(RCPort.STATUS.ACTIVE);
- //rcSwitch.addPortId(rcPort.getId());
groupOp.add(WriteOp.ForceCreate(rcPort));
}
@@ -81,7 +68,7 @@
// Conditional operation (Create/Update) then we should retry here.
}
}
- return (! failed);
+ return !failed;
}
/**
@@ -94,43 +81,21 @@
log.debug("Deactivating switch {}", sw);
RCSwitch rcSwitch = new RCSwitch(sw.getDpid());
- List<RCObject> objectsToDeactive = new ArrayList<RCObject>();
+ List<WriteOp> groupOp = new ArrayList<>();
+ rcSwitch.setStatus(RCSwitch.STATUS.INACTIVE);
- for (int i = 0; i < NUM_RETRIES; i++) {
- try {
- rcSwitch.read();
- rcSwitch.setStatus(RCSwitch.STATUS.INACTIVE);
- objectsToDeactive.add(rcSwitch);
+ groupOp.add(WriteOp.ForceCreate(rcSwitch));
-// for (Port p : sw.getPorts()) {
-// RCPort rcPort = new RCPort(sw.getDpid(), (long)p.getNumber());
-// rcPort.read();
-// rcPort.setStatus(RCPort.STATUS.INACTIVE);
-// objectsToDeactive.add(rcPort);
-// }
- } catch (ObjectDoesntExistException e) {
- log.warn("Trying to deactivate an object that doesn't exist", e);
- // We don't care to much if the object wasn't there, it's
- // being deactivated anyway
- return true; // Success: no object in the DB
- }
+ for (PortEvent portEvent : sw.getPorts()) {
+ RCPort rcPort = new RCPort(sw.getDpid(), (long)portEvent.getNumber());
+ rcPort.setStatus(RCPort.STATUS.INACTIVE);
- try {
- for (RCObject rcObject : objectsToDeactive) {
- rcObject.update();
- }
- return true; // Success
- } catch (ObjectDoesntExistException e) {
- // Unlikely, and we don't care anyway.
- // TODO But, this will cause everything else to fail
- log.warn("Trying to deactivate object that doesn't exist", e);
- return true; // Success: no object in the DB
- } catch (WrongVersionException e) {
- // Need to re-read and retry
- }
+ groupOp.add(WriteOp.ForceCreate(rcPort));
}
- return false; // Failure
+ boolean failed = RCObject.multiWrite(groupOp);
+
+ return !failed;
}
/**
@@ -141,24 +106,14 @@
*/
public boolean addPort(PortEvent port) {
log.debug("Adding port {}", port);
- //RCSwitch rcSwitch = new RCSwitch(sw.getDpid());
-
- //try {
- //rcSwitch.read();
- //} catch (ObjectDoesntExistException e) {
- //log.warn("Add port failed because switch {} doesn't exist", sw.getDpid(), e);
- //return false;
- //}
RCPort rcPort = new RCPort(port.getDpid(), port.getNumber());
rcPort.setStatus(RCPort.STATUS.ACTIVE);
+ rcPort.forceCreate();
// TODO add description into RCPort
//rcPort.setDescription(port.getDescription());
- //rcSwitch.addPortId(rcPort.getId());
- boolean success = writeObject(rcPort);
- // success &= writeObject(rcSwitch);
- return success;
+ return true;
}
/**
@@ -169,28 +124,13 @@
*/
public boolean deactivatePort(PortEvent port) {
log.debug("Deactivating port {}", port);
+
RCPort rcPort = new RCPort(port.getDpid(), port.getNumber());
+ rcPort.setStatus(STATUS.INACTIVE);
+
+ rcPort.forceCreate();
- for (int i = 0; i < NUM_RETRIES; i++) {
- try {
- rcPort.read();
- } catch (ObjectDoesntExistException e) {
- // oh well, we were deactivating anyway
- log.warn("Trying to deactivate a port that doesn't exist: {}", port);
- return true; // Success: no object in the DB
- }
-
- rcPort.setStatus(RCPort.STATUS.INACTIVE);
-
- try {
- rcPort.update();
- return true; // Success
- } catch (ObjectDoesntExistException | WrongVersionException e) {
- // retry
- }
- }
-
- return false; // Failure
+ return true;
}
/**
@@ -218,50 +158,14 @@
return true; // Success
}
- /**
- * Remove a link from the database.
- *
- * @param link the link to remove.
- * @return true on success, otherwise false.
- */
- public boolean removeLink(LinkEvent link) {
- log.debug("Removing link {}", link);
- RCLink rcLink = new RCLink(link.getSrc().getDpid(),
- link.getSrc().getNumber(),
- link.getDst().getDpid(),
- link.getDst().getNumber());
+ public boolean removeLink(LinkEvent linkEvent) {
+ log.debug("Removing link {}", linkEvent);
+
+ RCLink rcLink = new RCLink(linkEvent.getSrc().getDpid(), linkEvent.getSrc().getNumber(),
+ linkEvent.getDst().getDpid(), linkEvent.getDst().getNumber());
+ rcLink.forceDelete();
- //RCPort rcSrcPort = new RCPort(link.getSourceSwitchDpid(), (long)link.getSourcePortNumber());
- //RCPort rcDstPort = new RCPort(link.getDestinationSwitchDpid(), (long)link.getDestinationPortNumber());
-
- for (int i = 0; i < NUM_RETRIES; i++) {
- try {
- //rcSrcPort.read();
- //rcDstPort.read();
- rcLink.read();
- } catch (ObjectDoesntExistException e) {
- // XXX Note: This error might be harmless, if triggered by out-dated remove Link event
- log.error("Remove link failed {}", link, e);
- return true; // Success: no object in the DB
- }
-
- //rcSrcPort.removeLinkId(rcLink.getId());
- //rcDstPort.removeLinkId(rcLink.getId());
-
- try {
- //rcSrcPort.update();
- //rcDstPort.update();
- rcLink.delete();
- return true; // Success
- } catch (ObjectDoesntExistException e) {
- log.error("Remove link failed {}", link, e);
- return true; // Success: no object in the DB
- } catch (WrongVersionException e) {
- // retry
- }
- }
-
- return false; // Failure
+ return true;
}
/**
@@ -285,37 +189,4 @@
// TODO implement
return false; // Failure: not implemented yet
}
-
- /**
- * Write an object to the database.
- *
- * @param object the object to write.
- * @return true on success, otherwise false.
- */
- private boolean writeObject(RCObject object) {
- for (int i = 0; i < NUM_RETRIES; i++) {
- try {
- object.create();
- } catch (ObjectExistsException e) {
- try {
- object.read();
- } catch (ObjectDoesntExistException e1) {
- // TODO Auto-generated catch block
- log.error(" ", e);
- return false; // Failure
- }
- }
-
- try {
- // TODO check API for writing without caring what's there
- object.update();
- return true; // Success
- } catch (ObjectDoesntExistException | WrongVersionException e) {
- log.debug(" ", e);
- // re-read and retry
- }
- }
-
- return false; // Failure
- }
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/TopologyManager.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/TopologyManager.java
index 0118294..43f287c 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/TopologyManager.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/TopologyManager.java
@@ -63,7 +63,7 @@
*/
public TopologyManager(IControllerRegistryService registryService,
CopyOnWriteArrayList<INetworkGraphListener> networkGraphListeners) {
- datastore = new NetworkGraphDatastore(this);
+ datastore = new NetworkGraphDatastore();
this.registryService = registryService;
this.networkGraphListeners = networkGraphListeners;
}
@@ -318,6 +318,8 @@
if (prepareForRemoveLinkEvent(linkEvent)) {
if (dstCheckBeforeDBmodify) {
// write to DB only if it is owner of the dst dpid
+ // XXX this will cause link remove events to be dropped
+ // if the dst switch just disconnected
if (registryService.hasControl(linkEvent.getDst().dpid)) {
datastore.removeLink(linkEvent);
}