Handle openflowplugin returning null

It turns out that openflow plugin reports success with null transaction
ID if the switch does not support a feature. This is quite bad, but we
can recover rather easily. Rework the registration contract to take care
of the changes.

Change-Id: I62fe7badeb5d792c8533762f5e68b7d37811f8ed
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/AbstractStatsTracker.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/AbstractStatsTracker.java
index 7b756d8f48747fbdb89006c606ee91563b836340..c29b6a7730ced4f24374d2e4e67a650f3ed69293 100644 (file)
--- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/AbstractStatsTracker.java
+++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/AbstractStatsTracker.java
@@ -25,23 +25,38 @@ import org.opendaylight.yangtools.yang.common.RpcResult;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 

-import com.google.common.base.Function;

 import com.google.common.base.Preconditions;
 import com.google.common.base.Preconditions;

+import com.google.common.util.concurrent.FutureCallback;

 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.JdkFutureAdapters;
 
 abstract class AbstractStatsTracker<I, K> {
     private static final Logger logger = LoggerFactory.getLogger(AbstractStatsTracker.class);
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.JdkFutureAdapters;
 
 abstract class AbstractStatsTracker<I, K> {
     private static final Logger logger = LoggerFactory.getLogger(AbstractStatsTracker.class);

-    private static final Function<RpcResult<? extends TransactionAware>, TransactionId> FUNCTION =
-            new Function<RpcResult<? extends TransactionAware>, TransactionId>() {
+    private final FutureCallback<RpcResult<? extends TransactionAware>> callback =
+            new FutureCallback<RpcResult<? extends TransactionAware>>() {

         @Override
         @Override

-        public TransactionId apply(RpcResult<? extends TransactionAware> input) {
-            if (!input.isSuccessful()) {
-                logger.debug("Statistics request failed: {}", input.getErrors());
-                throw new RPCFailedException("Failed to send statistics request", input.getErrors());
+        public void onSuccess(RpcResult<? extends TransactionAware> result) {
+            if (result.isSuccessful()) {
+                final TransactionId id = result.getResult().getTransactionId();
+                if (id == null) {
+                    final Throwable t = new UnsupportedOperationException("No protocol support");
+                    t.fillInStackTrace();
+                    onFailure(t);
+                } else {
+                    context.registerTransaction(id);
+                }
+            } else {
+                logger.debug("Statistics request failed: {}", result.getErrors());
+
+                final Throwable t = new RPCFailedException("Failed to send statistics request", result.getErrors());
+                t.fillInStackTrace();
+                onFailure(t);

             }
             }

+        }

 
 

-            return input.getResult().getTransactionId();
+        @Override
+        public void onFailure(Throwable t) {
+            logger.debug("Failed to send statistics request", t);

         }
     };
 
         }
     };
 


@@ -67,7 +82,7 @@ abstract class AbstractStatsTracker<I, K> {
     }
 
     protected final <T extends TransactionAware> void requestHelper(Future<RpcResult<T>> future) {
     }
 
     protected final <T extends TransactionAware> void requestHelper(Future<RpcResult<T>> future) {

-        context.registerTransaction(Futures.transform(JdkFutureAdapters.listenInPoolThread(future), FUNCTION));
+        Futures.addCallback(JdkFutureAdapters.listenInPoolThread(future), callback);

     }
 
     protected final DataModificationTransaction startTransaction() {
     }
 
     protected final DataModificationTransaction startTransaction() {

diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowCapableContext.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowCapableContext.java
index 27aa17a9045e9f30c616152a65ada7e69d5397e0..520b344199a2051c1841acd6409d34b6ed5bc5a8 100644 (file)
--- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowCapableContext.java
+++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowCapableContext.java
@@ -13,8 +13,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeRef
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 

-import com.google.common.util.concurrent.ListenableFuture;
-

 /**
  * Interface exposed to AbstractStatsTracker by its parent NodeStatisticsHandler.
  * While we could simply exist without this interface, its purpose is to document
 /**
  * Interface exposed to AbstractStatsTracker by its parent NodeStatisticsHandler.
  * While we could simply exist without this interface, its purpose is to document


@@ -24,6 +22,6 @@ interface FlowCapableContext {
     InstanceIdentifier<Node> getNodeIdentifier();
     NodeRef getNodeRef();
     DataModificationTransaction startDataModification();
     InstanceIdentifier<Node> getNodeIdentifier();
     NodeRef getNodeRef();
     DataModificationTransaction startDataModification();

-    void registerTransaction(ListenableFuture<TransactionId> future);
-    void registerTableTransaction(ListenableFuture<TransactionId> future, Short tableId);
+    void registerTransaction(TransactionId id);
+    void registerTableTransaction(TransactionId id, Short tableId);

 }
 }

diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/NodeStatisticsHandler.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/NodeStatisticsHandler.java
index d62b680e1466c89d88180b51b0d89a97b2120602..ecf2cf366591bc1f7383981c52957741d42cc669 100644 (file)
--- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/NodeStatisticsHandler.java
+++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/NodeStatisticsHandler.java
@@ -55,9 +55,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Preconditions;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Preconditions;

-import com.google.common.util.concurrent.FutureCallback;
-import com.google.common.util.concurrent.Futures;
-import com.google.common.util.concurrent.ListenableFuture;

 
 /**
  * This class handles the lifecycle of per-node statistics. It receives data
 
 /**
  * This class handles the lifecycle of per-node statistics. It receives data


@@ -298,34 +295,14 @@ public final class NodeStatisticsHandler implements AutoCloseable, FlowCapableCo
     }
 
     @Override
     }
 
     @Override

-    public void registerTransaction(final ListenableFuture<TransactionId> future) {
-        Futures.addCallback(future, new FutureCallback<TransactionId>() {
-            @Override
-            public void onSuccess(TransactionId result) {
-                msgManager.recordExpectedTransaction(result);
-                logger.debug("Transaction {} for node {} sent successfully", result, targetNodeKey);
-            }
-
-            @Override
-            public void onFailure(Throwable t) {
-                logger.warn("Failed to send statistics request for node {}", targetNodeKey, t);
-            }
-        });
+    public void registerTransaction(TransactionId id) {
+        msgManager.recordExpectedTransaction(id);
+        logger.debug("Transaction {} for node {} sent successfully", id, targetNodeKey);

     }
 
     @Override
     }
 
     @Override

-    public void registerTableTransaction(final ListenableFuture<TransactionId> future, final Short id) {
-        Futures.addCallback(future, new FutureCallback<TransactionId>() {
-            @Override
-            public void onSuccess(TransactionId result) {
-                msgManager.recordExpectedTableTransaction(result, id);
-                logger.debug("Transaction {} for node {} table {} sent successfully", result, targetNodeKey, id);
-            }
-
-            @Override
-            public void onFailure(Throwable t) {
-                logger.warn("Failed to send table statistics request for node {} table {}", targetNodeKey, id, t);
-            }
-        });
+    public void registerTableTransaction(final TransactionId id, final Short table) {
+        msgManager.recordExpectedTableTransaction(id, table);
+        logger.debug("Transaction {} for node {} table {} sent successfully", id, targetNodeKey, table);

     }
 }
     }
 }