From 94fb90ab450470ee1b3225d737cd394f034ea932 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 20 Apr 2020 11:35:37 +0200 Subject: [PATCH] Expose completion future from WriteOperations WriteOperations (and its counterparts) does not allow code to control when a transaction is committed or aborted. This adds a layer of separation, but unfortunately also prevents the encapsulated code from reacting to when the changes are actually committed (or not). This capability is quite important for chaining cache updates and similar tasks. Expose a FluentFuture. which is guaranteed to complete when its transaction is completed -- either successfully or not. JIRA: MDSAL-61 Change-Id: Ie75671842b93fb9e63f1c2aa9ec72f25904da039 Signed-off-by: Robert Varga --- .../mdsal/binding/api/WriteOperations.java | 8 ++++ .../BindingDOMWriteTransactionAdapter.java | 5 ++ .../spi/ForwardingReadWriteTransaction.java | 5 ++ .../spi/ForwardingWriteTransaction.java | 5 ++ .../binding/util/TransactionAdapter.java | 5 ++ .../binding/util/TypedWriteTransaction.java | 11 +++++ .../util/TypedWriteTransactionImpl.java | 6 +++ .../dom/api/DOMDataTreeWriteOperations.java | 13 ++++++ .../dom/api/DOMDataTreeWriteTransaction.java | 2 +- .../broker/DOMForwardedWriteTransaction.java | 46 ++++++++++++------- .../spi/AbstractPingPongTransactionChain.java | 2 +- ...ForwardingDOMDataReadWriteTransaction.java | 5 ++ .../ForwardingDOMDataWriteTransaction.java | 5 ++ .../mdsal/dom/spi/PingPongTransaction.java | 5 +- .../spi/UncancellableListenableFuture.java | 29 ++++++++++++ .../impl/AbstractTracingWriteTransaction.java | 5 ++ 16 files changed, 136 insertions(+), 21 deletions(-) create mode 100644 dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/UncancellableListenableFuture.java diff --git a/binding/mdsal-binding-api/src/main/java/org/opendaylight/mdsal/binding/api/WriteOperations.java b/binding/mdsal-binding-api/src/main/java/org/opendaylight/mdsal/binding/api/WriteOperations.java index bd695d3659..b054d43f7b 100644 --- a/binding/mdsal-binding-api/src/main/java/org/opendaylight/mdsal/binding/api/WriteOperations.java +++ b/binding/mdsal-binding-api/src/main/java/org/opendaylight/mdsal/binding/api/WriteOperations.java @@ -7,6 +7,7 @@ */ package org.opendaylight.mdsal.binding.api; +import com.google.common.util.concurrent.FluentFuture; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; import org.opendaylight.mdsal.common.api.TransactionDatastoreMismatchException; @@ -148,4 +149,11 @@ public interface WriteOperations { * @throws TransactionDatastoreMismatchException if this transaction is already bound to a different data store */ void delete(@NonNull LogicalDatastoreType store, @NonNull InstanceIdentifier path); + + /** + * Return a {@link FluentFuture} which completes. + * + * @return A future which completes when the requested operations complete. + */ + @NonNull FluentFuture completionFuture(); } diff --git a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingDOMWriteTransactionAdapter.java b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingDOMWriteTransactionAdapter.java index 016b781d59..565f070f46 100644 --- a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingDOMWriteTransactionAdapter.java +++ b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingDOMWriteTransactionAdapter.java @@ -133,6 +133,11 @@ class BindingDOMWriteTransactionAdapter e return getDelegate().cancel(); } + @Override + public FluentFuture completionFuture() { + return getDelegate().completionFuture(); + } + /** * Subclasses of this class are required to implement creation of parent nodes based on behaviour of their * underlying transaction. diff --git a/binding/mdsal-binding-spi/src/main/java/org/opendaylight/mdsal/binding/spi/ForwardingReadWriteTransaction.java b/binding/mdsal-binding-spi/src/main/java/org/opendaylight/mdsal/binding/spi/ForwardingReadWriteTransaction.java index 8f4b12616d..31fc020421 100644 --- a/binding/mdsal-binding-spi/src/main/java/org/opendaylight/mdsal/binding/spi/ForwardingReadWriteTransaction.java +++ b/binding/mdsal-binding-spi/src/main/java/org/opendaylight/mdsal/binding/spi/ForwardingReadWriteTransaction.java @@ -84,4 +84,9 @@ public class ForwardingReadWriteTransaction extends ForwardingTransaction implem public void delete(final LogicalDatastoreType store, final InstanceIdentifier path) { delegate.delete(store, path); } + + @Override + public FluentFuture completionFuture() { + return delegate.completionFuture(); + } } diff --git a/binding/mdsal-binding-spi/src/main/java/org/opendaylight/mdsal/binding/spi/ForwardingWriteTransaction.java b/binding/mdsal-binding-spi/src/main/java/org/opendaylight/mdsal/binding/spi/ForwardingWriteTransaction.java index 4203d26515..93378266aa 100644 --- a/binding/mdsal-binding-spi/src/main/java/org/opendaylight/mdsal/binding/spi/ForwardingWriteTransaction.java +++ b/binding/mdsal-binding-spi/src/main/java/org/opendaylight/mdsal/binding/spi/ForwardingWriteTransaction.java @@ -73,4 +73,9 @@ public class ForwardingWriteTransaction extends ForwardingTransaction implements public FluentFuture commit() { return delegate.commit(); } + + @Override + public FluentFuture completionFuture() { + return delegate.completionFuture(); + } } diff --git a/binding/mdsal-binding-util/src/main/java/org/opendaylight/mdsal/binding/util/TransactionAdapter.java b/binding/mdsal-binding-util/src/main/java/org/opendaylight/mdsal/binding/util/TransactionAdapter.java index 31419dcf93..0d0c5e7bc9 100644 --- a/binding/mdsal-binding-util/src/main/java/org/opendaylight/mdsal/binding/util/TransactionAdapter.java +++ b/binding/mdsal-binding-util/src/main/java/org/opendaylight/mdsal/binding/util/TransactionAdapter.java @@ -127,6 +127,11 @@ public final class TransactionAdapter { return delegate.getIdentifier(); } + @Override + public FluentFuture completionFuture() { + return delegate.completionFuture(); + } + @Override protected D delegate() { return delegate; diff --git a/binding/mdsal-binding-util/src/main/java/org/opendaylight/mdsal/binding/util/TypedWriteTransaction.java b/binding/mdsal-binding-util/src/main/java/org/opendaylight/mdsal/binding/util/TypedWriteTransaction.java index 7782e79712..97951a1149 100644 --- a/binding/mdsal-binding-util/src/main/java/org/opendaylight/mdsal/binding/util/TypedWriteTransaction.java +++ b/binding/mdsal-binding-util/src/main/java/org/opendaylight/mdsal/binding/util/TypedWriteTransaction.java @@ -8,6 +8,8 @@ package org.opendaylight.mdsal.binding.util; import com.google.common.annotations.Beta; +import com.google.common.util.concurrent.FluentFuture; +import edu.umd.cs.findbugs.annotations.CheckReturnValue; import org.opendaylight.mdsal.binding.api.Transaction; import org.opendaylight.mdsal.binding.api.WriteOperations; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; @@ -79,4 +81,13 @@ public interface TypedWriteTransaction extends Transaction * @param path The path to delete. */ void delete(InstanceIdentifier path); + + /** + * Return a {@link FluentFuture} which completes. + * + * @return A future which completes when the requested operations complete. + */ + @Beta + @CheckReturnValue + FluentFuture completionFuture(); } diff --git a/binding/mdsal-binding-util/src/main/java/org/opendaylight/mdsal/binding/util/TypedWriteTransactionImpl.java b/binding/mdsal-binding-util/src/main/java/org/opendaylight/mdsal/binding/util/TypedWriteTransactionImpl.java index f5fa3e0700..b872606308 100644 --- a/binding/mdsal-binding-util/src/main/java/org/opendaylight/mdsal/binding/util/TypedWriteTransactionImpl.java +++ b/binding/mdsal-binding-util/src/main/java/org/opendaylight/mdsal/binding/util/TypedWriteTransactionImpl.java @@ -7,6 +7,7 @@ */ package org.opendaylight.mdsal.binding.util; +import com.google.common.util.concurrent.FluentFuture; import org.opendaylight.mdsal.binding.api.WriteTransaction; import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; @@ -55,6 +56,11 @@ class TypedWriteTransactionImpl postOperation(); } + @Override + public final FluentFuture completionFuture() { + return delegate().completionFuture(); + } + void postOperation() { // Defaults to no-op } diff --git a/dom/mdsal-dom-api/src/main/java/org/opendaylight/mdsal/dom/api/DOMDataTreeWriteOperations.java b/dom/mdsal-dom-api/src/main/java/org/opendaylight/mdsal/dom/api/DOMDataTreeWriteOperations.java index 4422f25c6f..7bb139665a 100644 --- a/dom/mdsal-dom-api/src/main/java/org/opendaylight/mdsal/dom/api/DOMDataTreeWriteOperations.java +++ b/dom/mdsal-dom-api/src/main/java/org/opendaylight/mdsal/dom/api/DOMDataTreeWriteOperations.java @@ -7,6 +7,10 @@ */ package org.opendaylight.mdsal.dom.api; +import com.google.common.annotations.Beta; +import com.google.common.util.concurrent.FluentFuture; +import edu.umd.cs.findbugs.annotations.CheckReturnValue; +import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; import org.opendaylight.mdsal.common.api.TransactionDatastoreMismatchException; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; @@ -59,4 +63,13 @@ public interface DOMDataTreeWriteOperations { * @throws TransactionDatastoreMismatchException if this transaction is already bound to a different data store */ void delete(LogicalDatastoreType store, YangInstanceIdentifier path); + + /** + * Return a {@link FluentFuture} which completes. + * + * @return A future which completes when the requested operations complete. + */ + @Beta + @CheckReturnValue + @NonNull FluentFuture completionFuture(); } diff --git a/dom/mdsal-dom-api/src/main/java/org/opendaylight/mdsal/dom/api/DOMDataTreeWriteTransaction.java b/dom/mdsal-dom-api/src/main/java/org/opendaylight/mdsal/dom/api/DOMDataTreeWriteTransaction.java index 9c06d7d589..87036e1088 100644 --- a/dom/mdsal-dom-api/src/main/java/org/opendaylight/mdsal/dom/api/DOMDataTreeWriteTransaction.java +++ b/dom/mdsal-dom-api/src/main/java/org/opendaylight/mdsal/dom/api/DOMDataTreeWriteTransaction.java @@ -443,7 +443,7 @@ public interface DOMDataTreeWriteTransaction extends DOMDataTreeTransaction, DOM /** * Cancels the transaction. Transactions can only be cancelled if it was not yet committed. - * Invoking cancel() on failed or already canceled will have no effect, and transaction is considered cancelled. + * Invoking cancel() on failed or already cancelled will have no effect, and transaction is considered cancelled. * Invoking cancel() on finished transaction (future returned by {@link #commit()} already successfully completed) * will always fail (return false). * diff --git a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/DOMForwardedWriteTransaction.java b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/DOMForwardedWriteTransaction.java index f105071113..70b1e4043f 100644 --- a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/DOMForwardedWriteTransaction.java +++ b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/DOMForwardedWriteTransaction.java @@ -12,15 +12,16 @@ import static java.util.Objects.requireNonNull; import static org.opendaylight.yangtools.util.concurrent.FluentFutures.immediateFailedFluentFuture; import com.google.common.util.concurrent.FluentFuture; -import com.google.common.util.concurrent.Futures; -import java.util.concurrent.Future; +import com.google.common.util.concurrent.SettableFuture; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import java.util.function.Function; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.mdsal.common.api.CommitInfo; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; import org.opendaylight.mdsal.dom.api.DOMDataTreeWriteTransaction; +import org.opendaylight.mdsal.dom.spi.UncancellableListenableFuture; import org.opendaylight.mdsal.dom.spi.store.DOMStoreWriteTransaction; +import org.opendaylight.yangtools.util.concurrent.FluentFutures; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.slf4j.Logger; @@ -50,10 +51,14 @@ class DOMForwardedWriteTransaction AbstractDOMForwardedTransactionFactory> IMPL_UPDATER = AtomicReferenceFieldUpdater.newUpdater( DOMForwardedWriteTransaction.class, AbstractDOMForwardedTransactionFactory.class, "commitImpl"); @SuppressWarnings("rawtypes") - private static final AtomicReferenceFieldUpdater FUTURE_UPDATER = - AtomicReferenceFieldUpdater.newUpdater(DOMForwardedWriteTransaction.class, Future.class, "commitFuture"); + private static final AtomicReferenceFieldUpdater FUTURE_UPDATER = + AtomicReferenceFieldUpdater.newUpdater(DOMForwardedWriteTransaction.class, FluentFuture.class, + "commitFuture"); private static final Logger LOG = LoggerFactory.getLogger(DOMForwardedWriteTransaction.class); - private static final Future CANCELLED_FUTURE = Futures.immediateCancelledFuture(); + + private final @NonNull SettableFuture<@NonNull CommitInfo> settableCompletion = SettableFuture.create(); + private final @NonNull FluentFuture completionFuture = FluentFuture.from( + new UncancellableListenableFuture<>(settableCompletion)); /* * Implementation of real commit. It also acts as an indication that the transaction is running -- which we flip @@ -69,7 +74,7 @@ class DOMForwardedWriteTransaction * busy-wait for it. The fast path gets the benefit of a store-store barrier instead of the usual store-load * barrier. */ - private volatile Future commitFuture; + private volatile FluentFuture commitFuture; protected DOMForwardedWriteTransaction(final Object identifier, final Function backingTxFactory, @@ -79,46 +84,47 @@ class DOMForwardedWriteTransaction } @Override - public void put(final LogicalDatastoreType store, final YangInstanceIdentifier path, final NormalizedNode data) { + public final void put(final LogicalDatastoreType store, final YangInstanceIdentifier path, + final NormalizedNode data) { checkRunning(commitImpl); getSubtransaction(store).write(path, data); } @Override - public void delete(final LogicalDatastoreType store, final YangInstanceIdentifier path) { + public final void delete(final LogicalDatastoreType store, final YangInstanceIdentifier path) { checkRunning(commitImpl); getSubtransaction(store).delete(path); } @Override - public void merge(final LogicalDatastoreType store, final YangInstanceIdentifier path, final NormalizedNode data) { + public final void merge(final LogicalDatastoreType store, final YangInstanceIdentifier path, + final NormalizedNode data) { checkRunning(commitImpl); getSubtransaction(store).merge(path, data); } @Override - public boolean cancel() { + public final boolean cancel() { final AbstractDOMForwardedTransactionFactory impl = IMPL_UPDATER.getAndSet(this, null); if (impl != null) { LOG.trace("Transaction {} cancelled before submit", getIdentifier()); - FUTURE_UPDATER.lazySet(this, CANCELLED_FUTURE); + FUTURE_UPDATER.lazySet(this, FluentFutures.immediateCancelledFluentFuture()); closeSubtransactions(); return true; } - // The transaction is in process of being submitted or cancelled. Busy-wait - // for the corresponding future. - Future future; + // The transaction is in process of being submitted or cancelled. Busy-wait for the corresponding future. + FluentFuture future; do { future = commitFuture; } while (future == null); - return future.cancel(false); + return future.isCancelled(); } @Override @SuppressWarnings("checkstyle:IllegalCatch") - public @NonNull FluentFuture commit() { + public final FluentFuture commit() { final AbstractDOMForwardedTransactionFactory impl = IMPL_UPDATER.getAndSet(this, null); checkRunning(impl); @@ -134,8 +140,14 @@ class DOMForwardedWriteTransaction } } + settableCompletion.setFuture(ret); FUTURE_UPDATER.lazySet(this, ret); - return ret; + return completionFuture; + } + + @Override + public final FluentFuture completionFuture() { + return completionFuture; } private void checkRunning(final AbstractDOMForwardedTransactionFactory impl) { diff --git a/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/AbstractPingPongTransactionChain.java b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/AbstractPingPongTransactionChain.java index 3e8ee02e21..ce4da1b258 100644 --- a/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/AbstractPingPongTransactionChain.java +++ b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/AbstractPingPongTransactionChain.java @@ -489,7 +489,7 @@ abstract class AbstractPingPongTransactionChain implements DOMTransactionChain { public FluentFuture commit() { readyTransaction(tx); isOpen = false; - return tx.getCommitFuture().transform(ignored -> CommitInfo.empty(), MoreExecutors.directExecutor()); + return tx.completionFuture(); } @Override diff --git a/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/ForwardingDOMDataReadWriteTransaction.java b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/ForwardingDOMDataReadWriteTransaction.java index ea290a829c..7fcd5125e6 100644 --- a/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/ForwardingDOMDataReadWriteTransaction.java +++ b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/ForwardingDOMDataReadWriteTransaction.java @@ -66,4 +66,9 @@ public abstract class ForwardingDOMDataReadWriteTransaction extends ForwardingOb public FluentFuture commit() { return delegate().commit(); } + + @Override + public FluentFuture completionFuture() { + return delegate().completionFuture(); + } } diff --git a/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/ForwardingDOMDataWriteTransaction.java b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/ForwardingDOMDataWriteTransaction.java index 8e748e957e..f32ac7fc39 100644 --- a/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/ForwardingDOMDataWriteTransaction.java +++ b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/ForwardingDOMDataWriteTransaction.java @@ -50,6 +50,11 @@ public abstract class ForwardingDOMDataWriteTransaction extends ForwardingObject delegate().delete(store, path); } + @Override + public FluentFuture completionFuture() { + return delegate().completionFuture(); + } + @Override public FluentFuture commit() { return delegate().commit(); diff --git a/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/PingPongTransaction.java b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/PingPongTransaction.java index b4472248ac..f55d674be3 100644 --- a/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/PingPongTransaction.java +++ b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/PingPongTransaction.java @@ -25,7 +25,8 @@ import org.opendaylight.mdsal.dom.api.DOMDataTreeReadWriteTransaction; */ final class PingPongTransaction implements FutureCallback { private final @NonNull SettableFuture future = SettableFuture.create(); - private final @NonNull FluentFuture fluent = FluentFuture.from(future); + private final @NonNull FluentFuture fluent = + FluentFuture.from(new UncancellableListenableFuture<>(future)); private final @NonNull DOMDataTreeReadWriteTransaction delegate; private @Nullable DOMDataTreeReadWriteTransaction frontendTransaction; @@ -42,7 +43,7 @@ final class PingPongTransaction implements FutureCallback { return frontendTransaction; } - @NonNull FluentFuture getCommitFuture() { + @NonNull FluentFuture completionFuture() { return fluent; } diff --git a/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/UncancellableListenableFuture.java b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/UncancellableListenableFuture.java new file mode 100644 index 0000000000..d80634bb7e --- /dev/null +++ b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/UncancellableListenableFuture.java @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2020 PANTHEON.tech, s.r.o. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.mdsal.dom.spi; + +import com.google.common.annotations.Beta; +import com.google.common.util.concurrent.ForwardingListenableFuture.SimpleForwardingListenableFuture; +import com.google.common.util.concurrent.ListenableFuture; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Beta +public final class UncancellableListenableFuture extends SimpleForwardingListenableFuture { + private static final Logger LOG = LoggerFactory.getLogger(UncancellableListenableFuture.class); + + public UncancellableListenableFuture(final ListenableFuture delegate) { + super(delegate); + } + + @Override + public boolean cancel(final boolean mayInterruptIfRunning) { + LOG.debug("Attempted to cancel future", new Throwable()); + return false; + } +} diff --git a/trace/mdsal-trace-impl/src/main/java/org/opendaylight/mdsal/trace/impl/AbstractTracingWriteTransaction.java b/trace/mdsal-trace-impl/src/main/java/org/opendaylight/mdsal/trace/impl/AbstractTracingWriteTransaction.java index 6982302cde..1955fb772c 100644 --- a/trace/mdsal-trace-impl/src/main/java/org/opendaylight/mdsal/trace/impl/AbstractTracingWriteTransaction.java +++ b/trace/mdsal-trace-impl/src/main/java/org/opendaylight/mdsal/trace/impl/AbstractTracingWriteTransaction.java @@ -106,6 +106,11 @@ abstract class AbstractTracingWriteTransaction implements DOMDataTreeWriteTransa return delegate.getIdentifier(); } + @Override + public final FluentFuture completionFuture() { + return delegate.completionFuture(); + } + // https://jira.opendaylight.org/browse/CONTROLLER-1792 @Override -- 2.36.6