BUG-8403: fix DONE state propagation 87/58387/1
authorRobert Varga <robert.varga@pantheon.tech>
Mon, 5 Jun 2017 17:50:52 +0000 (19:50 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 7 Jun 2017 08:20:30 +0000 (10:20 +0200)
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)

opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractProxyTransaction.java

index 1eff70f..3a463e5 100644 (file)
@@ -147,13 +147,15 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
         }
 
         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);
+            // 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
@@ -626,10 +628,8 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
         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(),
-            isSnapshotOnly(), isDone);
+            isSnapshotOnly(), local.isDone());
         LOG.debug("{} created successor {}", this, successor);
         local.setSuccessor(successor);
 

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.