BUG-5280: fix problems identified by integration tests 06/48706/17
authorRobert Varga <rovarga@cisco.com>
Fri, 25 Nov 2016 15:16:41 +0000 (16:16 +0100)
committerRobert Varga <rovarga@cisco.com>
Wed, 30 Nov 2016 11:46:33 +0000 (12:46 +0100)
Switching the integration test suite has flushed out couple
of problems in the implementation, notably:

- wrong formatting placeholder
- unhandled requests during replay
- uninitialized path in AbstractReadTransactionRequestProxyV1
- missing sequence number bump in local commit case
- wrong writeObject() in ReadTransactionSuccessProxyV1
- IllegalStateException thrown instead of TransactionChainClosedException
- attempt to create history=0 on the backend
- mismatched sequences during preCommit message replay
- ConcurrentModificationException during localAbort()
- missing upcalls to LocalHistory concretizations when transactions abort
  and complete
- incorrect order on enqueue/send, leading to unpaired responses

Change-Id: I252a795dadb917452b9eb6d591a5c12ca5b69a45
Signed-off-by: Robert Varga <rovarga@cisco.com>
16 files changed:
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequestProxyV1.java
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/CommitLocalTransactionRequest.java
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionSuccessProxyV1.java
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/concepts/TransactionIdentifier.java
opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ClientActorBehavior.java
opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ConnectedClientConnection.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHistory.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractProxyTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientLocalHistory.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalProxyTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ProxyHistory.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/RemoteProxyTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java
opendaylight/md-sal/sal-distributed-datastore/src/test/resources/simplelogger.properties

index b73928574f53d9a9864ed81104fa28dd4436d1d5..d3a60af6359c71769d1b4ad0eae0dc06f461f3c2 100644 (file)
@@ -35,6 +35,7 @@ abstract class AbstractReadTransactionRequestProxyV1<T extends AbstractReadTrans
 
     AbstractReadTransactionRequestProxyV1(final T request) {
         super(request);
 
     AbstractReadTransactionRequestProxyV1(final T request) {
         super(request);
+        path = request.getPath();
     }
 
     @Override
     }
 
     @Override
index 495111096ca6cffbf8f017c75305b1003032ffd6..48f5cc58718f9cd4f4693f5dc5acd71d93b86a0f 100644 (file)
@@ -33,9 +33,9 @@ public final class CommitLocalTransactionRequest
     private final DataTreeModification mod;
     private final boolean coordinated;
 
     private final DataTreeModification mod;
     private final boolean coordinated;
 
-    public CommitLocalTransactionRequest(@Nonnull final TransactionIdentifier identifier,
+    public CommitLocalTransactionRequest(@Nonnull final TransactionIdentifier identifier, final long sequence,
             @Nonnull final ActorRef replyTo, @Nonnull final DataTreeModification mod, final boolean coordinated) {
             @Nonnull final ActorRef replyTo, @Nonnull final DataTreeModification mod, final boolean coordinated) {
-        super(identifier, 0, replyTo);
+        super(identifier, sequence, replyTo);
         this.mod = Preconditions.checkNotNull(mod);
         this.coordinated = coordinated;
     }
         this.mod = Preconditions.checkNotNull(mod);
         this.coordinated = coordinated;
     }
index 3637aa8ac98acb2d1ba6f536f3e88b24e9353196..dafc26b125471e026a755c0706763d14b10351e9 100644 (file)
@@ -50,8 +50,6 @@ final class ReadTransactionSuccessProxyV1 extends AbstractTransactionSuccessProx
         } else {
             out.writeBoolean(false);
         }
         } else {
             out.writeBoolean(false);
         }
-
-        out.writeObject(data);
     }
 
     @Override
     }
 
     @Override
index 405d1f9c8287829b877997e108ef2c5f4ee9ff74..5f96074fceec577f802595ab571f5390018fc38d 100644 (file)
@@ -112,7 +112,8 @@ public final class TransactionIdentifier implements WritableIdentifier {
             String histStr = historyId.getHistoryId() == 0 ? "" : "-chn-" + historyId.getHistoryId();
             shortString = historyId.getClientId().getFrontendId().getMemberName().getName() + "-"
                     + historyId.getClientId().getFrontendId().getClientType().getName() + "-fe-"
             String histStr = historyId.getHistoryId() == 0 ? "" : "-chn-" + historyId.getHistoryId();
             shortString = historyId.getClientId().getFrontendId().getMemberName().getName() + "-"
                     + historyId.getClientId().getFrontendId().getClientType().getName() + "-fe-"
-                    + historyId.getClientId().getGeneration() + histStr + "-txn-" + transactionId;
+                    + historyId.getClientId().getGeneration() + histStr + "-txn-" + transactionId
+                    + "-" + historyId.getCookie();
         }
 
         return shortString;
         }
 
         return shortString;
index 97d312ce394a186ec4d9b4b311676218d2f21bc3..ddb7bcdad112e84623c797f8790a968eac07791a 100644 (file)
@@ -202,6 +202,7 @@ public abstract class ClientActorBehavior<T extends BackendInfo> extends
             return;
         }
 
             return;
         }
 
