BUG-8372: fix AbstractProxyTransaction.replayMessages()
[controller.git] / opendaylight / md-sal / sal-distributed-datastore / src / main / java / org / opendaylight / controller / cluster / databroker / actors / dds / LocalReadWriteProxyTransaction.java
index 3407da7eab4aa5c11a758f5c0ae039abd154e855..720ada3191e7fd5d06892e0583dcabd55097ffa6 100644 (file)
@@ -10,9 +10,11 @@ package org.opendaylight.controller.cluster.databroker.actors.dds;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Verify;
 import java.util.function.Consumer;
+import java.util.function.Supplier;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.NotThreadSafe;
 import org.opendaylight.controller.cluster.access.commands.AbortLocalTransactionRequest;
+import org.opendaylight.controller.cluster.access.commands.AbstractLocalTransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.CommitLocalTransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.ModifyTransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.PersistenceProtocol;
@@ -22,7 +24,6 @@ import org.opendaylight.controller.cluster.access.commands.TransactionDoCommitRe
 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.TransactionPurgeRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionRequest;
 import org.opendaylight.controller.cluster.access.commands.TransactionWrite;
 import org.opendaylight.controller.cluster.access.concepts.Response;
@@ -58,9 +59,27 @@ import org.slf4j.LoggerFactory;
 final class LocalReadWriteProxyTransaction extends LocalProxyTransaction {
     private static final Logger LOG = LoggerFactory.getLogger(LocalReadWriteProxyTransaction.class);
 
-    private CursorAwareDataTreeModification modification;
+    /**
+     * This field needs to be accessed via {@link #getModification()}, which performs state checking to ensure
+     * the modification can actually be accessed.
+     */
+    private final CursorAwareDataTreeModification modification;
+
+    private Supplier<? extends RuntimeException> closedException;
+
     private CursorAwareDataTreeModification sealedModification;
 
+    /**
+     * Recorded failure from previous operations. Normally we would want to propagate the error directly to the
+     * offending call site, but that exposes inconsistency in behavior during initial connection, when we go through
+     * {@link RemoteProxyTransaction}, which detects this sort of issues at canCommit/directCommit time on the backend.
+     *
+     * <p>
+     * We therefore do not report incurred exceptions directly, but report them once the user attempts to commit
+     * this transaction.
+     */
+    private Exception recordedFailure;
+
     LocalReadWriteProxyTransaction(final ProxyHistory parent, final TransactionIdentifier identifier,
         final DataTreeSnapshot snapshot) {
         super(parent, identifier);
@@ -74,22 +93,61 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction {
 
     @Override
     CursorAwareDataTreeSnapshot readOnlyView() {
-        return modification;
+        return getModification();
     }
 
     @Override
+    @SuppressWarnings("checkstyle:IllegalCatch")
     void doDelete(final YangInstanceIdentifier path) {
-        modification.delete(path);
+        final CursorAwareDataTreeModification mod = getModification();
+        if (recordedFailure != null) {
+            LOG.debug("Transaction {} recorded failure, ignoring delete of {}", getIdentifier(), path);
+            return;
+        }
+
+        try {
+            mod.delete(path);
+        } catch (Exception e) {
+            LOG.debug("Transaction {} delete on {} incurred failure, delaying it until commit", getIdentifier(), path,
+                e);
+            recordedFailure = e;
+        }
     }
 
     @Override
+    @SuppressWarnings("checkstyle:IllegalCatch")
     void doMerge(final YangInstanceIdentifier path, final NormalizedNode<?, ?> data) {
-        modification.merge(path, data);
+        final CursorAwareDataTreeModification mod = getModification();
+        if (recordedFailure != null) {
+            LOG.debug("Transaction {} recorded failure, ignoring merge to {}", getIdentifier(), path);
+            return;
+        }
+
+        try {
+            mod.merge(path, data);
+        } catch (Exception e) {
+            LOG.debug("Transaction {} merge to {} incurred failure, delaying it until commit", getIdentifier(), path,
+                e);
+            recordedFailure = e;
+        }
     }
 
     @Override
+    @SuppressWarnings("checkstyle:IllegalCatch")
     void doWrite(final YangInstanceIdentifier path, final NormalizedNode<?, ?> data) {
-        modification.write(path, data);
+        final CursorAwareDataTreeModification mod = getModification();
+        if (recordedFailure != null) {
+            LOG.debug("Transaction {} recorded failure, ignoring write to {}", getIdentifier(), path);
+            return;
+        }
+
+        try {
+            mod.write(path, data);
+        } catch (Exception e) {
+            LOG.debug("Transaction {} write to {} incurred failure, delaying it until commit", getIdentifier(), path,
+                e);
+            recordedFailure = e;
+        }
     }
 
     private RuntimeException abortedException() {
@@ -102,16 +160,19 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction {
 
     @Override
     CommitLocalTransactionRequest commitRequest(final boolean coordinated) {
+        final CursorAwareDataTreeModification mod = getModification();
         final CommitLocalTransactionRequest ret = new CommitLocalTransactionRequest(getIdentifier(), nextSequence(),
-            localActor(), modification, coordinated);
-        modification = new FailedDataTreeModification(this::submittedException);
+            localActor(), mod, recordedFailure, coordinated);
+        closedException = this::submittedException;
         return ret;
     }
 
     @Override
     void doSeal() {
-        modification.ready();
-        sealedModification = modification;
+        Preconditions.checkState(sealedModification == null, "Transaction %s is already sealed", getIdentifier());
+        final CursorAwareDataTreeModification mod = getModification();
+        mod.ready();
+        sealedModification = mod;
     }
 
     @Override
@@ -142,13 +203,13 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction {
     @Override
     void applyModifyTransactionRequest(final ModifyTransactionRequest request,
             final @Nullable Consumer<Response<?, ?>> callback) {
-        for (TransactionModification mod : request.getModifications()) {
+        for (final TransactionModification mod : request.getModifications()) {
             if (mod instanceof TransactionWrite) {
-                modification.write(mod.getPath(), ((TransactionWrite)mod).getData());
+                write(mod.getPath(), ((TransactionWrite)mod).getData());
             } else if (mod instanceof TransactionMerge) {
-                modification.merge(mod.getPath(), ((TransactionMerge)mod).getData());
+                merge(mod.getPath(), ((TransactionMerge)mod).getData());
             } else if (mod instanceof TransactionDelete) {
-                modification.delete(mod.getPath());
+                delete(mod.getPath());
             } else {
                 throw new IllegalArgumentException("Unsupported modification " + mod);
             }
@@ -161,7 +222,7 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction {
 
             switch (maybeProtocol.get()) {
                 case ABORT:
-                    sendAbort(callback);
+                    sendRequest(new AbortLocalTransactionRequest(getIdentifier(), localActor()), callback);
                     break;
                 case READY:
                     // No-op, as we have already issued a seal()
@@ -178,6 +239,16 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction {
         }
     }
 
+    @Override
+    void handleForwardedLocalRequest(final AbstractLocalTransactionRequest<?> request,
+            final Consumer<Response<?, ?>> callback) {
+        if (request instanceof CommitLocalTransactionRequest) {
+            sendCommit((CommitLocalTransactionRequest) request, callback);
+        } else {
+            super.handleForwardedLocalRequest(request, callback);
+        }
+    }
+
     @Override
     void handleForwardedRemoteRequest(final TransactionRequest<?> request,
             final @Nullable Consumer<Response<?, ?>> callback) {
@@ -194,46 +265,6 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction {
         }
     }
 
-    @Override
-    void forwardToRemote(final RemoteProxyTransaction successor, final TransactionRequest<?> request,
-            final Consumer<Response<?, ?>> callback) {
-        if (request instanceof CommitLocalTransactionRequest) {
-            final CommitLocalTransactionRequest req = (CommitLocalTransactionRequest) request;
-            final DataTreeModification mod = req.getModification();
-
-            LOG.debug("Applying modification {} to successor {}", mod, successor);
-            mod.applyToCursor(new AbstractDataTreeModificationCursor() {
-                @Override
-                public void write(final PathArgument child, final NormalizedNode<?, ?> data) {
-                    successor.write(current().node(child), data);
-                }
-
-                @Override
-                public void merge(final PathArgument child, final NormalizedNode<?, ?> data) {
-                    successor.merge(current().node(child), data);
-                }
-
-                @Override
-                public void delete(final PathArgument child) {
-                    successor.delete(current().node(child));
-                }
-            });
-
-            successor.ensureSealed();
-
-            final ModifyTransactionRequest successorReq = successor.commitRequest(req.isCoordinated());
-            successor.sendRequest(successorReq, callback);
-        } else if (request instanceof AbortLocalTransactionRequest) {
-            LOG.debug("Forwarding abort {} to successor {}", request, successor);
-            successor.abort();
-        } else if (request instanceof TransactionPurgeRequest) {
-            LOG.debug("Forwarding purge {} to successor {}", request, successor);
-            successor.purge();
-        } else {
-            throw new IllegalArgumentException("Unhandled request" + request);
-        }
-    }
-
     @Override
     void forwardToLocal(final LocalProxyTransaction successor, final TransactionRequest<?> request,
             final Consumer<Response<?, ?>> callback) {
@@ -249,12 +280,22 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction {
     @Override
     void sendAbort(final TransactionRequest<?> request, final Consumer<Response<?, ?>> callback) {
         super.sendAbort(request, callback);
-        modification = new FailedDataTreeModification(this::abortedException);
+        closedException = this::abortedException;
+    }
+
+    private CursorAwareDataTreeModification getModification() {
+        if (closedException != null) {
+            throw closedException.get();
+        }
+
+        return modification;
     }
 
     private void sendCommit(final CommitLocalTransactionRequest request, final Consumer<Response<?, ?>> callback) {
         // Rebase old modification on new data tree.
-        try (DataTreeModificationCursor cursor = modification.createCursor(YangInstanceIdentifier.EMPTY)) {
+        final CursorAwareDataTreeModification mod = getModification();
+
+        try (DataTreeModificationCursor cursor = mod.createCursor(YangInstanceIdentifier.EMPTY)) {
             request.getModification().applyToCursor(cursor);
         }