Improve ShardedDOMDataTreeProducer locking

[mdsal.git] / dom / mdsal-dom-broker / src / main / java / org / opendaylight / mdsal / dom / broker / ShardedDOMDataTreeProducer.java

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 b19d956192f23abd9f992b075a98fd272e1c0678..773bd27b87170652a385cd4f1dac7ef3c9f8c5e9 100644 (file)
--- 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();
         }
     }