+        LOG.debug("{}: resolved shard {} to {}", persistenceId(), shard, backend);
         final long stamp = connectionsLock.writeLock();
         try {
             // Bring the connection up
         final long stamp = connectionsLock.writeLock();
         try {
             // Bring the connection up
index 6c1507c50d7f89fc11da21a7ecb55b4ec4b5a0c9..eab11429b8a9e3403ac92adbdb1e1f737865e89f 100644 (file)
@@ -25,23 +25,28 @@ public final class ConnectedClientConnection<T extends BackendInfo> extends Abst
         super(context, cookie, backend);
     }
 
         super(context, cookie, backend);
     }
 
-    private TransmittedConnectionEntry transmit(final ConnectionEntry entry) {
+    private void transmit(final ConnectionEntry entry) {
         final long txSequence = nextTxSequence++;
 
         final RequestEnvelope toSend = new RequestEnvelope(entry.getRequest().toVersion(remoteVersion()), sessionId(),
             txSequence);
 
         final long txSequence = nextTxSequence++;
 
         final RequestEnvelope toSend = new RequestEnvelope(entry.getRequest().toVersion(remoteVersion()), sessionId(),
             txSequence);
 
+        // We need to enqueue the request before we send it to the actor, as we may be executing on a different thread
+        // than the client actor thread, in which case the round-trip could be made faster than we can enqueue --
+        // in which case the receive routine would not find the entry.
+        final TransmittedConnectionEntry txEntry = new TransmittedConnectionEntry(entry, sessionId(), txSequence,
+            readTime());
+        appendToInflight(txEntry);
+
         final ActorRef actor = remoteActor();
         LOG.trace("Transmitting request {} as {} to {}", entry.getRequest(), toSend, actor);
         actor.tell(toSend, ActorRef.noSender());
         final ActorRef actor = remoteActor();
         LOG.trace("Transmitting request {} as {} to {}", entry.getRequest(), toSend, actor);
         actor.tell(toSend, ActorRef.noSender());
-
-        return new TransmittedConnectionEntry(entry, sessionId(), txSequence, readTime());
     }
 
     @Override
     void enqueueEntry(final ConnectionEntry entry) {
         if (inflightSize() < remoteMaxMessages()) {
     }
 
     @Override
     void enqueueEntry(final ConnectionEntry entry) {
         if (inflightSize() < remoteMaxMessages()) {
-            appendToInflight(transmit(entry));
+            transmit(entry);
             LOG.debug("Enqueued request {} to queue {}", entry.getRequest(), this);
         } else {
             LOG.debug("Queue is at capacity, delayed sending of request {}", entry.getRequest());
             LOG.debug("Enqueued request {} to queue {}", entry.getRequest(), this);
         } else {
             LOG.debug("Queue is at capacity, delayed sending of request {}", entry.getRequest());
@@ -60,7 +65,7 @@ public final class ConnectedClientConnection<T extends BackendInfo> extends Abst
             }
 
             LOG.debug("Transmitting entry {}", e);
             }
 
             LOG.debug("Transmitting entry {}", e);
-            appendToInflight(transmit(e));
+            transmit(e);
             toSend--;
         }
     }
             toSend--;
         }
     }
index 951b540f1de3f4550ba8efaac4f8a207e479caea..8ab58e410aa419a3f0711188f6b5af29fe2177a9 100644 (file)
@@ -22,6 +22,7 @@ import org.opendaylight.controller.cluster.access.commands.CreateLocalHistoryReq
 import org.opendaylight.controller.cluster.access.concepts.LocalHistoryIdentifier;
 import org.opendaylight.controller.cluster.access.concepts.Response;
 import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier;
 import org.opendaylight.controller.cluster.access.concepts.LocalHistoryIdentifier;
 import org.opendaylight.controller.cluster.access.concepts.Response;
 import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier;
+import org.opendaylight.mdsal.common.api.TransactionChainClosedException;
 import org.opendaylight.yangtools.concepts.Identifiable;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.slf4j.Logger;
 import org.opendaylight.yangtools.concepts.Identifiable;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.slf4j.Logger;
@@ -113,12 +114,17 @@ abstract class AbstractClientHistory extends LocalAbortable implements Identifia
      */
     private ProxyHistory createHistoryProxy(final Long shard) {
         final AbstractClientConnection<ShardBackendInfo> connection = client.getConnection(shard);
      */
     private ProxyHistory createHistoryProxy(final Long shard) {
         final AbstractClientConnection<ShardBackendInfo> connection = client.getConnection(shard);
-        final ProxyHistory ret = createHistoryProxy(new LocalHistoryIdentifier(identifier.getClientId(),
-            identifier.getHistoryId(), shard), connection);
+        final LocalHistoryIdentifier proxyId = new LocalHistoryIdentifier(identifier.getClientId(),
+            identifier.getHistoryId(), shard);
+        LOG.debug("Created proxyId {} for history {} shard {}", proxyId, identifier, shard);
 
 
-        // Request creation of the history.
-        connection.sendRequest(new CreateLocalHistoryRequest(ret.getIdentifier(), connection.localActor()),
-            this::createHistoryCallback);
+        final ProxyHistory ret = createHistoryProxy(proxyId, connection);
+
+        // Request creation of the history, if it is not the single history
+        if (ret.getIdentifier().getHistoryId() != 0) {
+            connection.sendRequest(new CreateLocalHistoryRequest(ret.getIdentifier(), connection.localActor()),
+                this::createHistoryCallback);
+        }
         return ret;
     }
 
         return ret;
     }
 
