From 26aee5f28675801253a1244d0c0ad74a7e51b3e4 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Thu, 29 Nov 2018 22:32:48 -0500 Subject: [PATCH] Improve error reporting in WriteTransactionsHandler The Leader Isolation CSIT is failing intermittently with a failed tx and it would help to log the exception trace as restconf no longer reports it. Also modified some fields to be thread-safe as they are accessed by multiple threads although not concurrently/ Change-Id: Ifce3cd177e5e0cc7c7020866000cb915b9f9c4bf Signed-off-by: Tom Pantelis --- .../impl/AbstractTransactionHandler.java | 17 ++++++------ .../impl/ProduceTransactionsHandler.java | 9 +++---- .../impl/WriteTransactionsHandler.java | 26 +++++++++---------- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/AbstractTransactionHandler.java b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/AbstractTransactionHandler.java index 90cdce32dd..1a0f39e693 100644 --- a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/AbstractTransactionHandler.java +++ b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/AbstractTransactionHandler.java @@ -19,6 +19,7 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicLong; import org.opendaylight.yang.gen.v1.tag.opendaylight.org._2017.controller.yang.lowlevel.control.rev170215.TransactionsParams; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; @@ -76,7 +77,7 @@ abstract class AbstractTransactionHandler { private ScheduledFuture writingFuture; private ScheduledFuture completingFuture; - private long txCounter; + private final AtomicLong txCounter = new AtomicLong(); private volatile State state; AbstractTransactionHandler(final TransactionsParams params) { @@ -117,7 +118,7 @@ abstract class AbstractTransactionHandler { } // Not completed yet: create a transaction and hook it up - final long txId = txCounter++; + final long txId = txCounter.incrementAndGet(); final ListenableFuture execFuture = execWrite(txId); LOG.debug("New future #{} allocated", txId); @@ -150,7 +151,7 @@ abstract class AbstractTransactionHandler { LOG.debug("Completed waiting for all futures"); state = State.SUCCESSFUL; completingFuture.cancel(false); - runSuccessful(txCounter); + runSuccessful(txCounter.get()); return true; } @@ -176,7 +177,7 @@ abstract class AbstractTransactionHandler { } final void txFailure(final ListenableFuture execFuture, final long txId, final Throwable cause) { - LOG.debug("Future #{} failed", txId, cause); + LOG.error("Commit future failed for tx # {}", txId, cause); futures.remove(execFuture); final State local = state; @@ -188,7 +189,7 @@ abstract class AbstractTransactionHandler { case WAITING: state = State.FAILED; writingFuture.cancel(false); - runFailed(cause); + runFailed(cause, txId); break; default: throw new IllegalStateException("Unhandled state " + local); @@ -221,14 +222,14 @@ abstract class AbstractTransactionHandler { } state = State.FAILED; - runTimedOut(new TimeoutException("Collection did not finish in " + DEAD_TIMEOUT_SECONDS + " seconds")); + runTimedOut("Transactions did not finish in " + DEAD_TIMEOUT_SECONDS + " seconds"); } abstract ListenableFuture execWrite(long txId); - abstract void runFailed(Throwable cause); + abstract void runFailed(Throwable cause, long txId); abstract void runSuccessful(long allTx); - abstract void runTimedOut(Exception cause); + abstract void runTimedOut(String cause); } diff --git a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/ProduceTransactionsHandler.java b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/ProduceTransactionsHandler.java index 99e48b2980..db29583b44 100644 --- a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/ProduceTransactionsHandler.java +++ b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/ProduceTransactionsHandler.java @@ -135,10 +135,10 @@ public final class ProduceTransactionsHandler extends AbstractTransactionHandler } @Override - void runFailed(final Throwable cause) { + void runFailed(final Throwable cause, final long txId) { closeProducer(itemProducer); future.set(RpcResultBuilder.failed() - .withError(RpcError.ErrorType.APPLICATION, "Submit failed", cause).build()); + .withError(RpcError.ErrorType.APPLICATION, "Commit failed for tx # " + txId, cause).build()); } @Override @@ -154,10 +154,9 @@ public final class ProduceTransactionsHandler extends AbstractTransactionHandler } @Override - void runTimedOut(final Exception cause) { + void runTimedOut(final String cause) { closeProducer(itemProducer); future.set(RpcResultBuilder.failed() - .withError(RpcError.ErrorType.APPLICATION, - "Final submit was timed out by the test provider or was interrupted", cause).build()); + .withError(RpcError.ErrorType.APPLICATION, cause).build()); } } diff --git a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/WriteTransactionsHandler.java b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/WriteTransactionsHandler.java index 6426bc09e2..e2418dc84b 100644 --- a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/WriteTransactionsHandler.java +++ b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/WriteTransactionsHandler.java @@ -12,13 +12,14 @@ import com.google.common.base.Preconditions; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.SettableFuture; -import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Set; import java.util.SplittableRandom; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicLong; import org.opendaylight.controller.md.sal.common.api.data.AsyncTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.common.api.data.OptimisticLockFailedException; @@ -115,11 +116,11 @@ public abstract class WriteTransactionsHandler extends AbstractTransactionHandle private static final Logger LOG = LoggerFactory.getLogger(WriteTransactionsHandler.class); final SettableFuture> completionFuture = SettableFuture.create(); - private final Set usedValues = new HashSet<>(); + private final Set usedValues = ConcurrentHashMap.newKeySet(); private final YangInstanceIdentifier idListItem; - private long insertTx = 0; - private long deleteTx = 0; + private final AtomicLong insertTx = new AtomicLong(); + private final AtomicLong deleteTx = new AtomicLong(); WriteTransactionsHandler(final YangInstanceIdentifier idListItem, final WriteTransactionsInput input) { super(input); @@ -205,13 +206,13 @@ public abstract class WriteTransactionsHandler extends AbstractTransactionHandle if (usedValues.contains(i)) { LOG.debug("Deleting item: {}", i); - deleteTx++; + deleteTx.incrementAndGet(); tx.delete(LogicalDatastoreType.CONFIGURATION, entryId); usedValues.remove(i); } else { LOG.debug("Inserting item: {}", i); - insertTx++; + insertTx.incrementAndGet(); final MapEntryNode entry = ImmutableNodes.mapEntry(ITEM, NUMBER, i); tx.put(LogicalDatastoreType.CONFIGURATION, entryId, entry); usedValues.add(i); @@ -221,17 +222,17 @@ public abstract class WriteTransactionsHandler extends AbstractTransactionHandle } @Override - void runFailed(final Throwable cause) { + void runFailed(final Throwable cause, final long txId) { completionFuture.set(RpcResultBuilder.failed() - .withError(RpcError.ErrorType.APPLICATION, "Submit failed", cause).build()); + .withError(RpcError.ErrorType.APPLICATION, "Commit failed for tx # " + txId, cause).build()); } @Override void runSuccessful(final long allTx) { final WriteTransactionsOutput output = new WriteTransactionsOutputBuilder() .setAllTx(allTx) - .setInsertTx(insertTx) - .setDeleteTx(deleteTx) + .setInsertTx(insertTx.get()) + .setDeleteTx(deleteTx.get()) .build(); completionFuture.set(RpcResultBuilder.success() @@ -239,10 +240,9 @@ public abstract class WriteTransactionsHandler extends AbstractTransactionHandle } @Override - void runTimedOut(final Exception cause) { + void runTimedOut(final String cause) { completionFuture.set(RpcResultBuilder.failed() - .withError(RpcError.ErrorType.APPLICATION, - "Final submit was timed out by the test provider or was interrupted", cause).build()); + .withError(RpcError.ErrorType.APPLICATION, cause).build()); } abstract DOMDataWriteTransaction createTransaction(); -- 2.36.6