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>

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 f8cb9e01bce7fdd4656c6c74d162e29119858712..546738fb670ec9592f7086ac4ea84c3b8f8310c1 100644 (file)
--- 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) {
      * @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);
     }
         // 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 ccce603652d9f1e52be7345fd80b093e6b3f78ce..8b19e30b4b641db2eea3abca902fbded1f086c0d 100644 (file)
--- 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
     }
 
     @Override

-    void sealOnly() {
+    boolean sealOnly() {

         sealModification();
         sealModification();

-        super.sealOnly();
+        return super.sealOnly();

     }
 
     @Override
     }
 
     @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 46e1924dd51e44d49f4ccf4f0a213fda82d6e948..d36218c19e13a5eae47d10ef9bbf22ed9061ab5e 100644 (file)
--- 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;
 
  */
 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;
 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()) {
             // 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;
             }
 
             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()) {
             // 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;
             }
 
             final TransactionRequest<?> tmp;