From 17a38939f6ba3cbbc1ff0f1f3e00b58f5002813d Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Thu, 23 Mar 2017 14:13:44 +0100 Subject: [PATCH] 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 --- .../cluster/datastore/AbstractFrontendHistory.java | 2 +- .../datastore/AbstractShardDataTreeTransaction.java | 7 ------- .../datastore/FrontendReadOnlyTransaction.java | 5 ----- .../datastore/FrontendReadWriteTransaction.java | 5 ----- .../cluster/datastore/FrontendTransaction.java | 3 --- .../controller/cluster/datastore/ShardDataTree.java | 12 +++++------- .../datastore/ShardDataTreeTransactionChain.java | 5 ----- .../datastore/ShardDataTreeTransactionParent.java | 2 -- 8 files changed, 6 insertions(+), 35 deletions(-) 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 f23a8ffe98..aef83aee67 100644 --- 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 { + 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 56e11c1249..c19cb9a080 100644 --- 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 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 5c0d913963..a3cffd1d5e 100644 --- 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 9c4d4ddbca..f9d9580214 100644 --- 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() { 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 fccf3bf4a1..8846467b59 100644 --- 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 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 660a736920..3b15b656b3 100644 --- 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> 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 dfd6680045..e620e52eb3 100644 --- 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 2b0b74d02c..21c8c3bba2 100644 --- 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); -- 2.36.6