From: Robert Varga Date: Sat, 15 Feb 2014 02:17:36 +0000 (+0100) Subject: Handle openflowplugin returning null X-Git-Tag: autorelease-tag-v20140601202136_82eb3f9~435 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=86fbf772e22bd386e790682594b8188fb450ad8c 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 --- 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 7b756d8f48..c29b6a7730 100644 --- 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 com.google.common.base.Function; 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 { private static final Logger logger = LoggerFactory.getLogger(AbstractStatsTracker.class); - private static final Function, TransactionId> FUNCTION = - new Function, TransactionId>() { + private final FutureCallback> callback = + new FutureCallback>() { @Override - public TransactionId apply(RpcResult 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 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 { } protected final void requestHelper(Future> future) { - context.registerTransaction(Futures.transform(JdkFutureAdapters.listenInPoolThread(future), FUNCTION)); + Futures.addCallback(JdkFutureAdapters.listenInPoolThread(future), callback); } 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 27aa17a904..520b344199 100644 --- 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 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 @@ -24,6 +22,6 @@ interface FlowCapableContext { InstanceIdentifier getNodeIdentifier(); NodeRef getNodeRef(); DataModificationTransaction startDataModification(); - void registerTransaction(ListenableFuture future); - void registerTableTransaction(ListenableFuture 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 d62b680e14..ecf2cf3665 100644 --- 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 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 @@ -298,34 +295,14 @@ public final class NodeStatisticsHandler implements AutoCloseable, FlowCapableCo } @Override - public void registerTransaction(final ListenableFuture future) { - Futures.addCallback(future, new FutureCallback() { - @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 - public void registerTableTransaction(final ListenableFuture future, final Short id) { - Futures.addCallback(future, new FutureCallback() { - @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); } }