@@ -145,8 +151,16 @@ abstract class AbstractClientHistory extends LocalAbortable implements Identifia
         }
     }
 
         }
     }
 
+    /**
+     * Allocate a {@link ClientTransaction}.
+     *
+     * @return A new {@link ClientTransaction}
+     * @throws TransactionChainClosedException if this history is closed
+     */
     public final ClientTransaction createTransaction() {
     public final ClientTransaction createTransaction() {
-        Preconditions.checkState(state != State.CLOSED);
+        if (state == State.CLOSED) {
+            throw new TransactionChainClosedException(String.format("Local history %s is closed", identifier));
+        }
 
         synchronized (this) {
             final ClientTransaction ret = doCreateTransaction();
 
         synchronized (this) {
             final ClientTransaction ret = doCreateTransaction();
index adfc0df876cb6180961ee535519dd44aa1e946ee..803908d8c603848683d55d74d897c65bd9cd9931 100644 (file)
@@ -14,7 +14,10 @@ import com.google.common.base.Verify;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
+import java.util.ArrayDeque;
+import java.util.Deque;
 import java.util.function.Consumer;
 import java.util.function.Consumer;
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import org.opendaylight.controller.cluster.access.commands.TransactionAbortRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionAbortSuccess;
 import javax.annotation.Nullable;
 import org.opendaylight.controller.cluster.access.commands.TransactionAbortRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionAbortSuccess;
@@ -49,8 +52,21 @@ import org.slf4j.LoggerFactory;
  * @author Robert Varga
  */
 abstract class AbstractProxyTransaction implements Identifiable<TransactionIdentifier> {
  * @author Robert Varga
  */
 abstract class AbstractProxyTransaction implements Identifiable<TransactionIdentifier> {
+    private static final class IncrementSequence {
+        private long delta = 1;
+
+        long getDelta() {
+            return delta;
+        }
+
+        void incrementDelta() {
+            delta++;
+        }
+    }
+
     private static final Logger LOG = LoggerFactory.getLogger(AbstractProxyTransaction.class);
 
     private static final Logger LOG = LoggerFactory.getLogger(AbstractProxyTransaction.class);
 
+    private final Deque<Object> successfulRequests = new ArrayDeque<>();
     private final ProxyHistory parent;
 
     private AbstractProxyTransaction successor;
     private final ProxyHistory parent;
 
     private AbstractProxyTransaction successor;
@@ -65,8 +81,15 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
         return parent.localActor();
     }
 
         return parent.localActor();
     }
 
+    private void incrementSequence(final long delta) {
+        sequence += delta;
+        LOG.debug("Transaction {} incremented sequence to {}", this, sequence);
+    }
+
     final long nextSequence() {
     final long nextSequence() {
-        return sequence++;
+        final long ret = sequence++;
+        LOG.debug("Transaction {} allocated sequence {}", this, ret);
+        return ret;
     }
 
     final void delete(final YangInstanceIdentifier path) {
     }
 
     final void delete(final YangInstanceIdentifier path) {
@@ -117,6 +140,19 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
         Preconditions.checkState(sealed, "Transaction %s has not been sealed yet", getIdentifier());
     }
 
         Preconditions.checkState(sealed, "Transaction %s has not been sealed yet", getIdentifier());
     }
 
+    final void recordSuccessfulRequest(final @Nonnull TransactionRequest<?> req) {
+        successfulRequests.add(Verify.verifyNotNull(req));
+    }
+
+    final void recordFinishedRequest() {
+        final Object last = successfulRequests.peekLast();
+        if (last instanceof IncrementSequence) {
+            ((IncrementSequence) last).incrementDelta();
+        } else {
+            successfulRequests.addLast(new IncrementSequence());
+        }
+    }
+
     /**
      * Abort this transaction. This is invoked only for read-only transactions and will result in an explicit message
      * being sent to the backend.
     /**
      * Abort this transaction. This is invoked only for read-only transactions and will result in an explicit message
      * being sent to the backend.
@@ -139,6 +175,8 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
                 ret.voteNo(new IllegalStateException("Unhandled response " + t.getClass()));
             }
 
                 ret.voteNo(new IllegalStateException("Unhandled response " + t.getClass()));
             }
 
+            // This is a terminal request, hence we do not need to record it
+            LOG.debug("Transaction {} abort completed", this);
             parent.completeTransaction(this);
         });
     }
             parent.completeTransaction(this);
         });
     }
@@ -166,6 +204,8 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
                 ret.setException(new IllegalStateException("Unhandled response " + t.getClass()));
             }
 
                 ret.setException(new IllegalStateException("Unhandled response " + t.getClass()));
             }
 
+            // This is a terminal request, hence we do not need to record it
+            LOG.debug("Transaction {} directCommit completed", this);
             parent.completeTransaction(this);
         });
         return ret;
             parent.completeTransaction(this);
         });
         return ret;
@@ -175,7 +215,8 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
     void canCommit(final VotingFuture<?> ret) {
         checkSealed();
 
     void canCommit(final VotingFuture<?> ret) {
         checkSealed();
 
-        sendRequest(Verify.verifyNotNull(commitRequest(true)), t -> {
+        final TransactionRequest<?> req = Verify.verifyNotNull(commitRequest(true));
+        sendRequest(req, t -> {
             if (t instanceof TransactionCanCommitSuccess) {
                 ret.voteYes();
             } else if (t instanceof RequestFailure) {
             if (t instanceof TransactionCanCommitSuccess) {
                 ret.voteYes();
             } else if (t instanceof RequestFailure) {
@@ -183,13 +224,18 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
             } else {
                 ret.voteNo(new IllegalStateException("Unhandled response " + t.getClass()));
             }
             } else {
                 ret.voteNo(new IllegalStateException("Unhandled response " + t.getClass()));
             }
+
+            recordSuccessfulRequest(req);
+            LOG.debug("Transaction {} canCommit completed", this);
         });
     }
 
     void preCommit(final VotingFuture<?> ret) {
         checkSealed();
 
         });
     }
 
     void preCommit(final VotingFuture<?> ret) {
         checkSealed();
 
-        sendRequest(new TransactionPreCommitRequest(getIdentifier(), nextSequence(), localActor()), t -> {
+        final TransactionRequest<?> req = new TransactionPreCommitRequest(getIdentifier(), nextSequence(),
+            localActor());
+        sendRequest(req, t -> {
             if (t instanceof TransactionPreCommitSuccess) {
                 ret.voteYes();
             } else if (t instanceof RequestFailure) {
             if (t instanceof TransactionPreCommitSuccess) {
                 ret.voteYes();
             } else if (t instanceof RequestFailure) {
@@ -197,6 +243,9 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
             } else {
                 ret.voteNo(new IllegalStateException("Unhandled response " + t.getClass()));
             }
             } else {
                 ret.voteNo(new IllegalStateException("Unhandled response " + t.getClass()));
             }
+
+            recordSuccessfulRequest(req);
+            LOG.debug("Transaction {} preCommit completed", this);
         });
     }
 
         });
     }
 
@@ -212,12 +261,25 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
                 ret.voteNo(new IllegalStateException("Unhandled response " + t.getClass()));
             }
 
                 ret.voteNo(new IllegalStateException("Unhandled response " + t.getClass()));
             }
 
+            LOG.debug("Transaction {} doCommit completed", this);
             parent.completeTransaction(this);
         });
     }
 
             parent.completeTransaction(this);
         });
     }
 
-    void replaySuccessfulRequests(final AbstractProxyTransaction successor) {
+    final void replaySuccessfulRequests(final AbstractProxyTransaction successor) {
         this.successor = Preconditions.checkNotNull(successor);
         this.successor = Preconditions.checkNotNull(successor);
+
+        for (Object obj : successfulRequests) {
+            if (obj instanceof TransactionRequest) {
+                LOG.debug("Forwarding request {} to successor {}", obj, successor);
+                successor.handleForwardedRemoteRequest((TransactionRequest<?>) obj, null);
+            } else {
+                Verify.verify(obj instanceof IncrementSequence);
+                successor.incrementSequence(((IncrementSequence) obj).getDelta());
+            }
+        }
+        LOG.debug("{} replayed {} successful requests", getIdentifier(), successfulRequests.size());
+        successfulRequests.clear();
     }
 
     /**
     }
 
     /**
index b6c274628c752f19aa3728bb6e77996051e12240..ac1872835ac37a612acdd0e9401c57c17f789c6a 100644 (file)
@@ -48,6 +48,16 @@ public final class ClientLocalHistory extends AbstractClientHistory implements A
         return new ClientTransaction(this, new TransactionIdentifier(getIdentifier(), nextTx()));
     }
 
         return new ClientTransaction(this, new TransactionIdentifier(getIdentifier(), nextTx()));
     }
 
+    @Override
+    void onTransactionAbort(final TransactionIdentifier txId) {
+        final State local = state();
+        if (local == State.TX_OPEN) {
+            updateState(local, State.IDLE);
+        }
+
+        super.onTransactionAbort(txId);
+    }
+
     @Override
     AbstractTransactionCommitCohort onTransactionReady(final TransactionIdentifier txId,
             final AbstractTransactionCommitCohort cohort) {
     @Override
     AbstractTransactionCommitCohort onTransactionReady(final TransactionIdentifier txId,
             final AbstractTransactionCommitCohort cohort) {
index 8450c67224fb7625bffbf64d3d4741737d3049db..abb134526954f90a63c7f394ae6a91bba887c5cb 100644 (file)
@@ -152,20 +152,27 @@ public final class ClientTransaction extends LocalAbortable implements Identifia
      * Release all state associated with this transaction.
      */
     public void abort() {
      * Release all state associated with this transaction.
      */
     public void abort() {
-        if (ensureClosed()) {
-            for (AbstractProxyTransaction proxy : proxies.values()) {
-                proxy.abort();
-            }
-            proxies.clear();
-
+        if (commonAbort()) {
             parent.onTransactionAbort(transactionId);
         }
     }
 
             parent.onTransactionAbort(transactionId);
         }
     }
 
+    private boolean commonAbort() {
+        if (!ensureClosed()) {
+            return false;
+        }
+
+        for (AbstractProxyTransaction proxy : proxies.values()) {
+            proxy.abort();
+        }
+        proxies.clear();
+        return true;
+    }
+
     @Override
     void localAbort(final Throwable cause) {
     @Override
     void localAbort(final Throwable cause) {
-        LOG.debug("Aborting transaction {}", getIdentifier(), cause);
-        abort();
+        LOG.debug("Local abort of transaction {}", getIdentifier(), cause);
+        commonAbort();
     }
 
     Map<Long, AbstractProxyTransaction> getProxies() {
     }
 
     Map<Long, AbstractProxyTransaction> getProxies() {
index 3869e66d3e90141b9390491de9c5c20afbd60a91..61f83c2db1e8172c9374665bfe41eba3e2e13d3d 100644 (file)
@@ -17,11 +17,18 @@ import javax.annotation.Nullable;
 import javax.annotation.concurrent.NotThreadSafe;
 import org.opendaylight.controller.cluster.access.commands.AbortLocalTransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.CommitLocalTransactionRequest;
 import javax.annotation.concurrent.NotThreadSafe;
 import org.opendaylight.controller.cluster.access.commands.AbortLocalTransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.CommitLocalTransactionRequest;
+import org.opendaylight.controller.cluster.access.commands.ExistsTransactionRequest;
+import org.opendaylight.controller.cluster.access.commands.ExistsTransactionSuccess;
 import org.opendaylight.controller.cluster.access.commands.ModifyTransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.PersistenceProtocol;
 import org.opendaylight.controller.cluster.access.commands.ModifyTransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.PersistenceProtocol;
+import org.opendaylight.controller.cluster.access.commands.ReadTransactionRequest;
+import org.opendaylight.controller.cluster.access.commands.ReadTransactionSuccess;
+import org.opendaylight.controller.cluster.access.commands.TransactionAbortRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionDelete;
 import org.opendaylight.controller.cluster.access.commands.TransactionDelete;
+import org.opendaylight.controller.cluster.access.commands.TransactionDoCommitRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionMerge;
 import org.opendaylight.controller.cluster.access.commands.TransactionModification;
 import org.opendaylight.controller.cluster.access.commands.TransactionMerge;
 import org.opendaylight.controller.cluster.access.commands.TransactionModification;
+import org.opendaylight.controller.cluster.access.commands.TransactionPreCommitRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionWrite;
 import org.opendaylight.controller.cluster.access.concepts.RequestException;
 import org.opendaylight.controller.cluster.access.commands.TransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionWrite;
 import org.opendaylight.controller.cluster.access.concepts.RequestException;
@@ -118,8 +125,8 @@ final class LocalProxyTransaction extends AbstractProxyTransaction {
 
     @Override
     CommitLocalTransactionRequest commitRequest(final boolean coordinated) {
 
     @Override
     CommitLocalTransactionRequest commitRequest(final boolean coordinated) {
-        final CommitLocalTransactionRequest ret = new CommitLocalTransactionRequest(identifier, localActor(),
-            modification, coordinated);
+        final CommitLocalTransactionRequest ret = new CommitLocalTransactionRequest(identifier, nextSequence(),
+            localActor(), modification, coordinated);
         modification = new FailedDataTreeModification(this::submittedException);
         return ret;
     }
         modification = new FailedDataTreeModification(this::submittedException);
         return ret;
     }
@@ -173,10 +180,24 @@ final class LocalProxyTransaction extends AbstractProxyTransaction {
     @Override
     void handleForwardedRemoteRequest(final TransactionRequest<?> request,
             final @Nullable Consumer<Response<?, ?>> callback) {
     @Override
     void handleForwardedRemoteRequest(final TransactionRequest<?> request,
             final @Nullable Consumer<Response<?, ?>> callback) {
-        LOG.debug("Applying forwaded request {}", request);
+        LOG.debug("Applying forwarded request {}", request);
 
         if (request instanceof ModifyTransactionRequest) {
             applyModifyTransactionRequest((ModifyTransactionRequest) request, callback);
 
         if (request instanceof ModifyTransactionRequest) {
             applyModifyTransactionRequest((ModifyTransactionRequest) request, callback);
+        } else if (request instanceof ReadTransactionRequest) {
+            final YangInstanceIdentifier path = ((ReadTransactionRequest) request).getPath();
+            final Optional<NormalizedNode<?, ?>> result = modification.readNode(path);
+            callback.accept(new ReadTransactionSuccess(request.getTarget(), request.getSequence(), result));
+        } else if (request instanceof ExistsTransactionRequest) {
+            final YangInstanceIdentifier path = ((ExistsTransactionRequest) request).getPath();
+            final boolean result = modification.readNode(path).isPresent();
+            callback.accept(new ExistsTransactionSuccess(request.getTarget(), request.getSequence(), result));
+        } else if (request instanceof TransactionPreCommitRequest) {
+            sendRequest(new TransactionPreCommitRequest(getIdentifier(), nextSequence(), localActor()), callback);
+        } else if (request instanceof TransactionDoCommitRequest) {
+            sendRequest(new TransactionDoCommitRequest(getIdentifier(), nextSequence(), localActor()), callback);
+        } else if (request instanceof TransactionAbortRequest) {
+            sendAbort(callback);
         } else {
             throw new IllegalArgumentException("Unhandled request " + request);
         }
         } else {
             throw new IllegalArgumentException("Unhandled request " + request);
         }
index ae5537915556cfa9c3018ef6b27f072c7d2e2003..5257c6c7f6299bcfd85e08f67711256dbe906c63 100644 (file)
@@ -87,7 +87,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
         @Override
         AbstractProxyTransaction doCreateTransactionProxy(final AbstractClientConnection<ShardBackendInfo> connection,
                 final TransactionIdentifier txId) {
         @Override
         AbstractProxyTransaction doCreateTransactionProxy(final AbstractClientConnection<ShardBackendInfo> connection,
                 final TransactionIdentifier txId) {
-            Preconditions.checkState(lastOpen == null, "Proxy {} is currently open", lastOpen);
+            Preconditions.checkState(lastOpen == null, "Proxy %s has %s currently open", this, lastOpen);
 
             // onTransactionCompleted() runs concurrently
             final LocalProxyTransaction localSealed = lastSealed;
 
             // onTransactionCompleted() runs concurrently
             final LocalProxyTransaction localSealed = lastSealed;
@@ -100,6 +100,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
 
             lastOpen = new LocalProxyTransaction(this, txId,
                 (CursorAwareDataTreeModification) baseSnapshot.newModification());
 
             lastOpen = new LocalProxyTransaction(this, txId,
                 (CursorAwareDataTreeModification) baseSnapshot.newModification());
+            LOG.debug("Proxy {} open transaction {}", this, lastOpen);
             return lastOpen;
         }
 
             return lastOpen;
         }
 
@@ -196,8 +197,9 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
         @Override
         void replaySuccessfulRequests() {
             for (AbstractProxyTransaction t : proxies.values()) {
         @Override
         void replaySuccessfulRequests() {
             for (AbstractProxyTransaction t : proxies.values()) {
+                LOG.debug("{} creating successor transaction proxy for {}", identifier, t);
                 final AbstractProxyTransaction newProxy = successor.createTransactionProxy(t.getIdentifier());
                 final AbstractProxyTransaction newProxy = successor.createTransactionProxy(t.getIdentifier());
-                LOG.debug("{} created successor transaction proxy {} for {}", identifier, newProxy, t);
+                LOG.debug("{} created successor transaction proxy {}", identifier, newProxy);
                 t.replaySuccessfulRequests(newProxy);
             }
         }
                 t.replaySuccessfulRequests(newProxy);
             }
         }
@@ -282,10 +284,13 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
     }
 
     final AbstractProxyTransaction createTransactionProxy(final TransactionIdentifier txId) {
     }
 
     final AbstractProxyTransaction createTransactionProxy(final TransactionIdentifier txId) {
-        final TransactionIdentifier proxyId = new TransactionIdentifier(identifier, txId.getTransactionId());
-
         lock.lock();
         try {
         lock.lock();
         try {
+            if (successor != null) {
+                return successor.createTransactionProxy(txId);
+            }
+
+            final TransactionIdentifier proxyId = new TransactionIdentifier(identifier, txId.getTransactionId());
             final AbstractProxyTransaction ret = doCreateTransactionProxy(connection, proxyId);
             proxies.put(proxyId, ret);
             LOG.debug("Allocated proxy {} for transaction {}", proxyId, txId);
             final AbstractProxyTransaction ret = doCreateTransactionProxy(connection, proxyId);
             proxies.put(proxyId, ret);
             LOG.debug("Allocated proxy {} for transaction {}", proxyId, txId);
@@ -299,6 +304,8 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
         lock.lock();
         try {
             proxies.remove(tx.getIdentifier());
         lock.lock();
         try {
             proxies.remove(tx.getIdentifier());
+            LOG.debug("Proxy {} aborting transaction {}", this, tx);
+            onTransactionAborted(tx);
         } finally {
             lock.unlock();
         }
         } finally {
             lock.unlock();
         }
@@ -308,6 +315,8 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
         lock.lock();
         try {
             proxies.remove(tx.getIdentifier());
         lock.lock();
         try {
             proxies.remove(tx.getIdentifier());
+            LOG.debug("Proxy {} completing transaction {}", this, tx);
+            onTransactionCompleted(tx);
         } finally {
             lock.unlock();
         }
         } finally {
             lock.unlock();
         }
@@ -332,6 +341,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
         }
 
         successor = createSuccessor(newConnection);
         }
 
         successor = createSuccessor(newConnection);
+        LOG.debug("History {} instantiated successor {}", this, successor);
         return new ReconnectCohort();
     }
 
         return new ReconnectCohort();
     }
 
index 347c7eac033ae1bf4a63db2f3d4ba0d26b981d6c..175483855f14009a0f86ecc7107c07870bbaa76c 100644 (file)
@@ -13,8 +13,6 @@ import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
-import java.util.ArrayList;
-import java.util.Collection;
 import java.util.function.Consumer;
 import javax.annotation.Nullable;
 import org.opendaylight.controller.cluster.access.commands.AbstractReadTransactionRequest;
 import java.util.function.Consumer;
 import javax.annotation.Nullable;
 import org.opendaylight.controller.cluster.access.commands.AbstractReadTransactionRequest;
@@ -25,9 +23,12 @@ import org.opendaylight.controller.cluster.access.commands.ModifyTransactionRequ
 import org.opendaylight.controller.cluster.access.commands.PersistenceProtocol;
 import org.opendaylight.controller.cluster.access.commands.ReadTransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.ReadTransactionSuccess;
 import org.opendaylight.controller.cluster.access.commands.PersistenceProtocol;
 import org.opendaylight.controller.cluster.access.commands.ReadTransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.ReadTransactionSuccess;
+import org.opendaylight.controller.cluster.access.commands.TransactionAbortRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionDelete;
 import org.opendaylight.controller.cluster.access.commands.TransactionDelete;
+import org.opendaylight.controller.cluster.access.commands.TransactionDoCommitRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionMerge;
 import org.opendaylight.controller.cluster.access.commands.TransactionModification;
 import org.opendaylight.controller.cluster.access.commands.TransactionMerge;
 import org.opendaylight.controller.cluster.access.commands.TransactionModification;
+import org.opendaylight.controller.cluster.access.commands.TransactionPreCommitRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionSuccess;
 import org.opendaylight.controller.cluster.access.commands.TransactionWrite;
 import org.opendaylight.controller.cluster.access.commands.TransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionSuccess;
 import org.opendaylight.controller.cluster.access.commands.TransactionWrite;
@@ -62,7 +63,6 @@ final class RemoteProxyTransaction extends AbstractProxyTransaction {
     // FIXME: make this tuneable
     private static final int REQUEST_MAX_MODIFICATIONS = 1000;
 
     // FIXME: make this tuneable
     private static final int REQUEST_MAX_MODIFICATIONS = 1000;
 
-    private final Collection<TransactionRequest<?>> successfulRequests = new ArrayList<>();
     private final ModifyTransactionRequestBuilder builder;
 
     private boolean builderBusy;
     private final ModifyTransactionRequestBuilder builder;
 
     private boolean builderBusy;
@@ -195,7 +195,7 @@ final class RemoteProxyTransaction extends AbstractProxyTransaction {
 
         if (response instanceof TransactionSuccess) {
             // Happy path
 
         if (response instanceof TransactionSuccess) {
             // Happy path
-            successfulRequests.add(request);
+            recordSuccessfulRequest(request);
         } else {
             recordFailedResponse(response);
         }
         } else {
             recordFailedResponse(response);
         }
@@ -229,6 +229,8 @@ final class RemoteProxyTransaction extends AbstractProxyTransaction {
         } else {
             failFuture(future, response);
         }
         } else {
             failFuture(future, response);
         }
+
+        recordFinishedRequest();
     }
 
     private void completeRead(final SettableFuture<Optional<NormalizedNode<?, ?>>> future,
     }
 
     private void completeRead(final SettableFuture<Optional<NormalizedNode<?, ?>>> future,
@@ -240,6 +242,8 @@ final class RemoteProxyTransaction extends AbstractProxyTransaction {
         } else {
             failFuture(future, response);
         }
         } else {
             failFuture(future, response);
         }
+
+        recordFinishedRequest();
     }
 
     @Override
     }
 
     @Override
@@ -257,17 +261,6 @@ final class RemoteProxyTransaction extends AbstractProxyTransaction {
         // No-op
     }
 
         // No-op
     }
 
-    @Override
-    void replaySuccessfulRequests(final AbstractProxyTransaction successor) {
-        super.replaySuccessfulRequests(successor);
-
-        for (TransactionRequest<?> req : successfulRequests) {
-            LOG.debug("Forwarding request {} to successor {}", req, successor);
-            successor.handleForwardedRemoteRequest(req, null);
-        }
-        successfulRequests.clear();
-    }
-
     @Override
     void forwardToRemote(final RemoteProxyTransaction successor, final TransactionRequest<?> request,
             final Consumer<Response<?, ?>> callback) throws RequestException {
     @Override
     void forwardToRemote(final RemoteProxyTransaction successor, final TransactionRequest<?> request,
             final Consumer<Response<?, ?>> callback) throws RequestException {
@@ -299,6 +292,23 @@ final class RemoteProxyTransaction extends AbstractProxyTransaction {
                         throw new IllegalArgumentException("Unhandled protocol " + maybeProto.get());
                 }
             }
                         throw new IllegalArgumentException("Unhandled protocol " + maybeProto.get());
                 }
             }
+        } else if (request instanceof ReadTransactionRequest) {
+            ensureFlushedBuider();
+            sendRequest(new ReadTransactionRequest(getIdentifier(), nextSequence(), localActor(),
+                ((ReadTransactionRequest) request).getPath()), callback);
+        } else if (request instanceof ExistsTransactionRequest) {
+            ensureFlushedBuider();
+            sendRequest(new ExistsTransactionRequest(getIdentifier(), nextSequence(), localActor(),
+                ((ExistsTransactionRequest) request).getPath()), callback);
+        } else if (request instanceof TransactionPreCommitRequest) {
+            ensureFlushedBuider();
+            sendRequest(new TransactionPreCommitRequest(getIdentifier(), nextSequence(), localActor()), callback);
+        } else if (request instanceof TransactionDoCommitRequest) {
+            ensureFlushedBuider();
+            sendRequest(new TransactionDoCommitRequest(getIdentifier(), nextSequence(), localActor()), callback);
+        } else if (request instanceof TransactionAbortRequest) {
+            ensureFlushedBuider();
+            sendAbort(callback);
         } else {
             throw new IllegalArgumentException("Unhandled request {}" + request);
         }
         } else {
             throw new IllegalArgumentException("Unhandled request {}" + request);
         }
index 7ddad749d26552676a8057b40ca7526677bac130..1adca56af2680a32a5c4f68afe39bc1d0f79533d 100644 (file)
@@ -77,9 +77,11 @@ abstract class AbstractFrontendHistory implements Identifiable<LocalHistoryIdent
 
             transactions.put(id, tx);
         } else {
 
             transactions.put(id, tx);
         } else {
-            final Optional<TransactionSuccess<?>> replay = tx.replaySequence(request.getSequence());
-            if (replay.isPresent()) {
-                return replay.get();
+            final Optional<TransactionSuccess<?>> maybeReplay = tx.replaySequence(request.getSequence());
+            if (maybeReplay.isPresent()) {
+                final TransactionSuccess<?> replay = maybeReplay.get();
+                LOG.debug("{}: envelope {} replaying response {}", persistenceId(), envelope, replay);
+                return replay;
             }
         }
 
             }
         }
 
