BUG-5280: fix NPE during transaction purge 31/53731/2
authorRobert Varga <rovarga@cisco.com>
Thu, 23 Mar 2017 13:13:44 +0000 (14:13 +0100)
committerTom Pantelis <tompantelis@gmail.com>
Thu, 23 Mar 2017 15:12:15 +0000 (15:12 +0000)
Read/write transactions which transition to ready state
throw away their open transaction, which causes the following
exception:

Shard - member-1-shard-people-testTransactionChainWithMultipleShards: request Envelope{sessionId=1, txSequence=11, message=TransactionPurgeRequest{target=member-2-datastore-testTransactionChainWithMultipleShards-fe-0-chn-1-txn-2-2, sequence=2, replyTo=Actor[akka://cluster-test@127.0.0.1:2559/user/$a#-493460599]}} caused failure
java.lang.NullPointerException
 at org.opendaylight.controller.cluster.datastore.FrontendReadWriteTransaction.purge(FrontendReadWriteTransaction.java:113)
 at org.opendaylight.controller.cluster.datastore.AbstractFrontendHistory.handleTransactionRequest(AbstractFrontendHistory.java:114)
 at org.opendaylight.controller.cluster.datastore.LeaderFrontendState.handleTransactionRequest(LeaderFrontendState.java:197)
 at org.opendaylight.controller.cluster.datastore.Shard.handleRequest(Shard.java:413)
 at org.opendaylight.controller.cluster.datastore.Shard.handleNonRaftCommand(Shard.java:277)

Rework purge logic to talk directly to the data tree, which
prevents this from happening and simplifies the code a bit.

Change-Id: I7cc08687648d2473a712c171944a06307e4d8f9f
Signed-off-by: Robert Varga <rovarga@cisco.com>
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractShardDataTreeTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadOnlyTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadWriteTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionParent.java

index f23a8ff..aef83ae 100644 (file)
@@ -111,7 +111,7 @@ abstract class AbstractFrontendHistory implements Identifiable<LocalHistoryIdent
                 return new TransactionPurgeResponse(id, request.getSequence());
             }
 
-            tx.purge(() -> {
+            tree.purgeTransaction(id, () -> {
                 purgedTransactions.add(Range.singleton(ul));
                 transactions.remove(id);
                 LOG.debug("{}: finished purging transaction {}", persistenceId(), id);
index 56e11c1..c19cb9a 100644 (file)
@@ -75,13 +75,6 @@ abstract class AbstractShardDataTreeTransaction<T extends DataTreeSnapshot>
         parent.abortTransaction(this, callback);
     }
 
-    final void purge(final Runnable callback) {
-        if (!closed) {
-            LOG.warn("Purging unclosed transaction {}", id);
-        }
-        parent.purgeTransaction(id, callback);
-    }
-
     @Override
     public final String toString() {
         return MoreObjects.toStringHelper(this).add("id", id).add("closed", closed).add("snapshot", snapshot)
index 5c0d913..a3cffd1 100644 (file)
@@ -60,11 +60,6 @@ final class FrontendReadOnlyTransaction extends FrontendTransaction {
         }
     }
 
-    @Override
-    void purge(final Runnable callback) {
-        openTransaction.purge(callback);
-    }
-
     private void handleTransactionAbort(final TransactionAbortRequest request, final RequestEnvelope envelope,
             final long now) throws RequestException {
         openTransaction.abort(() -> recordAndSendSuccess(envelope, now, new TransactionAbortSuccess(request.getTarget(),
index 9c4d4dd..f9d9580 100644 (file)
@@ -108,11 +108,6 @@ final class FrontendReadWriteTransaction extends FrontendTransaction {
         }
     }
 
-    @Override
-    void purge(final Runnable callback) {
-        openTransaction.purge(callback);
-    }
-
     private void handleTransactionPreCommit(final TransactionPreCommitRequest request,
             final RequestEnvelope envelope, final long now) throws RequestException {
         readyCohort.preCommit(new FutureCallback<DataTreeCandidate>() {
index fccf3bf..8846467 100644 (file)
@@ -112,9 +112,6 @@ abstract class FrontendTransaction implements Identifiable<TransactionIdentifier
     abstract @Nullable TransactionSuccess<?> handleRequest(TransactionRequest<?> request,
             RequestEnvelope envelope, long now) throws RequestException;
 
-    // Final request, needs routing to the data tree, so it can persist a tombstone
-    abstract void purge(Runnable callback);
-
     private void recordResponse(final long sequence, final Object response) {
         if (replayQueue.isEmpty()) {
             firstReplaySequence = sequence;
index 660a736..3b15b65 100644 (file)
@@ -659,13 +659,6 @@ public class ShardDataTree extends ShardDataTreeTransactionParent {
         replicatePayload(id, AbortTransactionPayload.create(id), callback);
     }
 
-
-    @Override
-    void purgeTransaction(final TransactionIdentifier id, final Runnable callback) {
-        LOG.debug("{}: purging transaction {}", logContext, id);
-        replicatePayload(id, PurgeTransactionPayload.create(id), callback);
-    }
-
     @Override
     ShardDataTreeCohort finishTransaction(final ReadWriteShardDataTreeTransaction transaction) {
         final DataTreeModification snapshot = transaction.getSnapshot();
@@ -674,6 +667,11 @@ public class ShardDataTree extends ShardDataTreeTransactionParent {
         return createReadyCohort(transaction.getIdentifier(), snapshot);
     }
 
+    void purgeTransaction(final TransactionIdentifier id, final Runnable callback) {
+        LOG.debug("{}: purging transaction {}", logContext, id);
+        replicatePayload(id, PurgeTransactionPayload.create(id), callback);
+    }
+
     public Optional<NormalizedNode<?, ?>> readNode(final YangInstanceIdentifier path) {
         return dataTree.takeSnapshot().readNode(path);
     }
index dfd6680..e620e52 100644 (file)
@@ -83,11 +83,6 @@ final class ShardDataTreeTransactionChain extends ShardDataTreeTransactionParent
         dataTree.abortTransaction(transaction, callback);
     }
 
-    @Override
-    void purgeTransaction(final TransactionIdentifier id, final Runnable callback) {
-        dataTree.purgeTransaction(id, callback);
-    }
-
     @Override
     ShardDataTreeCohort finishTransaction(final ReadWriteShardDataTreeTransaction transaction) {
         Preconditions.checkState(openTransaction != null,
index 2b0b74d..21c8c3b 100644 (file)
@@ -14,8 +14,6 @@ abstract class ShardDataTreeTransactionParent {
 
     abstract void abortTransaction(AbstractShardDataTreeTransaction<?> transaction, Runnable callback);
 
-    abstract void purgeTransaction(TransactionIdentifier id, Runnable callback);
-
     abstract ShardDataTreeCohort finishTransaction(ReadWriteShardDataTreeTransaction transaction);
 
     abstract ShardDataTreeCohort createReadyCohort(TransactionIdentifier id, DataTreeModification mod);

©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.