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 dab0c9fe27dab3e69ddc4cbbb43d87ea5be42b45..132ebbf34ba1a124241d6a2c77f36a2c25f712f1 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 284be4a45793699576e412c88d559d15bc916bba..ff405eeb48a9994c1c68eb38664e99e595113fa2 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;