From: Robert Varga Date: Sun, 14 Sep 2014 22:03:04 +0000 (+0200) Subject: BUG-865: TransactionCommitDeadlockException safety X-Git-Tag: release/helium~77 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=daf965d90a98f590751a1da6e61558497fff4e0a BUG-865: TransactionCommitDeadlockException safety This patch fixes safety (public non-constant field) and improves performance by using a shared RpcError instances. Also moves over to use a Supplier instead of a Function. Change-Id: Id9d269fe9be6dacea3c216d4ad76efa79bdb27f0 Signed-off-by: Robert Varga --- diff --git a/opendaylight/md-sal/sal-common-api/src/main/java/org/opendaylight/controller/md/sal/common/api/data/TransactionCommitDeadlockException.java b/opendaylight/md-sal/sal-common-api/src/main/java/org/opendaylight/controller/md/sal/common/api/data/TransactionCommitDeadlockException.java index 60313bf109..50952eaaf1 100644 --- a/opendaylight/md-sal/sal-common-api/src/main/java/org/opendaylight/controller/md/sal/common/api/data/TransactionCommitDeadlockException.java +++ b/opendaylight/md-sal/sal-common-api/src/main/java/org/opendaylight/controller/md/sal/common/api/data/TransactionCommitDeadlockException.java @@ -8,11 +8,10 @@ package org.opendaylight.controller.md.sal.common.api.data; +import com.google.common.base.Supplier; import org.opendaylight.yangtools.yang.common.RpcError; -import org.opendaylight.yangtools.yang.common.RpcResultBuilder; import org.opendaylight.yangtools.yang.common.RpcError.ErrorType; - -import com.google.common.base.Function; +import org.opendaylight.yangtools.yang.common.RpcResultBuilder; /** * A type of TransactionCommitFailedException that indicates a situation that would result in a @@ -24,23 +23,21 @@ import com.google.common.base.Function; * @author Thomas Pantelis */ public class TransactionCommitDeadlockException extends TransactionCommitFailedException { - private static final long serialVersionUID = 1L; - private static final String DEADLOCK_MESSAGE = "An attempt to block on a ListenableFuture via a get method from a write " + "transaction submit was detected that would result in deadlock. The commit " + "result must be obtained asynchronously, e.g. via Futures#addCallback, to avoid deadlock."; + private static final RpcError DEADLOCK_RPCERROR = RpcResultBuilder.newError(ErrorType.APPLICATION, "lock-denied", DEADLOCK_MESSAGE); - public static Function DEADLOCK_EXECUTOR_FUNCTION = new Function() { + public static final Supplier DEADLOCK_EXCEPTION_SUPPLIER = new Supplier() { @Override - public Exception apply(Void notUsed) { - return new TransactionCommitDeadlockException( DEADLOCK_MESSAGE, - RpcResultBuilder.newError(ErrorType.APPLICATION, "lock-denied", DEADLOCK_MESSAGE)); + public Exception get() { + return new TransactionCommitDeadlockException(DEADLOCK_MESSAGE, DEADLOCK_RPCERROR); } }; - public TransactionCommitDeadlockException(String message, final RpcError... errors) { + public TransactionCommitDeadlockException(final String message, final RpcError... errors) { super(message, errors); } } diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/dom/impl/DomInmemoryDataBrokerModule.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/dom/impl/DomInmemoryDataBrokerModule.java index b423bbd0e5..0841435785 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/dom/impl/DomInmemoryDataBrokerModule.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/dom/impl/DomInmemoryDataBrokerModule.java @@ -7,8 +7,8 @@ */ package org.opendaylight.controller.config.yang.md.sal.dom.impl; +import com.google.common.collect.ImmutableMap; import java.util.concurrent.ExecutorService; - import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitDeadlockException; import org.opendaylight.controller.md.sal.common.util.jmx.ThreadExecutorStatsMXBeanImpl; @@ -18,7 +18,6 @@ import org.opendaylight.controller.md.sal.dom.store.impl.InMemoryDOMDataStoreFac import org.opendaylight.controller.sal.core.spi.data.DOMStore; import org.opendaylight.yangtools.util.concurrent.DeadlockDetectingListeningExecutorService; import org.opendaylight.yangtools.util.concurrent.SpecialExecutors; -import com.google.common.collect.ImmutableMap; /** * @@ -88,7 +87,7 @@ public final class DomInmemoryDataBrokerModule extends DOMDataBrokerImpl newDataBroker = new DOMDataBrokerImpl(datastores, new DeadlockDetectingListeningExecutorService(commitExecutor, - TransactionCommitDeadlockException.DEADLOCK_EXECUTOR_FUNCTION, + TransactionCommitDeadlockException.DEADLOCK_EXCEPTION_SUPPLIER, listenableFutureExecutor)); final CommitStatsMXBeanImpl commitStatsMXBean = new CommitStatsMXBeanImpl( diff --git a/opendaylight/md-sal/sal-dom-broker/src/test/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMBrokerTest.java b/opendaylight/md-sal/sal-dom-broker/src/test/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMBrokerTest.java index e57d08f173..674d2ff44a 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/test/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMBrokerTest.java +++ b/opendaylight/md-sal/sal-dom-broker/src/test/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMBrokerTest.java @@ -1,12 +1,19 @@ package org.opendaylight.controller.md.sal.dom.broker.impl; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.assertEquals; import static org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType.CONFIGURATION; import static org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType.OPERATIONAL; - +import com.google.common.base.Optional; +import com.google.common.collect.ImmutableMap; +import com.google.common.util.concurrent.ForwardingExecutorService; +import com.google.common.util.concurrent.FutureCallback; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.common.util.concurrent.MoreExecutors; import java.util.Collections; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; @@ -15,7 +22,6 @@ import java.util.concurrent.Executors; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; - import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -39,15 +45,6 @@ import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; import org.opendaylight.yangtools.yang.model.api.SchemaContext; -import com.google.common.base.Optional; -import com.google.common.collect.ImmutableMap; -import com.google.common.util.concurrent.ForwardingExecutorService; -import com.google.common.util.concurrent.FutureCallback; -import com.google.common.util.concurrent.Futures; -import com.google.common.util.concurrent.ListenableFuture; -import com.google.common.util.concurrent.ListeningExecutorService; -import com.google.common.util.concurrent.MoreExecutors; - public class DOMBrokerTest { private SchemaContext schemaContext; @@ -76,7 +73,7 @@ public class DOMBrokerTest { commitExecutor = new CommitExecutorService(Executors.newSingleThreadExecutor()); futureExecutor = SpecialExecutors.newBlockingBoundedCachedThreadPool(1, 5, "FCB"); executor = new DeadlockDetectingListeningExecutorService(commitExecutor, - TransactionCommitDeadlockException.DEADLOCK_EXECUTOR_FUNCTION, futureExecutor); + TransactionCommitDeadlockException.DEADLOCK_EXCEPTION_SUPPLIER, futureExecutor); domBroker = new DOMDataBrokerImpl(stores, executor); } @@ -215,19 +212,19 @@ public class DOMBrokerTest { TestDOMDataChangeListener dcListener = new TestDOMDataChangeListener() { @Override - public void onDataChanged( AsyncDataChangeEvent> change ) { + public void onDataChanged( final AsyncDataChangeEvent> change ) { DOMDataWriteTransaction writeTx = domBroker.newWriteOnlyTransaction(); writeTx.put( OPERATIONAL, TestModel.TEST2_PATH, ImmutableNodes.containerNode( TestModel.TEST2_QNAME ) ); Futures.addCallback( writeTx.submit(), new FutureCallback() { @Override - public void onSuccess( Void result ) { + public void onSuccess( final Void result ) { commitCompletedLatch.countDown(); } @Override - public void onFailure( Throwable t ) { + public void onFailure( final Throwable t ) { caughtCommitEx.set( t ); commitCompletedLatch.countDown(); } @@ -271,7 +268,7 @@ public class DOMBrokerTest { TestDOMDataChangeListener dcListener = new TestDOMDataChangeListener() { @Override - public void onDataChanged( AsyncDataChangeEvent> change ) { + public void onDataChanged( final AsyncDataChangeEvent> change ) { DOMDataWriteTransaction writeTx = domBroker.newWriteOnlyTransaction(); writeTx.put( OPERATIONAL, TestModel.TEST2_PATH, ImmutableNodes.containerNode( TestModel.TEST2_QNAME ) ); @@ -333,7 +330,7 @@ public class DOMBrokerTest { private final CountDownLatch latch = new CountDownLatch( 1 ); @Override - public void onDataChanged( AsyncDataChangeEvent> change ) { + public void onDataChanged( final AsyncDataChangeEvent> change ) { this.change = change; latch.countDown(); } @@ -347,7 +344,7 @@ public class DOMBrokerTest { ExecutorService delegate; - public CommitExecutorService( ExecutorService delegate ) { + public CommitExecutorService( final ExecutorService delegate ) { this.delegate = delegate; }