BUG-5280: fix NPE during transaction purge

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>

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java
index f23a8ffe98abcec519fd9ed73f006c22a2417553..aef83aee6704a0d26585cf549949d4f1c93a332c 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java
@@ -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);

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractShardDataTreeTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractShardDataTreeTransaction.java
index 56e11c124984c6042f269cced30b06322d9e9475..c19cb9a0802c8ce285ce0cdc368ee824a4906d24 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractShardDataTreeTransaction.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractShardDataTreeTransaction.java
@@ -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)

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadOnlyTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadOnlyTransaction.java
index 5c0d91396347065375beb3e9eb583ba6ad8eaf7c..a3cffd1d5ee9f43fdd0510fa5899b0c525ac326d 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadOnlyTransaction.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadOnlyTransaction.java
@@ -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(),

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadWriteTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadWriteTransaction.java
index 9c4d4ddbca9f1cd1bdd81bb3654fc76d156b7fee..f9d9580214e41c2d28430c3a46c68b005ad6e8bc 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadWriteTransaction.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadWriteTransaction.java
@@ -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>() {

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendTransaction.java
index fccf3bf4a1d321384dafb49cf213cbea3f313922..8846467b59d2d924f783014215d947551a498f21 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendTransaction.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendTransaction.java
@@ -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;

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java
index 660a736920b2c930027d3f98c0686c7056c2292c..3b15b656b35f64fd0441da8a9c28489f2dc4a8dc 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java
@@ -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);
     }

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java
index dfd6680045e2d997744e903861dea7c6bc8fb24f..e620e52eb38026197953acfa01d0ae185a4834ca 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java
@@ -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,

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionParent.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionParent.java
index 2b0b74d02c3b9209a7d166a894b3b15cc0953aba..21c8c3bba2c667df3e131e46a1dc109f1faa8d40 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionParent.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionParent.java
@@ -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);