From: Robert Varga <rovarga@cisco.com>
Date: Fri, 16 Sep 2016 20:20:43 +0000 (+0200)
Subject: Improve ShardedDOMDataTreeProducer locking
X-Git-Tag: release/boron-sr1~26
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F33%2F45833%2F1;p=mdsal.git

Improve ShardedDOMDataTreeProducer locking

The path from backend was taking coarse lock even when not needed.
Explicitly annotate submitTransaction() as needing the lock being
held and push down its acquisition so we take it only after we
are certain we actually need it.

Change-Id: Icd1226796568829ea3735e6eec42677a79b9b3b5
Signed-off-by: Robert Varga <rovarga@cisco.com>
(cherry picked from commit 2ce13c8680d68ba1e50eab9609069b6aecfa62a5)
---

diff --git a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeProducer.java b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeProducer.java
index b19d956192..773bd27b87 100644
--- a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeProducer.java
+++ b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeProducer.java
@@ -47,7 +47,6 @@ class ShardedDOMDataTreeProducer implements DOMDataTreeProducer {
     private static final AtomicReferenceFieldUpdater<ShardedDOMDataTreeProducer, ShardedDOMDataTreeWriteTransaction>
         CURRENT_UPDATER = AtomicReferenceFieldUpdater.newUpdater(ShardedDOMDataTreeProducer.class,
             ShardedDOMDataTreeWriteTransaction.class, "currentTx");
-    @SuppressWarnings("unused")
     private volatile ShardedDOMDataTreeWriteTransaction currentTx;
 
     private static final AtomicReferenceFieldUpdater<ShardedDOMDataTreeProducer, ShardedDOMDataTreeWriteTransaction>
@@ -155,6 +154,7 @@ class ShardedDOMDataTreeProducer implements DOMDataTreeProducer {
         return ret;
     }
 
+    @GuardedBy("this")
     private void submitTransaction(final ShardedDOMDataTreeWriteTransaction current) {
         lastTx = current;
         current.doSubmit(this::transactionSuccessful, this::transactionFailed);
@@ -251,47 +251,61 @@ class ShardedDOMDataTreeProducer implements DOMDataTreeProducer {
         }
     }
 
-    void processTransaction(final ShardedDOMDataTreeWriteTransaction transaction) {
+    // Called when the user submits a transaction
+    void transactionSubmitted(final ShardedDOMDataTreeWriteTransaction transaction) {
         final boolean wasOpen = OPEN_UPDATER.compareAndSet(this, transaction, null);
-        Verify.verify(wasOpen);
-
-        if (lastTx != null) {
-            final boolean success = CURRENT_UPDATER.compareAndSet(this, null, transaction);
-            Verify.verify(success);
-            if (lastTx == null) {
-                // Dispatch after requeue
-                processCurrentTransaction();
+        Preconditions.checkState(wasOpen, "Attempted to submit non-open transaction %s", transaction);
+
+        if (lastTx == null) {
+            // No transaction outstanding, we need to submit it now
+            synchronized (this) {
+                submitTransaction(transaction);
             }
-        } else {
-            submitTransaction(transaction);
+
+            return;
+        }
+
+        // There is a potentially-racing submitted transaction. Publish the new one, which may end up being
+        // picked up by processNextTransaction.
+        final boolean success = CURRENT_UPDATER.compareAndSet(this, null, transaction);
+        Verify.verify(success);
+
+        // Now a quick check: if the racing transaction completed in between, it may have missed the current
+        // transaction, hence we need to re-check
+        if (lastTx == null) {
+            submitCurrentTransaction();
         }
     }
 
-    void transactionSuccessful(final ShardedDOMDataTreeWriteTransaction tx) {
+    private void submitCurrentTransaction() {
+        final ShardedDOMDataTreeWriteTransaction current = currentTx;
+        if (current != null) {
+            synchronized (this) {
+                if (CURRENT_UPDATER.compareAndSet(this, current, null)) {
+                    submitTransaction(current);
+                }
+            }
+        }
+    }
+
+    private void transactionSuccessful(final ShardedDOMDataTreeWriteTransaction tx) {
         LOG.debug("Transaction {} completed successfully", tx.getIdentifier());
 
         tx.onTransactionSuccess(null);
-        processNextTransaction(tx);
+        transactionCompleted(tx);
     }
 
-    void transactionFailed(final ShardedDOMDataTreeWriteTransaction tx, final Throwable throwable) {
+    private void transactionFailed(final ShardedDOMDataTreeWriteTransaction tx, final Throwable throwable) {
         LOG.debug("Transaction {} failed", tx.getIdentifier(), throwable);
 
         tx.onTransactionFailure(throwable);
-        processNextTransaction(tx);
-    }
-
-    private void processCurrentTransaction() {
-        final ShardedDOMDataTreeWriteTransaction current = CURRENT_UPDATER.getAndSet(this, null);
-        if (current != null) {
-            submitTransaction(current);
-        }
+        transactionCompleted(tx);
     }
 
-    private synchronized void processNextTransaction(final ShardedDOMDataTreeWriteTransaction tx) {
+    private void transactionCompleted(final ShardedDOMDataTreeWriteTransaction tx) {
         final boolean wasLast = LAST_UPDATER.compareAndSet(this, tx, null);
         if (wasLast) {
-            processCurrentTransaction();
+            submitCurrentTransaction();
         }
     }
 
diff --git a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeWriteTransaction.java b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeWriteTransaction.java
index 83080f7be1..fcbc9169fc 100644
--- a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeWriteTransaction.java
+++ b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeWriteTransaction.java
@@ -125,7 +125,7 @@ final class ShardedDOMDataTreeWriteTransaction implements DOMDataTreeCursorAware
         Preconditions.checkState(!closed, "Transaction %s is already closed", identifier);
         Preconditions.checkState(openCursor == null, "Cannot submit transaction while there is a cursor open");
 
-        producer.processTransaction(this);
+        producer.transactionSubmitted(this);
         return submitFuture;
     }