Isolate TransactionInvokerImpl.executeCommand()

This is a chunk of logic which is independent of its surroundings,
but requires proper locking.

JIRA: OVSDB-428
Change-Id: I7db00574a50955e3bf3531cea23ba85b80e11bdb
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transactions/md/TransactionInvokerImpl.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transactions/md/TransactionInvokerImpl.java
index e9d2dc348a3040ddf407ee3c77b273dcdd1236e1..7763d00deb0e1f0f75ff6224bb9c289a2f19a19b 100644 (file)
--- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transactions/md/TransactionInvokerImpl.java
+++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transactions/md/TransactionInvokerImpl.java
@@ -99,26 +99,7 @@ public class TransactionInvokerImpl implements TransactionInvoker,TransactionCha
             commandIterator = commands.iterator();
             try {
                 while (commandIterator.hasNext()) {
-                    TransactionCommand command = commandIterator.next();
-                    synchronized (this) {
-                        final ReadWriteTransaction transaction = chain.newReadWriteTransaction();
-                        transactionInFlight = transaction;
-                        recordPendingTransaction(command, transaction);
-                        command.execute(transaction);
-                        ListenableFuture<Void> ft = transaction.submit();
-                        command.setTransactionResultFuture(ft);
-                        Futures.addCallback(ft, new FutureCallback<Void>() {
-                            @Override
-                            public void onSuccess(final Void result) {
-                                forgetSuccessfulTransaction(transaction);
-                            }
-
-                            @Override
-                            public void onFailure(final Throwable throwable) {
-                                // NOOP - handled by failure of transaction chain
-                            }
-                        }, MoreExecutors.directExecutor());
-                    }
+                    executeCommand(commandIterator.next());
                 }
                 transactionInFlight = null;
             } catch (IllegalStateException e) {
@@ -135,6 +116,26 @@ public class TransactionInvokerImpl implements TransactionInvoker,TransactionCha
         }
     }
 
+    private synchronized void executeCommand(final TransactionCommand command) {
+        final ReadWriteTransaction transaction = chain.newReadWriteTransaction();
+        transactionInFlight = transaction;
+        recordPendingTransaction(command, transaction);
+        command.execute(transaction);
+        ListenableFuture<Void> ft = transaction.submit();
+        command.setTransactionResultFuture(ft);
+        Futures.addCallback(ft, new FutureCallback<Void>() {
+            @Override
+            public void onSuccess(final Void result) {
+                forgetSuccessfulTransaction(transaction);
+            }
+
+            @Override
+            public void onFailure(final Throwable throwable) {
+                // NOOP - handled by failure of transaction chain
+            }
+        }, MoreExecutors.directExecutor());
+    }
+
     private void offerFailedTransaction(final AsyncTransaction<?, ?> transaction) {
         if (!failedTransactionQueue.offer(transaction)) {
             LOG.warn("failedTransactionQueue is full (size: {})", failedTransactionQueue.size());

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/TransactionInvokerImpl.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/TransactionInvokerImpl.java
index 3b469cd06abcbd77714716fc13277f69fa5cce0f..7dcdbd74fd26be24b299f1ded6f5e6f28649f7c7 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/TransactionInvokerImpl.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/TransactionInvokerImpl.java
@@ -116,39 +116,39 @@ public class TransactionInvokerImpl implements TransactionInvoker,TransactionCha
                 continue;
             }
 
-            ReadWriteTransaction transactionInFlight = null;
-            try {
-                for (TransactionCommand command: commands) {
-                    synchronized (this) {
-                        final ReadWriteTransaction transaction = chain.newReadWriteTransaction();
-                        transactionInFlight = transaction;
-                        recordPendingTransaction(command, transaction);
-                        command.execute(transaction);
-                        Futures.addCallback(transaction.submit(), new FutureCallback<Void>() {
-                            @Override
-                            public void onSuccess(final Void result) {
-                                forgetSuccessfulTransaction(transaction);
-                                command.onSuccess();
-                            }
-
-                            @Override
-                            public void onFailure(final Throwable throwable) {
-                                command.onFailure(throwable);
-                                // NOOP - handled by failure of transaction chain
-                            }
-                        }, MoreExecutors.directExecutor());
-                    }
+            commands.forEach(this::executeCommand);
+        }
+    }
+
+    private synchronized void executeCommand(final TransactionCommand command) {
+        ReadWriteTransaction transactionInFlight = null;
+        try {
+            final ReadWriteTransaction transaction = chain.newReadWriteTransaction();
+            transactionInFlight = transaction;
+            recordPendingTransaction(command, transaction);
+            command.execute(transaction);
+            Futures.addCallback(transaction.submit(), new FutureCallback<Void>() {
+                @Override
+                public void onSuccess(final Void result) {
+                    forgetSuccessfulTransaction(transaction);
+                    command.onSuccess();
                 }
-            } catch (IllegalStateException e) {
-                if (transactionInFlight != null) {
-                    // TODO: This method should distinguish exceptions on which the command should be
-                    // retried from exceptions on which the command should NOT be retried.
-                    // Then it should retry only the commands which should be retried, otherwise
-                    // this method will retry commands which will never be successful forever.
-                    offerFailedTransaction(transactionInFlight);
+
+                @Override
+                public void onFailure(final Throwable throwable) {
+                    command.onFailure(throwable);
+                    // NOOP - handled by failure of transaction chain
                 }
-                LOG.warn("Failed to process an update notification from OVS.", e);
+            }, MoreExecutors.directExecutor());
+        } catch (IllegalStateException e) {
+            if (transactionInFlight != null) {
+                // TODO: This method should distinguish exceptions on which the command should be
+                // retried from exceptions on which the command should NOT be retried.
+                // Then it should retry only the commands which should be retried, otherwise
+                // this method will retry commands which will never be successful forever.
+                offerFailedTransaction(transactionInFlight);
             }
+            LOG.warn("Failed to process an update notification from OVS.", e);
         }
     }