- In SwitchHandler.java, Transmit Thread waits if the priority queue is empty. 90/590/2
authorJason Ye <yisye@cisco.com>
Wed, 10 Jul 2013 22:55:00 +0000 (15:55 -0700)
committerGerrit Code Review <gerrit@opendaylight.org>
Thu, 11 Jul 2013 18:05:09 +0000 (18:05 +0000)
- Change DiscoveryService timer from 1 sec to 2 sec per tick leading to less CPU utilization. Added enum class for some time intervals.
- MacAddress property was defined as a byte array. XML can not serialize byte array properly. Change it to String.
- SwitchManager osgi cmd "pns" now displays sorted list of nodes.

Signed-off-by: Jason Ye <yisye@cisco.com>
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/DiscoveryService.java
opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/core/MacAddress.java
opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerImpl.java

index 81729c3..3fa2513 100644 (file)
@@ -68,29 +68,29 @@ public class SwitchHandler implements ISwitch {
             .getLogger(SwitchHandler.class);
     private static final int SWITCH_LIVENESS_TIMER = 5000;
     private static final int switchLivenessTimeout = getSwitchLivenessTimeout();
-    private int MESSAGE_RESPONSE_TIMER = 2000;
+    private final int MESSAGE_RESPONSE_TIMER = 2000;
 
-    private String instanceName;
-    private ISwitch thisISwitch;
-    private IController core;
+    private final String instanceName;
+    private final ISwitch thisISwitch;
+    private final IController core;
     private Long sid;
     private Integer buffers;
     private Integer capabilities;
     private Byte tables;
     private Integer actions;
     private Selector selector;
-    private SocketChannel socket;
-    private BasicFactory factory;
-    private AtomicInteger xid;
+    private final SocketChannel socket;
+    private final BasicFactory factory;
+    private final AtomicInteger xid;
     private SwitchState state;
     private Timer periodicTimer;
-    private Map<Short, OFPhysicalPort> physicalPorts;
-    private Map<Short, Integer> portBandwidth;
-    private Date connectedDate;
+    private final Map<Short, OFPhysicalPort> physicalPorts;
+    private final Map<Short, Integer> portBandwidth;
+    private final Date connectedDate;
     private Long lastMsgReceivedTimeStamp;
     private Boolean probeSent;
-    private ExecutorService executor;
-    private ConcurrentHashMap<Integer, Callable<Object>> messageWaitingDone;
+    private final ExecutorService executor;
+    private final ConcurrentHashMap<Integer, Callable<Object>> messageWaitingDone;
     private boolean running;
     private IMessageReadWrite msgReadWriteService;
     private Thread switchHandlerThread;
@@ -767,23 +767,19 @@ public class SwitchHandler implements ISwitch {
      * messaging service to transmit it over the socket channel
      */
     class PriorityMessageTransmit implements Runnable {
+        @Override
         public void run() {
             running = true;
             while (running) {
                 try {
-                    while (!transmitQ.isEmpty()) {
-                        PriorityMessage pmsg = transmitQ.poll();
-                        msgReadWriteService.asyncSend(pmsg.msg);
-                        logger.trace("Message sent: {}", pmsg);
-                        /*
-                         * If syncReply is set to true, wait for the response
-                         * back.
-                         */
-                        if (pmsg.syncReply) {
-                            syncMessageInternal(pmsg.msg, pmsg.msg.getXid(), false);
-                        }
+                    PriorityMessage pmsg = transmitQ.take();
+                    msgReadWriteService.asyncSend(pmsg.msg);
+                    /*
+                     * If syncReply is set to true, wait for the response back.
+                     */
+                    if (pmsg.syncReply) {
+                        syncMessageInternal(pmsg.msg, pmsg.msg.getXid(), false);
                     }
-                    Thread.sleep(10);
                 } catch (InterruptedException ie) {
                     reportError(new InterruptedException(
                             "PriorityMessageTransmit thread interrupted"));
@@ -801,6 +797,7 @@ public class SwitchHandler implements ISwitch {
     private void startTransmitThread() {
         this.transmitQ = new PriorityBlockingQueue<PriorityMessage>(11,
                 new Comparator<PriorityMessage>() {
+                    @Override
                     public int compare(PriorityMessage p1, PriorityMessage p2) {
                         if (p2.priority != p1.priority) {
                             return p2.priority - p1.priority;
@@ -930,7 +927,7 @@ public class SwitchHandler implements ISwitch {
                 // the result if OFError already
                 if (logger.isDebugEnabled()) {
                   logger.debug("Send {} failed --> {}", msg.getType(),
-                               ((OFError) result));
+                               (result));
                 }
             }
             return result;
index 527382f..3f3c8bd 100644 (file)
@@ -88,19 +88,19 @@ public class DiscoveryService implements IInventoryShimExternalListener, IDataPa
 
     private Timer discoveryTimer;
     private DiscoveryTimerTask discoveryTimerTask;
-    private long discoveryTimerTick = 1L * 1000; // per tick in msec
+    private final static long discoveryTimerTick = 2L * 1000; // per tick in msec
     private int discoveryTimerTickCount = 0; // main tick counter
     // Max # of ports handled in one batch
     private int discoveryBatchMaxPorts = 500;
     // Periodically restart batching process
     private int discoveryBatchRestartTicks = getDiscoveryInterval();
-    private int discoveryBatchPausePeriod = 5; // pause for few secs
+    private int discoveryBatchPausePeriod = 5;
     // Pause after this point
     private int discoveryBatchPauseTicks = discoveryBatchRestartTicks - discoveryBatchPausePeriod;
     // Number of retries after initial timeout
     private int discoveryRetry = getDiscoveryRetry();
-    private int discoveryTimeoutTicks = getDiscoveryTimeout(); // timeout in sec
-    private int discoveryAgeoutTicks = 120; // age out 2 min
+    private int discoveryTimeoutTicks = getDiscoveryTimeout();
+    private int discoveryAgeoutTicks = getDiscoveryAgeout();
     // multiple of discoveryBatchRestartTicks
     private int discoveryConsistencyCheckMultiple = 2;
     // CC tick counter
@@ -161,6 +161,37 @@ public class DiscoveryService implements IInventoryShimExternalListener, IDataPa
         }
     }
 
+    public enum DiscoveryPeriod {
+        INTERVAL(300),
+        TIMEOUT (60),
+        AGEOUT  (120);
+
+        private int time;   // sec
+        private int tick;   // tick
+
+        DiscoveryPeriod(int time) {
+            this.time = time;
+            this.tick = time2Tick(time);
+        }
+
+        public int getTime() {
+            return time;
+        }
+
+        public void setTime(int time) {
+            this.time = time;
+            this.tick = time2Tick(time);
+        }
+
+        public int getTick() {
+            return tick;
+        }
+
+        private int time2Tick(int time) {
+            return (int) (time / (discoveryTimerTick / 1000));
+        }
+    }
+
     private RawPacket createDiscoveryPacket(NodeConnector nodeConnector) {
         String nodeId = HexEncode.longToHexString((Long) nodeConnector.getNode().getID());
 
@@ -1476,44 +1507,50 @@ public class DiscoveryService implements IInventoryShimExternalListener, IDataPa
         return sourceMac;
     }
 
+    private int getDiscoveryTicks(DiscoveryPeriod dp, String val) {
+        if (dp == null) {
+            return 0;
+        }
+
+        if (val != null) {
+            try {
+                dp.setTime(Integer.parseInt(val));
+            } catch (Exception e) {
+            }
+        }
+
+        return dp.getTick();
+    }
+
     /**
      * This method returns the interval which determines how often the discovery
-     * packets will be sent. Default is 300 seconds.
+     * packets will be sent.
      *
-     * @return The discovery interval in second
+     * @return The discovery interval in ticks
      */
     private int getDiscoveryInterval() {
-        String elapsedTime = System.getProperty("of.discoveryInterval");
-        int rv = 300;
-
-        try {
-            if (elapsedTime != null) {
-                rv = Integer.parseInt(elapsedTime);
-            }
-        } catch (Exception e) {
-        }
-
-        return rv;
+        String intvl = System.getProperty("of.discoveryInterval");
+        return getDiscoveryTicks(DiscoveryPeriod.INTERVAL, intvl);
     }
 
     /**
      * This method returns the timeout value in waiting for response of a
-     * discovery query. Default is 60 seconds.
+     * discovery query.
      *
-     * @return The discovery timeout in second
+     * @return The discovery timeout in ticks
      */
     private int getDiscoveryTimeout() {
-        String elapsedTime = System.getProperty("of.discoveryTimeout");
-        int rv = 60;
-
-        try {
-            if (elapsedTime != null) {
-                rv = Integer.parseInt(elapsedTime);
-            }
-        } catch (Exception e) {
-        }
+        String timeout = System.getProperty("of.discoveryTimeout");
+        return getDiscoveryTicks(DiscoveryPeriod.TIMEOUT, timeout);
+    }
 
-        return rv;
+    /**
+     * This method returns the discovery entry aging time in ticks.
+     *
+     * @return The aging time in ticks
+     */
+    private int getDiscoveryAgeout() {
+        return getDiscoveryTicks(DiscoveryPeriod.AGEOUT, null);
     }
 
     /**
index edb3537..4470e17 100644 (file)
@@ -8,14 +8,13 @@
 
 package org.opendaylight.controller.sal.core;
 
-import java.util.Arrays;
-
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
 
 import org.opendaylight.controller.sal.utils.HexEncode;
+
 /**
  * The class contains MAC address property.
  */
@@ -24,7 +23,7 @@ import org.opendaylight.controller.sal.utils.HexEncode;
 public class MacAddress extends Property implements Cloneable {
     private static final long serialVersionUID = 1L;
     @XmlElement(name="macAddress")
-    private final byte[] address;
+    private final String address;
     public static final String name = "macAddress";
 
     /*
@@ -42,20 +41,36 @@ public class MacAddress extends Property implements Cloneable {
      *
      *
      * @param nodeMacAddress
-     *            Data Link Address for the node
+     *            Data Link Address for the node in byte array format
      *
      * @return the constructed object
      */
     public MacAddress(byte[] nodeMacAddress) {
         super(name);
-        this.address = nodeMacAddress.clone();
+        this.address = HexEncode.bytesToHexStringFormat(nodeMacAddress);
+    }
+
+    /**
+     * Constructor to create DatalinkAddress property which contains the MAC
+     * address. The property will be attached to a
+     * {@link org.opendaylight.controller.sal.core.Node}.
+     *
+     *
+     * @param nodeMacAddress
+     *            Data Link Address for the node in String format
+     *
+     * @return the constructed object
+     */
+    public MacAddress(String nodeMacAddress) {
+        super(name);
+        this.address = nodeMacAddress;
     }
 
     /**
-     * @return the node MAC address
+     * @return the node MAC address in byte array format
      */
     public byte[] getMacAddress() {
-        return this.address.clone();
+        return HexEncode.bytesFromHexString(this.address);
     }
 
     @Override
@@ -67,7 +82,8 @@ public class MacAddress extends Property implements Cloneable {
     public int hashCode() {
         final int prime = 31;
         int result = super.hashCode();
-        result = prime * result + Arrays.hashCode(address);
+        result = prime * result
+                + ((address == null) ? 0 : address.hashCode());
         return result;
     }
 
@@ -83,7 +99,7 @@ public class MacAddress extends Property implements Cloneable {
             return false;
         }
         MacAddress other = (MacAddress) obj;
-        if (!Arrays.equals(address, other.address)) {
+        if (!address.equals(other.address)) {
             return false;
         }
         return true;
@@ -91,7 +107,6 @@ public class MacAddress extends Property implements Cloneable {
 
     @Override
     public String toString() {
-        return "MacAddress [address=" +
-            HexEncode.bytesToHexStringFormat(address) + "]";
+        return "MacAddress[" + address + "]";
     }
 }
index 8c95c46..7fd9bc1 100644 (file)
@@ -1662,7 +1662,13 @@ CommandProvider {
         if (nodeSet == null) {
             return;
         }
+        List<String> nodeArray = new ArrayList<String>();
         for (Node node : nodeSet) {
+            nodeArray.add(node.toString());
+        }
+        Collections.sort(nodeArray);
+        for (String str: nodeArray) {
+            Node node = Node.fromString(str);
             Description desc = ((Description) getNodeProp(node,
                     Description.propertyName));
             Tier tier = ((Tier) getNodeProp(node, Tier.TierPropName));