Fixing port stats rate calculation.

Change-Id: Ic4c803f58a53c293ae05bc0c207d7e23546f7158

Fixing port stats rate calculation.
Change-Id: Ic4c803f58a53c293ae05bc0c207d7e23546f7158
Thomas Vachuska
Commit 204cb6cab4caed52ffcd057a0cc25bf3c5411c62 204cb6ca 1 parent 6f90c59b
Showing 3 changed files with 61 additions and 82 deletions
core/api/src/main/java/org/onosproject/net/statistic/DefaultLoad.java
incubator/net/src/main/java/org/onosproject/incubator/net/impl/PortStatisticsManager.java
web/gui/src/main/java/org/onosproject/ui/impl/TopologyViewMessageHandlerBase.java
--- a/core/api/src/main/java/org/onosproject/net/statistic/DefaultLoad.java
View file @204cb6c
+++ b/core/api/src/main/java/org/onosproject/net/statistic/DefaultLoad.java
View file @204cb6c
@@ -28,12 +28,12 @@ public class DefaultLoad implements Load {
     private final long current;
     private final long previous;
     private final long time;
-    private final int interval;
+    private final long interval;
     /**
      * Indicates the flow statistics poll interval in seconds.
      */
-    private static int pollInterval = 10;
+    private static long pollInterval = 10;
     /**
      * Creates an invalid load.
@@ -63,7 +63,7 @@ public class DefaultLoad implements Load {
      * @param previous the previous value
      * @param interval poll interval for this load
      */
-    public DefaultLoad(long current, long previous, int interval) {
+    public DefaultLoad(long current, long previous, long interval) {
         checkArgument(interval > 0, "Interval must be greater than 0");
         this.current = current;
         this.previous = previous;
@@ -78,7 +78,7 @@ public class DefaultLoad implements Load {
      *
      * @param newPollInterval poll interval duration in seconds
      */
-    public static void setPollInterval(int newPollInterval) {
+    public static void setPollInterval(long newPollInterval) {
         pollInterval = newPollInterval;
     }
--- a/incubator/net/src/main/java/org/onosproject/incubator/net/impl/PortStatisticsManager.java
View file @204cb6c
+++ b/incubator/net/src/main/java/org/onosproject/incubator/net/impl/PortStatisticsManager.java
View file @204cb6c
@@ -49,8 +49,8 @@ public class PortStatisticsManager implements PortStatisticsService {
     private final Logger log = getLogger(getClass());
-    private static final int SECOND = 1_000; // milliseconds
+    private static final long POLL_FREQUENCY = 10_000; // milliseconds
-    private static final long STALE_LIMIT = 15_000; // milliseconds
+    private static final long STALE_LIMIT = (long) (1.5 * POLL_FREQUENCY);
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected DeviceService deviceService;
@@ -77,11 +77,15 @@ public class PortStatisticsManager implements PortStatisticsService {
         DataPoint c = current.get(connectPoint);
         DataPoint p = previous.get(connectPoint);
         long now = System.currentTimeMillis();
-        if (c != null && p != null && (now - c.time < STALE_LIMIT) &&
+
-                (c.time > p.time + SECOND) &&
+        if (c != null && p != null && (now - c.time < STALE_LIMIT)) {
-                (c.stats.bytesSent() - p.stats.bytesSent() >= 0)) {
+            if (c.stats.durationSec() > p.stats.durationSec() &&
-            return new DefaultLoad(c.stats.bytesSent(), p.stats.bytesSent(),
+                    c.stats.bytesSent() >= p.stats.bytesSent() &&
-                                   (int) (c.time - p.time) / SECOND);
+                    c.stats.durationSec() >= POLL_FREQUENCY / 1_000) {
+                return new DefaultLoad(c.stats.bytesSent(), p.stats.bytesSent(),
+                                       c.stats.durationSec() - p.stats.durationSec());
+            }
+            return new DefaultLoad(c.stats.bytesSent(), 0, c.stats.durationSec());
         }
         return null;
     }
@@ -114,15 +118,15 @@ public class PortStatisticsManager implements PortStatisticsService {
     // Updates the port stats for the specified port
     private void updatePortData(DeviceId deviceId, PortStatistics stats) {
         ConnectPoint cp = new ConnectPoint(deviceId, portNumber(stats.port()));
+        DataPoint c = current.get(cp);
+
+        // Create a new data point and make it the current one
+        current.put(cp, new DataPoint(stats));
         // If we have a current data point, demote it to previous
-        DataPoint c = current.get(cp);
         if (c != null) {
             previous.put(cp, c);
         }
-
-        // Create a new data point and make it the current one
-        current.put(cp, new DataPoint(stats));
     }
     // Cleans all port loads for the specified device
--- a/web/gui/src/main/java/org/onosproject/ui/impl/TopologyViewMessageHandlerBase.java
View file @204cb6c
+++ b/web/gui/src/main/java/org/onosproject/ui/impl/TopologyViewMessageHandlerBase.java
View file @204cb6c
@@ -16,7 +16,6 @@
 package org.onosproject.ui.impl;
 import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.common.collect.ImmutableList;
@@ -90,6 +89,7 @@ import static com.google.common.base.Strings.isNullOrEmpty;
 import static org.onosproject.cluster.ClusterEvent.Type.INSTANCE_ADDED;
 import static org.onosproject.cluster.ClusterEvent.Type.INSTANCE_REMOVED;
 import static org.onosproject.cluster.ControllerNode.State.ACTIVE;
+import static org.onosproject.net.DefaultEdgeLink.createEdgeLink;
 import static org.onosproject.net.DeviceId.deviceId;
 import static org.onosproject.net.HostId.hostId;
 import static org.onosproject.net.LinkKey.linkKey;
@@ -101,7 +101,8 @@ import static org.onosproject.net.host.HostEvent.Type.HOST_ADDED;
 import static org.onosproject.net.host.HostEvent.Type.HOST_REMOVED;
 import static org.onosproject.net.link.LinkEvent.Type.LINK_ADDED;
 import static org.onosproject.net.link.LinkEvent.Type.LINK_REMOVED;
-import static org.onosproject.ui.impl.TopologyViewMessageHandlerBase.StatsType.*;
+import static org.onosproject.ui.impl.TopologyViewMessageHandlerBase.StatsType.FLOW;
+import static org.onosproject.ui.impl.TopologyViewMessageHandlerBase.StatsType.PORT;
 /**
  * Facility for creating messages bound for the topology viewer.
@@ -126,7 +127,7 @@ public abstract class TopologyViewMessageHandlerBase extends UiMessageHandler {
     private static final String KB_UNIT = "KB";
     private static final String B_UNIT = "B";
-    private static final long BPS_THRESHOLD = 1024;
+    private static final double BPS_THRESHOLD = 4 * KB;
     protected ServiceDirectory directory;
     protected ClusterService clusterService;
@@ -567,37 +568,49 @@ public abstract class TopologyViewMessageHandlerBase extends UiMessageHandler {
         pathNodeT.set("labels", labelsT);
         paths.add(pathNodeT);
-        for (BiLink link : consolidateLinks(linkService.getLinks())) {
+        Map<LinkKey, BiLink> biLinks = consolidateLinks(linkService.getLinks());
+        addEdgeLinks(biLinks);
+        for (BiLink link : biLinks.values()) {
             boolean bi = link.two != null;
-            if (isInfrastructureEgress(link.one) ||
+            if (type == FLOW) {
-                    (bi && isInfrastructureEgress(link.two))) {
+                link.addLoad(getLinkLoad(link.one));
-                if (type == FLOW) {
+                link.addLoad(bi ? getLinkLoad(link.two) : null);
-                    link.addLoad(flowStatsService.load(link.one));
+            } else if (type == PORT) {
-                    link.addLoad(bi ? flowStatsService.load(link.two) : null);
+                link.addLoad(portStatsService.load(link.one.src()), BPS_THRESHOLD);
-                } else if (type == PORT) {
+                link.addLoad(portStatsService.load(link.one.dst()), BPS_THRESHOLD);
-                    link.addLoad(portStatsService.load(link.one.src()), BPS_THRESHOLD);
+            }
-                    link.addLoad(bi ? portStatsService.load(link.two.src()) : null, BPS_THRESHOLD);
+            if (link.hasTraffic) {
-                }
+                linksNodeT.add(compactLinkString(link.one));
-                if (link.hasTraffic) {
+                labelsT.add(type == PORT ?
-                    linksNodeT.add(compactLinkString(link.one));
+                                    formatBytes(link.rate) + "ps" :
-                    labelsT.add(type == PORT ?
+                                    formatBytes(link.bytes));
-                                        formatBytes(link.rate) + "ps" :
+            } else {
-                                        formatBytes(link.bytes));
+                linksNodeN.add(compactLinkString(link.one));
-                } else {
+                labelsN.add("");
-                    linksNodeN.add(compactLinkString(link.one));
-                    labelsN.add("");
-                }
             }
         }
         return JsonUtils.envelope("showTraffic", 0, payload);
     }
-    private Collection<BiLink> consolidateLinks(Iterable<Link> links) {
+    private Load getLinkLoad(Link link) {
+        if (link.src().elementId() instanceof DeviceId) {
+            return flowStatsService.load(link);
+        }
+        return null;
+    }
+
+    private void addEdgeLinks(Map<LinkKey, BiLink> biLinks) {
+        hostService.getHosts().forEach(host -> {
+            addLink(biLinks, createEdgeLink(host.location(), false));
+        });
+    }
+
+    private Map<LinkKey, BiLink> consolidateLinks(Iterable<Link> links) {
         Map<LinkKey, BiLink> biLinks = new HashMap<>();
         for (Link link : links) {
             addLink(biLinks, link);
         }
-        return biLinks.values();
+        return biLinks;
     }
     // Produces JSON message to trigger flow overview visualization
@@ -680,7 +693,7 @@ public abstract class TopologyViewMessageHandlerBase extends UiMessageHandler {
                                           ((LinkCollectionIntent) installable).links());
                         } else if (installable instanceof OpticalPathIntent) {
                             classifyLinks(type, biLinks, trafficClass.showTraffic,
-                                    ((OpticalPathIntent) installable).path().links());
+                                          ((OpticalPathIntent) installable).path().links());
                         }
                     }
                 }
@@ -708,12 +721,10 @@ public abstract class TopologyViewMessageHandlerBase extends UiMessageHandler {
         if (links != null) {
             for (Link link : links) {
                 BiLink biLink = addLink(biLinks, link);
-                if (isInfrastructureEgress(link)) {
+                if (showTraffic) {
-                    if (showTraffic) {
+                    biLink.addLoad(flowStatsService.load(link));
-                        biLink.addLoad(flowStatsService.load(link));
-                    }
-                    biLink.addClass(type);
                 }
+                biLink.addClass(type);
             }
         }
     }
@@ -731,37 +742,6 @@ public abstract class TopologyViewMessageHandlerBase extends UiMessageHandler {
         return biLink;
     }
-
-    // Adds the link segments (path or tree) associated with the specified
-    // connectivity intent
-    protected void addPathTraffic(ArrayNode paths, String type, String trafficType,
-                                  Iterable<Link> links) {
-        ObjectNode pathNode = objectNode();
-        ArrayNode linksNode = arrayNode();
-
-        if (links != null) {
-            ArrayNode labels = arrayNode();
-            boolean hasTraffic = false;
-            for (Link link : links) {
-                if (isInfrastructureEgress(link)) {
-                    linksNode.add(compactLinkString(link));
-                    Load load = flowStatsService.load(link);
-                    String label = "";
-                    if (load.rate() > 0) {
-                        hasTraffic = true;
-                        label = formatBytes(load.latest());
-                    }
-                    labels.add(label);
-                }
-            }
-            pathNode.put("class", hasTraffic ? type + " " + trafficType : type);
-            pathNode.put("traffic", hasTraffic);
-            pathNode.set("links", linksNode);
-            pathNode.set("labels", labels);
-            paths.add(pathNode);
-        }
-    }
-
     // Poor-mans formatting to get the labels with byte counts looking nice.
     private String formatBytes(long bytes) {
         // TODO: multiply everything by 8 to compute bits/second
@@ -790,10 +770,6 @@ public abstract class TopologyViewMessageHandlerBase extends UiMessageHandler {
         return format.format(number);
     }
-    private boolean isInfrastructureEgress(Link link) {
-        return link.src().elementId() instanceof DeviceId;
-    }
-
     // Produces compact string representation of a link.
     private static String compactLinkString(Link link) {
         return String.format(COMPACT, link.src().elementId(), link.src().port(),
@@ -802,7 +778,6 @@ public abstract class TopologyViewMessageHandlerBase extends UiMessageHandler {
     // Produces JSON property details.
     private ObjectNode json(String id, String type, Prop... props) {
-        ObjectMapper mapper = new ObjectMapper();
         ObjectNode result = objectNode()
                 .put("id", id).put("type", type);
         ObjectNode pnode = objectNode();
@@ -848,7 +823,7 @@ public abstract class TopologyViewMessageHandlerBase extends UiMessageHandler {
             addLoad(load, 0);
         }
-        void addLoad(Load load, long threshold) {
+        void addLoad(Load load, double threshold) {
             if (load != null) {
                 this.hasTraffic = hasTraffic || load.rate() > threshold;
                 this.bytes += load.latest();