From: Robert Varga <robert.varga@pantheon.tech>
Date: Wed, 20 Feb 2019 12:56:30 +0000 (+0100)
Subject: Do not assert seal transition on forward path
X-Git-Tag: release/neon~4
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=6a6c8b35a9c389488f76e221abca9bcd16ceff64

Do not assert seal transition on forward path

Unlike the replay path, where we expect no successor, the forward
path may very well encounter a successor being present.

Move the assertion to replay path only, as that is where it catches
issues. The forward path automatically propagates the seal via its
use of sendRequest(), which is forwarded as needed and therefore
we only note the race via LOG.debug() and not assert it.

Change-Id: I6f95acc49800d456c049d19982c801015fd69420
JIRA: CONTROLLER-1885
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit db3d7caeeb310f76a9a159f9a8d7e9beff89f645)
---

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractProxyTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractProxyTransaction.java
index dab0c9fe27..132ebbf34b 100644
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractProxyTransaction.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractProxyTransaction.java
@@ -363,10 +363,14 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
         }
     }
 
-    void sealOnly() {
-        parent.onTransactionSealed(this);
-        final boolean success = STATE_UPDATER.compareAndSet(this, OPEN, SEALED);
-        Verify.verify(success, "Attempted to replay seal on %s", this);
+    /**
+     * Seal this transaction. If this method reports false, the caller needs to deal with propagating the seal operation
+     * towards the successor.
+     *
+     * @return True if seal operation was successful, false if this proxy has a successor.
+     */
+    boolean sealOnly() {
+        return sealState();
     }
 
     /**
@@ -377,8 +381,11 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
      * @return True if seal operation was successful, false if this proxy has a successor.
      */
     boolean sealAndSend(final Optional<Long> enqueuedTicks) {
-        parent.onTransactionSealed(this);
+        return sealState();
+    }
 
+    private boolean sealState() {
+        parent.onTransactionSealed(this);
         // Transition internal state to sealed and detect presence of a successor
         return STATE_UPDATER.compareAndSet(this, OPEN, SEALED);
     }
diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadWriteProxyTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadWriteProxyTransaction.java
index ccce603652..8b19e30b4b 100644
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadWriteProxyTransaction.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadWriteProxyTransaction.java
@@ -185,9 +185,9 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction {
     }
 
     @Override
-    void sealOnly() {
+    boolean sealOnly() {
         sealModification();
-        super.sealOnly();
+        return super.sealOnly();
     }
 
     @Override
diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/RemoteProxyTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/RemoteProxyTransaction.java
index 284be4a457..ff405eeb48 100644
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/RemoteProxyTransaction.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/RemoteProxyTransaction.java
@@ -7,6 +7,8 @@
  */
 package org.opendaylight.controller.cluster.databroker.actors.dds;
 
+import static com.google.common.base.Verify.verify;
+
 import com.google.common.util.concurrent.FluentFuture;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
@@ -347,7 +349,9 @@ final class RemoteProxyTransaction extends AbstractProxyTransaction {
             // Persistence protocol implies we are sealed, propagate the marker, but hold off doing other actions
             // until we know what we are going to do.
             if (markSealed()) {
-                sealOnly();
+                if (!sealOnly()) {
+                    LOG.debug("Proxy {} has a successor, which should receive seal through a separate request", this);
+                }
             }
 
             final TransactionRequest<?> tmp;
@@ -488,7 +492,7 @@ final class RemoteProxyTransaction extends AbstractProxyTransaction {
             // Persistence protocol implies we are sealed, propagate the marker, but hold off doing other actions
             // until we know what we are going to do.
             if (markSealed()) {
-                sealOnly();
+                verify(sealOnly(), "Attempted to replay seal on %s", this);
             }
 
             final TransactionRequest<?> tmp;