From: Alexis de Talhouët Date: Mon, 4 Apr 2016 21:38:59 +0000 (-0400) Subject: Bug 4940 - correctly implement default-request-timeout-millis X-Git-Tag: release/beryllium-sr2~8 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F09%2F37109%2F5;p=netconf.git Bug 4940 - correctly implement default-request-timeout-millis This is a follow up patch of this one https://git.opendaylight.org/gerrit/#/c/33209/ This revert 2a301c41e506c834c63b53aa62df68cd83385553 The implementation made in previous patch was blocking transaction for a given timeout, but the blocking wasn't handled at the right place, ending up blocking restconf/user thread. Change-Id: Ide2f24890d6b9fa577bba793d0b1f0b8ea85a222 Signed-off-by: Alexis de Talhouët --- diff --git a/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/AbstractNetconfTopology.java b/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/AbstractNetconfTopology.java index 488b680ab5..0a5f6df9d8 100644 --- a/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/AbstractNetconfTopology.java +++ b/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/AbstractNetconfTopology.java @@ -275,11 +275,11 @@ public abstract class AbstractNetconfTopology implements NetconfTopology, Bindin RemoteDeviceId remoteDeviceId = new RemoteDeviceId(nodeId.getValue(), address); RemoteDeviceHandler salFacade = - createSalFacade(remoteDeviceId, domBroker, bindingAwareBroker, defaultRequestTimeoutMillis); + createSalFacade(remoteDeviceId, domBroker, bindingAwareBroker); if (keepaliveDelay > 0) { LOG.warn("Adding keepalive facade, for device {}", nodeId); - salFacade = new KeepaliveSalFacade(remoteDeviceId, salFacade, keepaliveExecutor.getExecutor(), keepaliveDelay); + salFacade = new KeepaliveSalFacade(remoteDeviceId, salFacade, keepaliveExecutor.getExecutor(), keepaliveDelay, defaultRequestTimeoutMillis); } final NetconfDevice.SchemaResourcesDTO schemaResourcesDTO = setupSchemaCacheDTO(nodeId, node); @@ -405,7 +405,7 @@ public abstract class AbstractNetconfTopology implements NetconfTopology, Bindin .build(); } - protected abstract RemoteDeviceHandler createSalFacade(final RemoteDeviceId id, final Broker domBroker, final BindingAwareBroker bindingBroker, long defaultRequestTimeoutMillis); + protected abstract RemoteDeviceHandler createSalFacade(final RemoteDeviceId id, final Broker domBroker, final BindingAwareBroker bindingBroker); @Override public abstract ConnectionStatusListenerRegistration registerConnectionStatusListener(NodeId node, RemoteDeviceHandler listener); diff --git a/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/impl/ClusteredNetconfTopology.java b/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/impl/ClusteredNetconfTopology.java index 76656b4674..45449de377 100644 --- a/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/impl/ClusteredNetconfTopology.java +++ b/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/impl/ClusteredNetconfTopology.java @@ -150,11 +150,11 @@ public class ClusteredNetconfTopology extends AbstractNetconfTopology implements RemoteDeviceId remoteDeviceId = new RemoteDeviceId(nodeId.getValue(), address); RemoteDeviceHandler salFacade = - createSalFacade(remoteDeviceId, domBroker, bindingAwareBroker, defaultRequestTimeoutMillis); + createSalFacade(remoteDeviceId, domBroker, bindingAwareBroker); if (keepaliveDelay > 0) { LOG.warn("Adding keepalive facade, for device {}", nodeId); - salFacade = new KeepaliveSalFacade(remoteDeviceId, salFacade, keepaliveExecutor.getExecutor(), keepaliveDelay); + salFacade = new KeepaliveSalFacade(remoteDeviceId, salFacade, keepaliveExecutor.getExecutor(), keepaliveDelay, defaultRequestTimeoutMillis); } final NetconfDevice.SchemaResourcesDTO schemaResourcesDTO = setupSchemaCacheDTO(nodeId, node); @@ -166,8 +166,8 @@ public class ClusteredNetconfTopology extends AbstractNetconfTopology implements } @Override - protected RemoteDeviceHandler createSalFacade(final RemoteDeviceId id, final Broker domBroker, final BindingAwareBroker bindingBroker, long defaultRequestTimeoutMillis) { - return new TopologyMountPointFacade(topologyId, id, domBroker, bindingBroker, defaultRequestTimeoutMillis); + protected RemoteDeviceHandler createSalFacade(final RemoteDeviceId id, final Broker domBroker, final BindingAwareBroker bindingBroker) { + return new TopologyMountPointFacade(topologyId, id, domBroker, bindingBroker); } @Override diff --git a/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImpl.java b/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImpl.java index 0a14740903..b31943d7a6 100644 --- a/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImpl.java +++ b/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImpl.java @@ -79,8 +79,8 @@ public class NetconfTopologyImpl extends AbstractNetconfTopology implements Data } @Override - protected RemoteDeviceHandler createSalFacade(RemoteDeviceId id, Broker domBroker, BindingAwareBroker bindingBroker, long defaultRequestTimeoutMillis) { - return new NetconfDeviceSalFacade(id, domBroker, bindingAwareBroker, defaultRequestTimeoutMillis); + protected RemoteDeviceHandler createSalFacade(RemoteDeviceId id, Broker domBroker, BindingAwareBroker bindingBroker) { + return new NetconfDeviceSalFacade(id, domBroker, bindingAwareBroker); } @Override diff --git a/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/pipeline/NetconfDeviceMasterDataBroker.java b/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/pipeline/NetconfDeviceMasterDataBroker.java index dbc11e4118..17bb793b1f 100644 --- a/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/pipeline/NetconfDeviceMasterDataBroker.java +++ b/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/pipeline/NetconfDeviceMasterDataBroker.java @@ -57,9 +57,9 @@ public class NetconfDeviceMasterDataBroker implements ProxyNetconfDeviceDataBrok public NetconfDeviceMasterDataBroker(final ActorSystem actorSystem, final RemoteDeviceId id, final SchemaContext schemaContext, final DOMRpcService rpc, - final NetconfSessionPreferences netconfSessionPreferences, final long requestTimeoutMillis) { + final NetconfSessionPreferences netconfSessionPreferences) { this.id = id; - delegateBroker = new NetconfDeviceDataBroker(id, schemaContext, rpc, netconfSessionPreferences, requestTimeoutMillis); + delegateBroker = new NetconfDeviceDataBroker(id, schemaContext, rpc, netconfSessionPreferences); this.actorSystem = actorSystem; // only ever need 1 readTx since it doesnt need to be closed diff --git a/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/pipeline/TopologyMountPointFacade.java b/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/pipeline/TopologyMountPointFacade.java index 269db6907b..c79627be5a 100644 --- a/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/pipeline/TopologyMountPointFacade.java +++ b/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/pipeline/TopologyMountPointFacade.java @@ -43,7 +43,6 @@ public class TopologyMountPointFacade implements AutoCloseable, RemoteDeviceHand private final RemoteDeviceId id; private final Broker domBroker; private final BindingAwareBroker bindingBroker; - private final long defaultRequestTimeoutMillis; private SchemaContext remoteSchemaContext = null; private NetconfSessionPreferences netconfSessionPreferences = null; @@ -58,13 +57,11 @@ public class TopologyMountPointFacade implements AutoCloseable, RemoteDeviceHand public TopologyMountPointFacade(final String topologyId, final RemoteDeviceId id, final Broker domBroker, - final BindingAwareBroker bindingBroker, - long defaultRequestTimeoutMillis) { + final BindingAwareBroker bindingBroker) { this.topologyId = topologyId; this.id = id; this.domBroker = domBroker; this.bindingBroker = bindingBroker; - this.defaultRequestTimeoutMillis = defaultRequestTimeoutMillis; this.salProvider = new ClusteredNetconfDeviceMountInstanceProxy(id); registerToSal(domBroker); } @@ -124,7 +121,7 @@ public class TopologyMountPointFacade implements AutoCloseable, RemoteDeviceHand deviceDataBroker = TypedActor.get(context).typedActorOf(new TypedProps<>(ProxyNetconfDeviceDataBroker.class, new Creator() { @Override public NetconfDeviceMasterDataBroker create() throws Exception { - return new NetconfDeviceMasterDataBroker(actorSystem, id, remoteSchemaContext, deviceRpc, netconfSessionPreferences, defaultRequestTimeoutMillis); + return new NetconfDeviceMasterDataBroker(actorSystem, id, remoteSchemaContext, deviceRpc, netconfSessionPreferences); } }), MOUNT_POINT); LOG.debug("Master data broker registered on path {}", TypedActor.get(actorSystem).getActorRefFor(deviceDataBroker).path()); diff --git a/opendaylight/netconf/netconf-topology/src/test/java/org/opendaylight/netconf/topology/pipeline/TopologyMountPointFacadeTest.java b/opendaylight/netconf/netconf-topology/src/test/java/org/opendaylight/netconf/topology/pipeline/TopologyMountPointFacadeTest.java index 2b4b37418a..68342a56e5 100644 --- a/opendaylight/netconf/netconf-topology/src/test/java/org/opendaylight/netconf/topology/pipeline/TopologyMountPointFacadeTest.java +++ b/opendaylight/netconf/netconf-topology/src/test/java/org/opendaylight/netconf/topology/pipeline/TopologyMountPointFacadeTest.java @@ -48,7 +48,7 @@ public class TopologyMountPointFacadeTest { MockitoAnnotations.initMocks(this); - mountPointFacade = new TopologyMountPointFacade(TOPOLOGY_ID, REMOTE_DEVICE_ID, domBroker, bindingBroker, 1L); + mountPointFacade = new TopologyMountPointFacade(TOPOLOGY_ID, REMOTE_DEVICE_ID, domBroker, bindingBroker); mountPointFacade.registerConnectionStatusListener(connectionStatusListener1); mountPointFacade.registerConnectionStatusListener(connectionStatusListener2); diff --git a/opendaylight/netconf/sal-netconf-connector/pom.xml b/opendaylight/netconf/sal-netconf-connector/pom.xml index e6e79d0d39..068f91ed74 100644 --- a/opendaylight/netconf/sal-netconf-connector/pom.xml +++ b/opendaylight/netconf/sal-netconf-connector/pom.xml @@ -174,18 +174,6 @@ mockito-core test - - org.powermock - powermock-api-mockito - 1.5.2 - test - - - org.powermock - powermock-module-junit4 - 1.5.2 - test - diff --git a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/controller/config/yang/md/sal/connector/netconf/NetconfConnectorModule.java b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/controller/config/yang/md/sal/connector/netconf/NetconfConnectorModule.java index 785be9954d..d7222c1bb2 100644 --- a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/controller/config/yang/md/sal/connector/netconf/NetconfConnectorModule.java +++ b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/controller/config/yang/md/sal/connector/netconf/NetconfConnectorModule.java @@ -192,14 +192,14 @@ public final class NetconfConnectorModule extends org.opendaylight.controller.co final ExecutorService globalProcessingExecutor = processingExecutor.getExecutor(); RemoteDeviceHandler salFacade - = new NetconfDeviceSalFacade(id, domRegistry, bindingRegistry, getDefaultRequestTimeoutMillis()); + = new NetconfDeviceSalFacade(id, domRegistry, bindingRegistry); final Long keepaliveDelay = getKeepaliveDelay(); if (shouldSendKeepalive()) { // Keepalive executor is optional for now and a default instance is supported final ScheduledExecutorService executor = keepaliveExecutor == null ? DEFAULT_KEEPALIVE_EXECUTOR : keepaliveExecutor.getExecutor(); - salFacade = new KeepaliveSalFacade(id, salFacade, executor, keepaliveDelay); + salFacade = new KeepaliveSalFacade(id, salFacade, executor, keepaliveDelay, getDefaultRequestTimeoutMillis()); } // Setup information related to the SchemaRegistry, SchemaResourceFactory, etc. diff --git a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/KeepaliveSalFacade.java b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/KeepaliveSalFacade.java index 569f642130..c450ac68fe 100644 --- a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/KeepaliveSalFacade.java +++ b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/KeepaliveSalFacade.java @@ -51,28 +51,33 @@ public final class KeepaliveSalFacade implements RemoteDeviceHandler salFacade; private final ScheduledExecutorService executor; private final long keepaliveDelaySeconds; private final ResetKeepalive resetKeepaliveTask; + private final long defaultRequestTimeoutMillis; private volatile NetconfDeviceCommunicator listener; private volatile ScheduledFuture currentKeepalive; private volatile DOMRpcService currentDeviceRpc; public KeepaliveSalFacade(final RemoteDeviceId id, final RemoteDeviceHandler salFacade, - final ScheduledExecutorService executor, final long keepaliveDelaySeconds) { + final ScheduledExecutorService executor, final long keepaliveDelaySeconds, final long defaultRequestTimeoutMillis) { this.id = id; this.salFacade = salFacade; this.executor = executor; this.keepaliveDelaySeconds = keepaliveDelaySeconds; + this.defaultRequestTimeoutMillis = defaultRequestTimeoutMillis; this.resetKeepaliveTask = new ResetKeepalive(); } public KeepaliveSalFacade(final RemoteDeviceId id, final RemoteDeviceHandler salFacade, final ScheduledExecutorService executor) { - this(id, salFacade, executor, DEFAULT_DELAY); + this(id, salFacade, executor, DEFAULT_DELAY, DEFAULT_TRANSACTION_TIMEOUT_MILLI); } /** @@ -118,7 +123,7 @@ public final class KeepaliveSalFacade implements RemoteDeviceHandler { + private class ResetKeepalive implements FutureCallback { @Override public void onSuccess(@Nullable final DOMRpcResult result) { // No matter what response we got, rpc-reply or rpc-error, we got it from device so the netconf session is OK @@ -230,17 +235,44 @@ public final class KeepaliveSalFacade implements RemoteDeviceHandler rpcResultFuture; + + public RequestTimeoutTask(final CheckedFuture rpcResultFuture) { + this.rpcResultFuture = rpcResultFuture; + } + + @Override + public void run() { + if (!rpcResultFuture.isDone()) { + rpcResultFuture.cancel(true); + } + } + } + /** - * DOMRpcService proxy that attaches reset-keepalive-task to each RPC invocation. + * DOMRpcService proxy that attaches reset-keepalive-task and schedule + * request-timeout-task to each RPC invocation. */ private static final class KeepaliveDOMRpcService implements DOMRpcService { private final DOMRpcService deviceRpc; private ResetKeepalive resetKeepaliveTask; + private final long defaultRequestTimeoutMillis; + private final ScheduledExecutorService executor; - public KeepaliveDOMRpcService(final DOMRpcService deviceRpc, final ResetKeepalive resetKeepaliveTask) { + public KeepaliveDOMRpcService(final DOMRpcService deviceRpc, final ResetKeepalive resetKeepaliveTask, + final long defaultRequestTimeoutMillis, final ScheduledExecutorService executor) { this.deviceRpc = deviceRpc; this.resetKeepaliveTask = resetKeepaliveTask; + this.defaultRequestTimeoutMillis = defaultRequestTimeoutMillis; + this.executor = executor; } @Nonnull @@ -248,6 +280,10 @@ public final class KeepaliveSalFacade implements RemoteDeviceHandler invokeRpc(@Nonnull final SchemaPath type, final NormalizedNode input) { final CheckedFuture domRpcResultDOMRpcExceptionCheckedFuture = deviceRpc.invokeRpc(type, input); Futures.addCallback(domRpcResultDOMRpcExceptionCheckedFuture, resetKeepaliveTask); + + final RequestTimeoutTask timeoutTask = new RequestTimeoutTask(domRpcResultDOMRpcExceptionCheckedFuture); + executor.schedule(timeoutTask, defaultRequestTimeoutMillis, TimeUnit.MILLISECONDS); + return domRpcResultDOMRpcExceptionCheckedFuture; } diff --git a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceDataBroker.java b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceDataBroker.java index 7283cae9b8..421e52da71 100644 --- a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceDataBroker.java +++ b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceDataBroker.java @@ -36,16 +36,14 @@ import org.opendaylight.yangtools.yang.model.api.SchemaContext; public final class NetconfDeviceDataBroker implements DOMDataBroker { private final RemoteDeviceId id; private final NetconfBaseOps netconfOps; - private final long requestTimeoutMillis; private final boolean rollbackSupport; private boolean candidateSupported; private boolean runningWritable; - public NetconfDeviceDataBroker(final RemoteDeviceId id, final SchemaContext schemaContext, final DOMRpcService rpc, final NetconfSessionPreferences netconfSessionPreferences, long requestTimeoutMillis) { + public NetconfDeviceDataBroker(final RemoteDeviceId id, final SchemaContext schemaContext, final DOMRpcService rpc, final NetconfSessionPreferences netconfSessionPreferences) { this.id = id; this.netconfOps = new NetconfBaseOps(rpc, schemaContext); - this.requestTimeoutMillis = requestTimeoutMillis; // get specific attributes from netconf preferences and get rid of it // no need to keep the entire preferences object, its quite big with all the capability QNames candidateSupported = netconfSessionPreferences.isCandidateSupported(); @@ -57,7 +55,7 @@ public final class NetconfDeviceDataBroker implements DOMDataBroker { @Override public DOMDataReadOnlyTransaction newReadOnlyTransaction() { - return new ReadOnlyTx(netconfOps, id, requestTimeoutMillis); + return new ReadOnlyTx(netconfOps, id); } @Override @@ -69,12 +67,12 @@ public final class NetconfDeviceDataBroker implements DOMDataBroker { public DOMDataWriteTransaction newWriteOnlyTransaction() { if(candidateSupported) { if(runningWritable) { - return new WriteCandidateRunningTx(id, netconfOps, rollbackSupport, requestTimeoutMillis); + return new WriteCandidateRunningTx(id, netconfOps, rollbackSupport); } else { - return new WriteCandidateTx(id, netconfOps, rollbackSupport, requestTimeoutMillis); + return new WriteCandidateTx(id, netconfOps, rollbackSupport); } } else { - return new WriteRunningTx(id, netconfOps, rollbackSupport, requestTimeoutMillis); + return new WriteRunningTx(id, netconfOps, rollbackSupport); } } diff --git a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalFacade.java b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalFacade.java index 7bb3df08a5..2a817db7ec 100644 --- a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalFacade.java +++ b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalFacade.java @@ -30,14 +30,12 @@ public final class NetconfDeviceSalFacade implements AutoCloseable, RemoteDevice private final RemoteDeviceId id; private final NetconfDeviceSalProvider salProvider; - private final long defaultRequestTimeoutMillis; private final List salRegistrations = Lists.newArrayList(); - public NetconfDeviceSalFacade(final RemoteDeviceId id, final Broker domBroker, final BindingAwareBroker bindingBroker, long defaultRequestTimeoutMillis) { + public NetconfDeviceSalFacade(final RemoteDeviceId id, final Broker domBroker, final BindingAwareBroker bindingBroker) { this.id = id; this.salProvider = new NetconfDeviceSalProvider(id); - this.defaultRequestTimeoutMillis = defaultRequestTimeoutMillis; registerToSal(domBroker, bindingBroker); } @@ -55,7 +53,7 @@ public final class NetconfDeviceSalFacade implements AutoCloseable, RemoteDevice public synchronized void onDeviceConnected(final SchemaContext schemaContext, final NetconfSessionPreferences netconfSessionPreferences, final DOMRpcService deviceRpc) { - final DOMDataBroker domBroker = new NetconfDeviceDataBroker(id, schemaContext, deviceRpc, netconfSessionPreferences, defaultRequestTimeoutMillis); + final DOMDataBroker domBroker = new NetconfDeviceDataBroker(id, schemaContext, deviceRpc, netconfSessionPreferences); final NetconfDeviceNotificationService notificationService = new NetconfDeviceNotificationService(); diff --git a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/AbstractWriteTx.java b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/AbstractWriteTx.java index a12ba3a02a..3aeff93da4 100644 --- a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/AbstractWriteTx.java +++ b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/AbstractWriteTx.java @@ -11,11 +11,8 @@ package org.opendaylight.netconf.sal.connect.netconf.sal.tx; import com.google.common.base.Function; import com.google.common.base.Optional; import com.google.common.base.Preconditions; -import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import org.opendaylight.controller.md.sal.common.api.TransactionStatus; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.dom.api.DOMDataWriteTransaction; @@ -29,7 +26,6 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild; import org.opendaylight.yangtools.yang.data.api.schema.MixinNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.model.repo.api.SchemaSourceException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,15 +33,13 @@ public abstract class AbstractWriteTx implements DOMDataWriteTransaction { private static final Logger LOG = LoggerFactory.getLogger(AbstractWriteTx.class); - protected final long defaultRequestTimeoutMillis; protected final RemoteDeviceId id; protected final NetconfBaseOps netOps; protected final boolean rollbackSupport; // Allow commit to be called only once protected boolean finished = false; - public AbstractWriteTx(final long requestTimeoutMillis, final NetconfBaseOps netOps, final RemoteDeviceId id, final boolean rollbackSupport) { - this.defaultRequestTimeoutMillis = requestTimeoutMillis; + public AbstractWriteTx(final NetconfBaseOps netOps, final RemoteDeviceId id, final boolean rollbackSupport) { this.netOps = netOps; this.id = id; this.rollbackSupport = rollbackSupport; @@ -176,18 +170,4 @@ public abstract class AbstractWriteTx implements DOMDataWriteTransaction { } protected abstract void editConfig(DataContainerChild editStructure, Optional defaultOperation) throws NetconfDocumentedException; - - - protected ListenableFuture perfomRequestWithTimeout(String operation, ListenableFuture future) { - try { - future.get(defaultRequestTimeoutMillis, TimeUnit.MILLISECONDS); - } catch (InterruptedException | ExecutionException e) { - LOG.error("{}: {} failed with error", operation, id, e); - return Futures.immediateFailedCheckedFuture(new RuntimeException(id + ": " + operation + " failed")); - } catch (TimeoutException e) { - LOG.warn("{}: Unable to {} after {} milliseconds", id, operation, defaultRequestTimeoutMillis, e); - return Futures.immediateFailedCheckedFuture(new SchemaSourceException(e.getMessage())); - } - return future; - } } diff --git a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/ReadOnlyTx.java b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/ReadOnlyTx.java index 28d75cdaa4..dda5485109 100644 --- a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/ReadOnlyTx.java +++ b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/ReadOnlyTx.java @@ -16,9 +16,6 @@ import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; - import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException; import org.opendaylight.controller.md.sal.dom.api.DOMDataReadOnlyTransaction; @@ -44,12 +41,9 @@ public final class ReadOnlyTx implements DOMDataReadOnlyTransaction { private final RemoteDeviceId id; private final FutureCallback loggingCallback; - private final long requestTimeoutMillis; - - public ReadOnlyTx(final NetconfBaseOps netconfOps, final RemoteDeviceId id, final long requestTimeoutMillis) { + public ReadOnlyTx(final NetconfBaseOps netconfOps, final RemoteDeviceId id) { this.netconfOps = netconfOps; this.id = id; - this.requestTimeoutMillis = requestTimeoutMillis; // Simple logging callback to log result of read operation loggingCallback = new FutureCallback() { @@ -84,10 +78,6 @@ public final class ReadOnlyTx implements DOMDataReadOnlyTransaction { } }); - if(!readWithTimeout("readConfigurationData", configRunning)) { - return null; - } - return MappingCheckedFuture.create(transformedFuture, ReadFailedException.MAPPER); } @@ -119,10 +109,6 @@ public final class ReadOnlyTx implements DOMDataReadOnlyTransaction { } }); - if(!readWithTimeout("readOperationalData", configCandidate)) { - return null; - } - return MappingCheckedFuture.create(transformedFuture, ReadFailedException.MAPPER); } @@ -161,18 +147,4 @@ public final class ReadOnlyTx implements DOMDataReadOnlyTransaction { public Object getIdentifier() { return this; } - - private boolean readWithTimeout(String operation, ListenableFuture future) { - try { - future.get(requestTimeoutMillis, TimeUnit.MILLISECONDS); - } catch (InterruptedException | ExecutionException e) { - LOG.error("{}: {} failed with error", id, operation, e); - throw new RuntimeException(id + ": readOperationalData failed"); - } catch (TimeoutException e) { - LOG.warn("{}: Unable to {} after {} milliseconds", id, operation, requestTimeoutMillis, e); - future.cancel(true); - return false; - } - return true; - } } diff --git a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/WriteCandidateRunningTx.java b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/WriteCandidateRunningTx.java index 0b9bb98e9c..c076d0b7a8 100644 --- a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/WriteCandidateRunningTx.java +++ b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/WriteCandidateRunningTx.java @@ -29,8 +29,8 @@ public class WriteCandidateRunningTx extends WriteCandidateTx { private static final Logger LOG = LoggerFactory.getLogger(WriteCandidateRunningTx.class); - public WriteCandidateRunningTx(final RemoteDeviceId id, final NetconfBaseOps netOps, final boolean rollbackSupport, final long requestTimeoutMillis) { - super(id, netOps, rollbackSupport, requestTimeoutMillis); + public WriteCandidateRunningTx(final RemoteDeviceId id, final NetconfBaseOps netOps, final boolean rollbackSupport) { + super(id, netOps, rollbackSupport); } @Override @@ -46,13 +46,11 @@ public class WriteCandidateRunningTx extends WriteCandidateTx { } private void lockRunning() { - final String operation = "Lock Running"; try { - invokeBlocking(operation, new Function>() { + invokeBlocking("Lock running", new Function>() { @Override public ListenableFuture apply(final NetconfBaseOps input) { - return perfomRequestWithTimeout(operation, input.lockRunning(new NetconfRpcFutureCallback(operation, id))); - + return input.lockRunning(new NetconfRpcFutureCallback("Lock running", id)); } }); } catch (final NetconfDocumentedException e) { diff --git a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/WriteCandidateTx.java b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/WriteCandidateTx.java index f607089dc0..b143a3bf8e 100644 --- a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/WriteCandidateTx.java +++ b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/WriteCandidateTx.java @@ -71,8 +71,8 @@ public class WriteCandidateTx extends AbstractWriteTx { } }; - public WriteCandidateTx(final RemoteDeviceId id, final NetconfBaseOps rpc, final boolean rollbackSupport, long requestTimeoutMillis) { - super(requestTimeoutMillis, rpc, id, rollbackSupport); + public WriteCandidateTx(final RemoteDeviceId id, final NetconfBaseOps rpc, final boolean rollbackSupport) { + super(rpc, id, rollbackSupport); } @Override @@ -95,12 +95,11 @@ public class WriteCandidateTx extends AbstractWriteTx { } private void lock() throws NetconfDocumentedException { - final String operation = "Lock candidate"; try { - invokeBlocking(operation, new Function>() { + invokeBlocking("Lock candidate", new Function>() { @Override public ListenableFuture apply(final NetconfBaseOps input) { - return perfomRequestWithTimeout(operation, input.lockCandidate(new NetconfRpcFutureCallback(operation, id))); + return input.lockCandidate(new NetconfRpcFutureCallback("Lock candidate", id)); } }); } catch (final NetconfDocumentedException e) { @@ -186,16 +185,14 @@ public class WriteCandidateTx extends AbstractWriteTx { @Override protected void editConfig(final DataContainerChild editStructure, final Optional defaultOperation) throws NetconfDocumentedException { - final String operation = "Edit candidate"; - invokeBlocking(operation, new Function>() { + invokeBlocking("Edit candidate", new Function>() { @Override public ListenableFuture apply(final NetconfBaseOps input) { - - return perfomRequestWithTimeout(operation, defaultOperation.isPresent() - ? input.editConfigCandidate(new NetconfRpcFutureCallback(operation, id), editStructure, defaultOperation.get(), - rollbackSupport) - : input.editConfigCandidate(new NetconfRpcFutureCallback(operation, id), editStructure, - rollbackSupport)); + return defaultOperation.isPresent() + ? input.editConfigCandidate(new NetconfRpcFutureCallback("Edit candidate", id), editStructure, defaultOperation.get(), + rollbackSupport) + : input.editConfigCandidate(new NetconfRpcFutureCallback("Edit candidate", id), editStructure, + rollbackSupport); } }); } diff --git a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/WriteRunningTx.java b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/WriteRunningTx.java index 1d0ff653ef..cf7594a7bc 100644 --- a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/WriteRunningTx.java +++ b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/WriteRunningTx.java @@ -51,8 +51,8 @@ public class WriteRunningTx extends AbstractWriteTx { private static final Logger LOG = LoggerFactory.getLogger(WriteRunningTx.class); public WriteRunningTx(final RemoteDeviceId id, final NetconfBaseOps netOps, - final boolean rollbackSupport, long requestTimeoutMillis) { - super(requestTimeoutMillis, netOps, id, rollbackSupport); + final boolean rollbackSupport) { + super(netOps, id, rollbackSupport); } @Override @@ -61,12 +61,11 @@ public class WriteRunningTx extends AbstractWriteTx { } private void lock() { - final String operation = "Lock running"; try { - invokeBlocking(operation, new Function>() { + invokeBlocking("Lock running", new Function>() { @Override public ListenableFuture apply(final NetconfBaseOps input) { - return perfomRequestWithTimeout(operation, input.lockRunning(new NetconfRpcFutureCallback(operation, id))); + return input.lockRunning(new NetconfRpcFutureCallback("Lock running", id)); } }); } catch (final NetconfDocumentedException e) { @@ -97,14 +96,14 @@ public class WriteRunningTx extends AbstractWriteTx { @Override public synchronized CheckedFuture submit() { - final ListenableFuture commitFutureAsVoid = Futures.transform(commit(), new Function, Void>() { + final ListenableFuture commmitFutureAsVoid = Futures.transform(commit(), new Function, Void>() { @Override public Void apply(final RpcResult input) { return null; } }); - return Futures.makeChecked(commitFutureAsVoid, new Function() { + return Futures.makeChecked(commmitFutureAsVoid, new Function() { @Override public TransactionCommitFailedException apply(final Exception input) { return new TransactionCommitFailedException("Submit of transaction " + getIdentifier() + " failed", input); @@ -120,26 +119,24 @@ public class WriteRunningTx extends AbstractWriteTx { @Override protected void editConfig(final DataContainerChild editStructure, final Optional defaultOperation) throws NetconfDocumentedException { - final String operation = "Edit running"; - invokeBlocking(operation, new Function>() { + invokeBlocking("Edit running", new Function>() { @Override public ListenableFuture apply(final NetconfBaseOps input) { - return perfomRequestWithTimeout(operation, defaultOperation.isPresent() - ? input.editConfigRunning(new NetconfRpcFutureCallback(operation, id), editStructure, defaultOperation.get(), - rollbackSupport) - : input.editConfigRunning(new NetconfRpcFutureCallback(operation, id), editStructure, - rollbackSupport)); + return defaultOperation.isPresent() + ? input.editConfigRunning(new NetconfRpcFutureCallback("Edit running", id), editStructure, defaultOperation.get(), + rollbackSupport) + : input.editConfigRunning(new NetconfRpcFutureCallback("Edit running", id), editStructure, + rollbackSupport); } }); } private void unlock() { - final String operation = "Unlocking running"; try { - invokeBlocking(operation, new Function>() { + invokeBlocking("Unlocking running", new Function>() { @Override public ListenableFuture apply(final NetconfBaseOps input) { - return perfomRequestWithTimeout(operation, input.unlockRunning(new NetconfRpcFutureCallback(operation, id))); + return input.unlockRunning(new NetconfRpcFutureCallback("Unlock running", id)); } }); } catch (final NetconfDocumentedException e) { diff --git a/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/KeepaliveSalFacadeTest.java b/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/KeepaliveSalFacadeTest.java index 2a6ba4301f..81e09708f3 100644 --- a/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/KeepaliveSalFacadeTest.java +++ b/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/KeepaliveSalFacadeTest.java @@ -106,7 +106,7 @@ public class KeepaliveSalFacadeTest { doReturn(Futures.immediateCheckedFuture(result)).when(deviceRpc).invokeRpc(any(SchemaPath.class), any(NormalizedNode.class)); final KeepaliveSalFacade keepaliveSalFacade = - new KeepaliveSalFacade(REMOTE_DEVICE_ID, underlyingSalFacade, executorServiceSpy, 1L); + new KeepaliveSalFacade(REMOTE_DEVICE_ID, underlyingSalFacade, executorServiceSpy, 1L, 1L); keepaliveSalFacade.setListener(listener); keepaliveSalFacade.onDeviceConnected(null, null, deviceRpc); @@ -133,7 +133,7 @@ public class KeepaliveSalFacadeTest { .when(deviceRpc).invokeRpc(any(SchemaPath.class), any(NormalizedNode.class)); final KeepaliveSalFacade keepaliveSalFacade = - new KeepaliveSalFacade(REMOTE_DEVICE_ID, underlyingSalFacade, executorServiceSpy, 1L); + new KeepaliveSalFacade(REMOTE_DEVICE_ID, underlyingSalFacade, executorServiceSpy, 1L, 1L); keepaliveSalFacade.setListener(listener); keepaliveSalFacade.onDeviceConnected(null, null, deviceRpc); @@ -190,7 +190,7 @@ public class KeepaliveSalFacadeTest { .when(deviceRpc).invokeRpc(any(SchemaPath.class), any(NormalizedNode.class)); final KeepaliveSalFacade keepaliveSalFacade = - new KeepaliveSalFacade(REMOTE_DEVICE_ID, underlyingSalFacade, executorServiceSpy, 100L); + new KeepaliveSalFacade(REMOTE_DEVICE_ID, underlyingSalFacade, executorServiceSpy, 100L, 1L); keepaliveSalFacade.setListener(listener); keepaliveSalFacade.onDeviceConnected(null, null, deviceRpc); diff --git a/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/NetconfDeviceWriteOnlyTxTest.java b/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/NetconfDeviceWriteOnlyTxTest.java index 82fe4b23e7..ecb0eeb779 100644 --- a/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/NetconfDeviceWriteOnlyTxTest.java +++ b/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/NetconfDeviceWriteOnlyTxTest.java @@ -73,7 +73,7 @@ public class NetconfDeviceWriteOnlyTxTest { @Test public void testIgnoreNonVisibleData() { final WriteCandidateTx tx = new WriteCandidateTx(id, new NetconfBaseOps(rpc, mock(SchemaContext.class)), - false, 60000L); + false); final MapNode emptyList = ImmutableNodes.mapNodeBuilder(NETCONF_FILTER_QNAME).build(); tx.merge(LogicalDatastoreType.CONFIGURATION, YangInstanceIdentifier.create(new YangInstanceIdentifier.NodeIdentifier(NETCONF_FILTER_QNAME)), emptyList); tx.put(LogicalDatastoreType.CONFIGURATION, YangInstanceIdentifier.create(new YangInstanceIdentifier.NodeIdentifier(NETCONF_FILTER_QNAME)), emptyList); @@ -84,7 +84,7 @@ public class NetconfDeviceWriteOnlyTxTest { @Test public void testDiscardChanges() { final WriteCandidateTx tx = new WriteCandidateTx(id, new NetconfBaseOps(rpc, mock(SchemaContext.class)), - false, 60000L); + false); final CheckedFuture submitFuture = tx.submit(); try { submitFuture.checkedGet(); @@ -110,7 +110,7 @@ public class NetconfDeviceWriteOnlyTxTest { .doReturn(rpcErrorFuture).when(rpc).invokeRpc(any(SchemaPath.class), any(NormalizedNode.class)); final WriteCandidateTx tx = new WriteCandidateTx(id, new NetconfBaseOps(rpc, mock(SchemaContext.class)), - false, 60000L); + false); final CheckedFuture submitFuture = tx.submit(); try { @@ -129,7 +129,7 @@ public class NetconfDeviceWriteOnlyTxTest { .when(rpc).invokeRpc(any(SchemaPath.class), any(NormalizedNode.class)); final WriteRunningTx tx = new WriteRunningTx(id, new NetconfBaseOps(rpc, NetconfMessageTransformer.BaseSchema.BASE_NETCONF_CTX_WITH_NOTIFICATIONS.getSchemaContext()), - false, 60000L); + false); try { tx.delete(LogicalDatastoreType.CONFIGURATION, yangIId); } catch (final Exception e) { diff --git a/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/ReadOnlyTxTest.java b/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/ReadOnlyTxTest.java index c005b5bf23..2ea94f47a2 100644 --- a/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/ReadOnlyTxTest.java +++ b/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/ReadOnlyTxTest.java @@ -13,27 +13,15 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import com.google.common.base.Optional; -import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; -import com.google.common.util.concurrent.ListenableFuture; - import java.net.InetSocketAddress; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; - -import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.common.impl.util.compat.DataNormalizationException; -import org.opendaylight.controller.md.sal.dom.api.DOMRpcResult; import org.opendaylight.controller.md.sal.dom.api.DOMRpcService; import org.opendaylight.controller.md.sal.dom.spi.DefaultDOMRpcResult; import org.opendaylight.netconf.sal.connect.netconf.util.NetconfBaseOps; @@ -43,12 +31,7 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.model.api.SchemaContext; import org.opendaylight.yangtools.yang.model.api.SchemaPath; -import org.powermock.api.mockito.PowerMockito; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; -@PrepareForTest({NetconfBaseOps.class}) -@RunWith(PowerMockRunner.class) public class ReadOnlyTxTest { private static final YangInstanceIdentifier path = YangInstanceIdentifier.create(); @@ -70,33 +53,11 @@ public class ReadOnlyTxTest { public void testRead() throws Exception { final NetconfBaseOps netconfOps = new NetconfBaseOps(rpc, mock(SchemaContext.class)); - final ReadOnlyTx readOnlyTx = new ReadOnlyTx(netconfOps, new RemoteDeviceId("a", new InetSocketAddress("localhost", 196)), 60000L); + final ReadOnlyTx readOnlyTx = new ReadOnlyTx(netconfOps, new RemoteDeviceId("a", new InetSocketAddress("localhost", 196))); readOnlyTx.read(LogicalDatastoreType.CONFIGURATION, YangInstanceIdentifier.create()); verify(rpc).invokeRpc(Mockito.eq(NetconfMessageTransformUtil.toPath(NetconfMessageTransformUtil.NETCONF_GET_CONFIG_QNAME)), any(NormalizedNode.class)); readOnlyTx.read(LogicalDatastoreType.OPERATIONAL, path); verify(rpc).invokeRpc(Mockito.eq(NetconfMessageTransformUtil.toPath(NetconfMessageTransformUtil.NETCONF_GET_QNAME)), any(NormalizedNode.class)); } - - @SuppressWarnings("unchecked") - @Test - public void testReadTimeout() throws Exception { - final ListenableFuture future = mock(ListenableFuture.class); - - Mockito.when(future.get(Mockito.anyLong(), any(TimeUnit.class))).then(new Answer() { - @Override - public DOMRpcResult answer(InvocationOnMock invocation) - throws Throwable { - throw new TimeoutException("Processing Timeout"); - } - }); - - final NetconfBaseOps netconfOps = PowerMockito.mock(NetconfBaseOps.class); - Mockito.when(netconfOps.getConfigRunning(any(FutureCallback.class), any(Optional.class))).thenReturn(future); - - - final ReadOnlyTx readOnlyTx = new ReadOnlyTx(netconfOps, new RemoteDeviceId("a", new InetSocketAddress("localhost", 196)), 100L); - Assert.assertNull("Read operation didn't correctly timeout", readOnlyTx.read(LogicalDatastoreType.CONFIGURATION, YangInstanceIdentifier.create())); - readOnlyTx.close(); - } } \ No newline at end of file