BUG-8403: fix DONE state propagation

The DONE state detection logic in replayMessages() was flawed, as
we checked the current state, which is guaranteed to be SuccessorState.

We should be checking the previous state, available from the successor
state. As it turns out we can do this very cleanly by setting the flag
when the successor state gets the previous state assigned.

This also has better performance, as we do not touch the volatile
field multiple times.

Change-Id: Ica2246160bf8fee7aa134bbacb45857235405f6a
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit b135d9ab189dfd7443f895aa96160e22666cb266)

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 1eff70f6d9299ab9f782bbe1e8ed54dbca259f4d..3a463e51906ed7ded5a4e657f044e0b7915e2ce7 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
@@ -147,13 +147,15 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
         }
 
         State getPrevState() {
         }
 
         State getPrevState() {

-            return prevState;
+            return Verify.verifyNotNull(prevState, "Attempted to access previous state, which was not set");

         }
 
         void setPrevState(final State prevState) {
             Verify.verify(this.prevState == null, "Attempted to set previous state to %s when we already have %s",
                     prevState, this.prevState);
             this.prevState = Preconditions.checkNotNull(prevState);
         }
 
         void setPrevState(final State prevState) {
             Verify.verify(this.prevState == null, "Attempted to set previous state to %s when we already have %s",
                     prevState, this.prevState);
             this.prevState = Preconditions.checkNotNull(prevState);

+            // We cannot have duplicate successor states, so this check is sufficient
+            this.done = DONE.equals(prevState);

         }
 
         // To be called from safe contexts, where successor is known to be completed
         }
 
         // To be called from safe contexts, where successor is known to be completed


@@ -626,10 +628,8 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
         final SuccessorState local = getSuccessorState();
         final State prevState = local.getPrevState();
 
         final SuccessorState local = getSuccessorState();
         final State prevState = local.getPrevState();
 

-        final boolean isDone = DONE.equals(state)
-                || state instanceof SuccessorState && ((SuccessorState) state).isDone();

         final AbstractProxyTransaction successor = successorHistory.createTransactionProxy(getIdentifier(),
         final AbstractProxyTransaction successor = successorHistory.createTransactionProxy(getIdentifier(),

-            isSnapshotOnly(), isDone);
+            isSnapshotOnly(), local.isDone());

         LOG.debug("{} created successor {}", this, successor);
         local.setSuccessor(successor);
 
         LOG.debug("{} created successor {}", this, successor);
         local.setSuccessor(successor);