From ee7508b4dc8bf3a69023ab35ea0fe1e44c74b724 Mon Sep 17 00:00:00 2001 From: Ryan Izard <rizard@g.clemson.edu> Date: Thu, 11 Sep 2014 16:27:14 -0400 Subject: [PATCH] Added option to remove debug counters from the hierarchy if it's been added; useful for keeping track of switch or device counters, which might not stick around forever. Fixed bug in CounterNode where non-null was returned regardless of whether a counter was already present or was a new counter. DebugCounterServiceImpl prints a debug message if non-null is returned, mistakenly indicating you're adding a counter that you already had registerd (generated TONS of annoying debug messages at startup). Next bug, I accidentally initialized counters twice in the OFSwitchManager; now only done once. --- .../core/internal/OFSwitchManager.java | 1 - .../debugcounter/CounterNode.java | 43 +++++++++++++++++-- .../debugcounter/DebugCounterServiceImpl.java | 23 ++++++++++ .../debugcounter/IDebugCounterService.java | 12 ++++++ .../debugcounter/MockDebugCounterService.java | 6 +++ 5 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/floodlightcontroller/core/internal/OFSwitchManager.java b/src/main/java/net/floodlightcontroller/core/internal/OFSwitchManager.java index b62c6241c..2fbc8ff04 100644 --- a/src/main/java/net/floodlightcontroller/core/internal/OFSwitchManager.java +++ b/src/main/java/net/floodlightcontroller/core/internal/OFSwitchManager.java @@ -640,7 +640,6 @@ public class OFSwitchManager implements IOFSwitchManager, INewOFConnectionListen this.switchListeners = new CopyOnWriteArraySet<IOFSwitchListener>(); - this.counters = new SwitchManagerCounters(debugCounterService); /* TODO @Ryan try { this.storeClient = this.syncService.getStoreClient( diff --git a/src/main/java/net/floodlightcontroller/debugcounter/CounterNode.java b/src/main/java/net/floodlightcontroller/debugcounter/CounterNode.java index f44ea7a17..751e2c793 100644 --- a/src/main/java/net/floodlightcontroller/debugcounter/CounterNode.java +++ b/src/main/java/net/floodlightcontroller/debugcounter/CounterNode.java @@ -148,6 +148,44 @@ class CounterNode implements Iterable<DebugCounterImpl> { } return cur; } + + /** + * Remove the CounterNode identified by the hieraryElements if it exists. + * Returns null if no such CounterNode is found. Must only be called on + * the root of the tree. + * @param hierarchyElements + * @return + */ + CounterNode remove(List<String> hierarchyElements) { + CounterNode cur = this; + /* The last element of the hierarchy is what we want to delete. + * remove(len - 1) will shorten the hierarchy list to the parent of the last + * descendant. This will be our stopping point. The parent of the last + * descendant will contain a TreeMap of all it's "children." The String + * key we removed can be used to remove the specific child from the parent's + * "children" TreeMap. + * + * We're directly reusing the shrunken hierarchyElements List and + * keyToRemove String, which IMHO is pretty cool how it works out. + */ + String keyToRemove = hierarchyElements.remove(hierarchyElements.size() - 1); + for (String element: hierarchyElements) { + cur = cur.children.get(element); + if (cur == null) { + break; + } + } + // At this point, if !null, cur will be the parent of the CounterNode we want to remove. + CounterNode removed = null; + if (cur != null) { + removed = cur.children.remove(keyToRemove); + } + //TODO @Ryan this will remove the CounterNode from IDebugCounterService, but if any + // other modules still have a refernence to the IDebugCounter within the CounterNode + // we just removed, they might mistakenly query the "dead" counter. + + return removed; + } /** * Add the given moduleName to the tree. Can only be called on the root. @@ -205,11 +243,10 @@ class CounterNode implements Iterable<DebugCounterImpl> { } else { CounterNode newNode = new CounterNode(path, counter); parent.children.put(newCounterName, newNode); - return counter; + return null; //TODO @Ryan this SHOULD technically return null. Hopefully this wont break anything.... } } - - + /** * Iterator over the counters in the counter hierarchy. * Iteration order is a pre-order tree walk. Children of a node are diff --git a/src/main/java/net/floodlightcontroller/debugcounter/DebugCounterServiceImpl.java b/src/main/java/net/floodlightcontroller/debugcounter/DebugCounterServiceImpl.java index ae245deb8..87f3b6590 100644 --- a/src/main/java/net/floodlightcontroller/debugcounter/DebugCounterServiceImpl.java +++ b/src/main/java/net/floodlightcontroller/debugcounter/DebugCounterServiceImpl.java @@ -117,6 +117,16 @@ public class DebugCounterServiceImpl implements IFloodlightModule, IDebugCounter node.resetHierarchy(); return true; } + + @GuardedBy("lock.readLock") + private boolean removeInternal(List<String> hierarchyElements) { + CounterNode node = root.lookup(hierarchyElements); // returns e.g. root/module-name/counter-node-to-remove + if (node == null) { + return false; + } + root.remove(hierarchyElements); + return true; + } @Override public boolean resetCounterHierarchy(String moduleName, @@ -152,6 +162,19 @@ public class DebugCounterServiceImpl implements IFloodlightModule, IDebugCounter lock.readLock().unlock(); } } + + @Override + public boolean removeCounterHierarchy(String moduleName, + String counterHierarchy) { + verifyModuleNameSanity(moduleName); + verifyStringSanity(counterHierarchy, "counterHierarchy"); + lock.readLock().lock(); + try { + return removeInternal(CounterNode.getHierarchyElements(moduleName, counterHierarchy)); + } finally { + lock.readLock().unlock(); + } + } @GuardedBy("lock.readLock") private List<DebugCounterResource> getCountersFromNode(CounterNode node) { diff --git a/src/main/java/net/floodlightcontroller/debugcounter/IDebugCounterService.java b/src/main/java/net/floodlightcontroller/debugcounter/IDebugCounterService.java index 524e7cea0..3c00fd8b0 100644 --- a/src/main/java/net/floodlightcontroller/debugcounter/IDebugCounterService.java +++ b/src/main/java/net/floodlightcontroller/debugcounter/IDebugCounterService.java @@ -79,6 +79,18 @@ public interface IDebugCounterService extends IFloodlightService { * @return false if the given module name does not exists */ public boolean resetAllModuleCounters(String moduleName); + + /** + * Removes/deletes the counter hierarchy AND ALL LEVELS BELOW it in the hierarchy. + * For example: If a hierarchy exists like "00:00:00:00:01:02:03:04/pktin/drops" + * specifying a remove hierarchy: "00:00:00:00:01:02:03:04" + * will remove all counters for the switch dpid specified; + * while specifying a remove hierarchy: ""00:00:00:00:01:02:03:04/pktin" + * will remove the pktin counter and all levels below it (like drops) + * for the switch dpid specified. + * @return false if the given moduleName, counterHierarchy does not exist + */ + public boolean removeCounterHierarchy(String moduleName, String counterHierarchy); /** diff --git a/src/main/java/net/floodlightcontroller/debugcounter/MockDebugCounterService.java b/src/main/java/net/floodlightcontroller/debugcounter/MockDebugCounterService.java index 36a19d548..429e75f2f 100644 --- a/src/main/java/net/floodlightcontroller/debugcounter/MockDebugCounterService.java +++ b/src/main/java/net/floodlightcontroller/debugcounter/MockDebugCounterService.java @@ -112,4 +112,10 @@ public class MockDebugCounterService implements IFloodlightModule, IDebugCounter } } + @Override + public boolean removeCounterHierarchy(String moduleName, + String counterHierarchy) { + return true; + } + } -- GitLab