index 9a8e89eb422c4b4f909e6a6c3ea97366d1ab01f2..312e11290c88e713827fe2b051d96b667eb633da 100644 (file)
@@ -42,10 +42,12 @@ final class ShardDataTreeTransactionChain extends ShardDataTreeTransactionParent
         Preconditions.checkState(openTransaction == null, "Transaction %s is open", openTransaction);
 
         if (previousTx == null) {
         Preconditions.checkState(openTransaction == null, "Transaction %s is open", openTransaction);
 
         if (previousTx == null) {
+            LOG.debug("Opening an unchained snapshot in {}", chainId);
             return dataTree.takeSnapshot();
             return dataTree.takeSnapshot();
-        } else {
-            return previousTx.getSnapshot();
         }
         }
+
+        LOG.debug("Reusing a chained snapshot in {}", chainId);
+        return previousTx.getSnapshot();
     }
 
     ReadOnlyShardDataTreeTransaction newReadOnlyTransaction(final TransactionIdentifier txId) {
     }
 
     ReadOnlyShardDataTreeTransaction newReadOnlyTransaction(final TransactionIdentifier txId) {
index db271ba737785283134f034b6d599ec184dd28a1..4cd5b220075b440c0ab697b8971cb22411294a7b 100644 (file)
@@ -3,5 +3,8 @@ org.slf4j.simpleLogger.dateTimeFormat=hh:mm:ss,S a
 org.slf4j.simpleLogger.logFile=System.out
 org.slf4j.simpleLogger.showShortLogName=true
 org.slf4j.simpleLogger.levelInBrackets=true
 org.slf4j.simpleLogger.logFile=System.out
 org.slf4j.simpleLogger.showShortLogName=true
 org.slf4j.simpleLogger.levelInBrackets=true
+org.slf4j.simpleLogger.log.org.opendaylight.controller.cluster.access=debug
+org.slf4j.simpleLogger.log.org.opendaylight.controller.cluster.databroker=debug
 org.slf4j.simpleLogger.log.org.opendaylight.controller.cluster.datastore=debug
 org.slf4j.simpleLogger.log.org.opendaylight.controller.cluster.datastore=debug
-org.slf4j.simpleLogger.log.org.opendaylight.controller.cluster.datastore.node.utils.stream=off
\ No newline at end of file
+org.slf4j.simpleLogger.log.org.opendaylight.controller.cluster.databroker.actors.dds=debug
+org.slf4j.simpleLogger.log.org.opendaylight.controller.cluster.datastore.node.utils.stream=off