Refactor the storage of the Topology Listeners: move the state
from the TopologyModule to the TopologyManager.
Also:
- Keep the last ADD MastershipEvent inside field
TopologyManager.lastAddMastershipEvents
- Minor editorial changes inside TopologyManager.java
No functional changes.
Change-Id: I99956fdbb1dcd847e3e6ed2ad27f68ff3464b491
diff --git a/src/main/java/net/onrc/onos/core/topology/TopologyManager.java b/src/main/java/net/onrc/onos/core/topology/TopologyManager.java
index 1e3dccd..1c0f531 100644
--- a/src/main/java/net/onrc/onos/core/topology/TopologyManager.java
+++ b/src/main/java/net/onrc/onos/core/topology/TopologyManager.java
@@ -70,9 +70,10 @@
private TopologyDatastore datastore;
private final TopologyImpl topology = new TopologyImpl();
private final IControllerRegistryService registryService;
- private CopyOnWriteArrayList<ITopologyListener> topologyListeners;
private Kryo kryo = KryoFactory.newKryoObject();
private TopologyEventPreprocessor eventPreprocessor;
+ private CopyOnWriteArrayList<ITopologyListener> topologyListeners =
+ new CopyOnWriteArrayList<>();
//
// Metrics
@@ -120,12 +121,13 @@
// driver module, since the invariant may be different on optical, etc.
//
// What happens on leadership change?
- // - Probably should: remove from discovered.. Maps, but not send DELETE events
- // XXX Switch/Port can be rediscovered by new leader, but Link, Host?
+ // - Probably should: remove from discovered.. Maps, but not send DELETE
+ // events
+ // XXX Switch/Port can be rediscovered by new leader, but Link, Host?
// - Current: There is no way to recognize leadership change?
// ZookeeperRegistry.requestControl(long, ControlChangeCallback)
- // is the only way to register listener, and it allows only 1 listener,
- // which is already used by Controller class.
+ // is the only way to register listener, and it allows only one
+ // listener, which is already used by Controller class.
//
// FIXME Replace with concurrent variant.
// #removeSwitchDiscoveryEvent(SwitchEvent) runs in different thread.
@@ -138,6 +140,13 @@
new HashMap<>();
//
+ // Local state for keeping the last ADD Mastership Event entries.
+ // TODO: In the future, we might have to keep this state somewhere else.
+ //
+ private Map<ByteBuffer, MastershipEvent> lastAddMastershipEvents =
+ new HashMap<>();
+
+ //
// Local state for keeping track of the application event notifications
//
// - Queue of events, which will be dispatched to local listeners
@@ -160,13 +169,10 @@
* Constructor.
*
* @param registryService the Registry Service to use.
- * @param topologyListeners the collection of topology listeners to use.
*/
- public TopologyManager(IControllerRegistryService registryService,
- CopyOnWriteArrayList<ITopologyListener> topologyListeners) {
+ public TopologyManager(IControllerRegistryService registryService) {
datastore = new TopologyDatastore();
this.registryService = registryService;
- this.topologyListeners = topologyListeners;
this.eventPreprocessor =
new TopologyEventPreprocessor(registryService);
}
@@ -207,8 +213,8 @@
for (TopologyEvent topologyEvent : allTopologyEvents) {
EventEntry<TopologyEvent> eventEntry =
- new EventEntry<TopologyEvent>(EventEntry.Type.ENTRY_ADD,
- topologyEvent);
+ new EventEntry<TopologyEvent>(EventEntry.Type.ENTRY_ADD,
+ topologyEvent);
events.add(eventEntry);
}
processEvents(events);
@@ -391,6 +397,25 @@
}
/**
+ * Registers a listener for topology events.
+ *
+ * @param listener the listener to register
+ */
+ void registerTopologyListener(ITopologyListener listener) {
+ topologyListeners.addIfAbsent(listener);
+ }
+
+ /**
+ * Deregisters a listener for topology events. The listener will no longer
+ * receive topology events after this call.
+ *
+ * @param listener the listener to deregister
+ */
+ void deregisterTopologyListener(ITopologyListener listener) {
+ topologyListeners.remove(listener);
+ }
+
+ /**
* Dispatch Topology Events to the listeners.
*/
private void dispatchTopologyEvents() {
@@ -796,7 +821,8 @@
new TopologyEvent(hostEvent,
registryService.getOnosInstanceId());
eventChannel.addEntry(topologyEvent.getID(), topologyEvent);
- log.debug("Put the host info into the cache of the topology. mac {}", hostEvent.getMac());
+ log.debug("Put the host info into the cache of the topology. mac {}",
+ hostEvent.getMac());
// Store the new Host Event in the local cache
// TODO: The implementation below is probably wrong
@@ -827,7 +853,8 @@
new TopologyEvent(hostEvent,
registryService.getOnosInstanceId());
eventChannel.removeEntry(topologyEvent.getID());
- log.debug("Remove the host info into the cache of the topology. mac {}", hostEvent.getMac());
+ log.debug("Remove the host info into the cache of the topology. mac {}",
+ hostEvent.getMac());
// Cleanup the Host Event from the local cache
// TODO: The implementation below is probably wrong
@@ -855,6 +882,8 @@
@GuardedBy("topology.writeLock")
private boolean addMastershipEvent(MastershipEvent mastershipEvent) {
log.debug("Added Mastership event {}", mastershipEvent);
+ lastAddMastershipEvents.put(mastershipEvent.getIDasByteBuffer(),
+ mastershipEvent);
apiAddedMastershipEvents.add(mastershipEvent);
return true;
}
@@ -867,6 +896,7 @@
@GuardedBy("topology.writeLock")
private void removeMastershipEvent(MastershipEvent mastershipEvent) {
log.debug("Removed Mastership event {}", mastershipEvent);
+ lastAddMastershipEvents.remove(mastershipEvent.getIDasByteBuffer());
apiRemovedMastershipEvents.add(mastershipEvent);
}
@@ -1078,7 +1108,8 @@
log.error("Host {} on Port {} should have been removed prior to adding Link {}",
host, port, linkEvent);
- HostEvent hostEvent = topology.getHostEvent(host.getMacAddress());
+ HostEvent hostEvent =
+ topology.getHostEvent(host.getMacAddress());
hostsToUpdate.add(hostEvent);
}
}
@@ -1149,7 +1180,8 @@
Link linkIn = dstPort.getIncomingLink(linkEvent.getType());
if (linkIn == null) {
- log.warn("Link {} already removed on destination Port", linkEvent);
+ log.warn("Link {} already removed on destination Port",
+ linkEvent);
}
Link linkOut = srcPort.getOutgoingLink(linkEvent.getType());
if (linkOut == null) {
@@ -1194,7 +1226,8 @@
// Attached Ports must exist
Port port = topology.getPort(swp.getDpid(), swp.getPortNumber());
if (port == null) {
- log.debug("{} reordered because port {} was not there", hostEvent, swp);
+ log.debug("{} reordered because port {} was not there",
+ hostEvent, swp);
// Reordered event
return false; // should not continue if re-applying later
}
@@ -1219,7 +1252,8 @@
if (attachmentFound) {
if (modifiedHostEvent.getAttachmentPoints().isEmpty()) {
log.warn("No valid attachment point left. Ignoring."
- + "original: {}, modified: {}", hostEvent, modifiedHostEvent);
+ + "original: {}, modified: {}",
+ hostEvent, modifiedHostEvent);
// TODO Should we call #removeHost to trigger remove event?
// only if this call is update.
return false;
diff --git a/src/main/java/net/onrc/onos/core/topology/TopologyModule.java b/src/main/java/net/onrc/onos/core/topology/TopologyModule.java
index 657682f..d24f4df 100644
--- a/src/main/java/net/onrc/onos/core/topology/TopologyModule.java
+++ b/src/main/java/net/onrc/onos/core/topology/TopologyModule.java
@@ -5,7 +5,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.concurrent.CopyOnWriteArrayList;
import net.floodlightcontroller.core.module.FloodlightModuleContext;
import net.floodlightcontroller.core.module.FloodlightModuleException;
@@ -23,9 +22,6 @@
private TopologyManager topologyManager;
private IDatagridService datagridService;
private IControllerRegistryService registryService;
-
- private CopyOnWriteArrayList<ITopologyListener> topologyListeners;
-
private IRestApiService restApi;
@Override
@@ -61,9 +57,7 @@
restApi = context.getServiceImpl(IRestApiService.class);
datagridService = context.getServiceImpl(IDatagridService.class);
registryService = context.getServiceImpl(IControllerRegistryService.class);
-
- topologyListeners = new CopyOnWriteArrayList<>();
- topologyManager = new TopologyManager(registryService, topologyListeners);
+ topologyManager = new TopologyManager(registryService);
}
@Override
@@ -84,12 +78,11 @@
@Override
public void registerTopologyListener(ITopologyListener listener) {
- topologyListeners.addIfAbsent(listener);
+ topologyManager.registerTopologyListener(listener);
}
@Override
public void deregisterTopologyListener(ITopologyListener listener) {
- topologyListeners.remove(listener);
+ topologyManager.deregisterTopologyListener(listener);
}
-
}
diff --git a/src/test/java/net/onrc/onos/core/topology/TopologyManagerTest.java b/src/test/java/net/onrc/onos/core/topology/TopologyManagerTest.java
index 9856366..231ba6f 100644
--- a/src/test/java/net/onrc/onos/core/topology/TopologyManagerTest.java
+++ b/src/test/java/net/onrc/onos/core/topology/TopologyManagerTest.java
@@ -60,7 +60,6 @@
private IDatagridService datagridService;
private TopologyDatastore dataStoreService;
private IControllerRegistryService registryService;
- private CopyOnWriteArrayList<ITopologyListener> topologyListeners;
private Collection<TopologyEvent> allTopologyEvents;
private static final OnosInstanceId ONOS_INSTANCE_ID_1 =
new OnosInstanceId("ONOS-Instance-ID-1");
@@ -170,9 +169,7 @@
*/
private void setupTopologyManager() {
// Create a TopologyManager object for testing
- topologyListeners = new CopyOnWriteArrayList<>();
- theTopologyManager = new TopologyManager(registryService,
- topologyListeners);
+ theTopologyManager = new TopologyManager(registryService);
// Replace the eventHandler to prevent the thread from starting
TestUtils.setField(theTopologyManager, "eventHandler",
@@ -188,10 +185,9 @@
*/
private void setupTopologyManagerWithEventHandler() {
// Create a TopologyManager object for testing
- topologyListeners = new CopyOnWriteArrayList<>();
- topologyListeners.add(theTopologyListener);
- theTopologyManager = new TopologyManager(registryService,
- topologyListeners);
+ theTopologyManager = new TopologyManager(registryService);
+ theTopologyManager.registerTopologyListener(theTopologyListener);
+
// Allocate the Event Handler, so we can have direct access to it
theEventHandler = theTopologyManager.new EventHandler();
TestUtils.setField(theTopologyManager, "eventHandler",