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 <rovarga@cisco.com>

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 60313bf109ba30dd6692365181456337f9a91402..50952eaaf1ca90c89e02a5636e5d3594f0809252 100644 (file)
--- 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<Void, Exception> DEADLOCK_EXECUTOR_FUNCTION = new Function<Void, Exception>() {
+    public static final Supplier<Exception> DEADLOCK_EXCEPTION_SUPPLIER = new Supplier<Exception>() {
         @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 b423bbd0e5c644c147a69398a65da0a564c985a1..0841435785ebb70a30dc10acc24fa13c1d6ad3bf 100644 (file)
--- 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 e57d08f1737fde07dc455eabfc53c2e5304cd53f..674d2ff44a24a959dd7a0ca7f3ecc6bcb5662f03 100644 (file)
--- 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<YangInstanceIdentifier, NormalizedNode<?, ?>> change ) {
+            public void onDataChanged( final AsyncDataChangeEvent<YangInstanceIdentifier, NormalizedNode<?, ?>> change ) {
 
                 DOMDataWriteTransaction writeTx = domBroker.newWriteOnlyTransaction();
                 writeTx.put( OPERATIONAL, TestModel.TEST2_PATH,
                              ImmutableNodes.containerNode( TestModel.TEST2_QNAME ) );
                 Futures.addCallback( writeTx.submit(), new FutureCallback<Void>() {
                     @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<YangInstanceIdentifier, NormalizedNode<?, ?>> change ) {
+            public void onDataChanged( final AsyncDataChangeEvent<YangInstanceIdentifier, NormalizedNode<?, ?>> 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<YangInstanceIdentifier, NormalizedNode<?, ?>> change ) {
+        public void onDataChanged( final AsyncDataChangeEvent<YangInstanceIdentifier, NormalizedNode<?, ?>> 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;
         }