From 7fde47bbf398e73144edfd14437ca72fd12e1daa Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Wed, 22 Apr 2015 16:17:57 +0200 Subject: [PATCH] IMDS: trim down commit overhead After refactoring things out it is quite obvious that we can skip adding a listener to the commit cohort when committing a chained transaction. Extract the inner class from InMemoryDOMDataStore so it can be reused and subclass it in ChainedTransactionCommitImpl. That makes it also obvious our commit cannot fail. Change-Id: I71d9fa60cf63d826ba066c381d8db91af7f8f31e Signed-off-by: Robert Varga --- .../impl/ChainedTransactionCommitImpl.java | 37 ++----- .../impl/DOMStoreTransactionChainImpl.java | 8 +- .../dom/store/impl/InMemoryDOMDataStore.java | 101 +++--------------- ...nMemoryDOMStoreThreePhaseCommitCohort.java | 100 +++++++++++++++++ 4 files changed, 122 insertions(+), 124 deletions(-) create mode 100644 opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMStoreThreePhaseCommitCohort.java diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChainedTransactionCommitImpl.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChainedTransactionCommitImpl.java index ff55c01a25..35d891dac0 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChainedTransactionCommitImpl.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChainedTransactionCommitImpl.java @@ -8,45 +8,24 @@ package org.opendaylight.controller.md.sal.dom.store.impl; import com.google.common.base.Preconditions; -import com.google.common.util.concurrent.FutureCallback; -import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; -import org.opendaylight.controller.sal.core.spi.data.DOMStoreThreePhaseCommitCohort; -import org.opendaylight.controller.sal.core.spi.data.ForwardingDOMStoreThreePhaseCommitCohort; import org.opendaylight.controller.sal.core.spi.data.SnapshotBackedWriteTransaction; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; -final class ChainedTransactionCommitImpl extends ForwardingDOMStoreThreePhaseCommitCohort { - private final SnapshotBackedWriteTransaction transaction; - private final DOMStoreThreePhaseCommitCohort delegate; +final class ChainedTransactionCommitImpl extends InMemoryDOMStoreThreePhaseCommitCohort { private final DOMStoreTransactionChainImpl txChain; - ChainedTransactionCommitImpl(final SnapshotBackedWriteTransaction transaction, - final DOMStoreThreePhaseCommitCohort delegate, final DOMStoreTransactionChainImpl txChain) { - this.transaction = Preconditions.checkNotNull(transaction); - this.delegate = Preconditions.checkNotNull(delegate); + ChainedTransactionCommitImpl(final InMemoryDOMDataStore store, final SnapshotBackedWriteTransaction transaction, + final DataTreeModification modification, final DOMStoreTransactionChainImpl txChain) { + super(store, transaction, modification); this.txChain = Preconditions.checkNotNull(txChain); } - @Override - protected DOMStoreThreePhaseCommitCohort delegate() { - return delegate; - } - @Override public ListenableFuture commit() { - ListenableFuture commitFuture = super.commit(); - Futures.addCallback(commitFuture, new FutureCallback() { - @Override - public void onFailure(final Throwable t) { - txChain.transactionFailed(transaction, t); - } - - @Override - public void onSuccess(final Void result) { - txChain.transactionCommited(transaction); - } - }); - return commitFuture; + ListenableFuture ret = super.commit(); + txChain.transactionCommited(getTransaction()); + return ret; } } \ No newline at end of file diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMStoreTransactionChainImpl.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMStoreTransactionChainImpl.java index e357c8dc9c..2cf79d899b 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMStoreTransactionChainImpl.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMStoreTransactionChainImpl.java @@ -22,8 +22,8 @@ final class DOMStoreTransactionChainImpl extends AbstractSnapshotBackedTransacti } @Override - protected DOMStoreThreePhaseCommitCohort createCohort(final SnapshotBackedWriteTransaction tx, final DataTreeModification tree) { - return new ChainedTransactionCommitImpl(tx, store.transactionReady(tx, tree), this); + protected DOMStoreThreePhaseCommitCohort createCohort(final SnapshotBackedWriteTransaction tx, final DataTreeModification modification) { + return new ChainedTransactionCommitImpl(store, tx, modification, this); } @Override @@ -41,10 +41,6 @@ final class DOMStoreTransactionChainImpl extends AbstractSnapshotBackedTransacti return store.getDebugTransactions(); } - void transactionFailed(final SnapshotBackedWriteTransaction transaction, final Throwable cause) { - super.onTransactionFailed(transaction, cause); - } - void transactionCommited(final SnapshotBackedWriteTransaction transaction) { super.onTransactionCommited(transaction); } diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java index 7e4056a190..a85d8ac3fb 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java @@ -7,21 +7,15 @@ */ package org.opendaylight.controller.md.sal.dom.store.impl; -import static com.google.common.base.Preconditions.checkState; import com.google.common.base.Optional; import com.google.common.base.Preconditions; -import com.google.common.util.concurrent.Futures; -import com.google.common.util.concurrent.ListenableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import org.opendaylight.controller.md.sal.common.api.data.AsyncDataBroker.DataChangeScope; import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeListener; -import org.opendaylight.controller.md.sal.common.api.data.OptimisticLockFailedException; -import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; import org.opendaylight.controller.md.sal.dom.api.DOMDataTreeChangeListener; import org.opendaylight.controller.md.sal.dom.store.impl.tree.ListenerTree; -import org.opendaylight.controller.sal.core.spi.data.AbstractDOMStoreTransaction; import org.opendaylight.controller.sal.core.spi.data.DOMStore; import org.opendaylight.controller.sal.core.spi.data.DOMStoreReadTransaction; import org.opendaylight.controller.sal.core.spi.data.DOMStoreReadWriteTransaction; @@ -40,7 +34,6 @@ import org.opendaylight.yangtools.util.concurrent.QueuedNotificationManager; import org.opendaylight.yangtools.util.concurrent.QueuedNotificationManager.Invoker; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.tree.ConflictingModificationAppliedException; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; @@ -63,8 +56,6 @@ import org.slf4j.LoggerFactory; */ public class InMemoryDOMDataStore extends TransactionReadyPrototype implements DOMStore, Identifiable, SchemaContextListener, AutoCloseable, DOMStoreTreeChangePublisher { private static final Logger LOG = LoggerFactory.getLogger(InMemoryDOMDataStore.class); - private static final ListenableFuture SUCCESSFUL_FUTURE = Futures.immediateFuture(null); - private static final ListenableFuture CAN_COMMIT_FUTURE = Futures.immediateFuture(Boolean.TRUE); private static final Invoker, DOMImmutableDataChangeEvent> DCL_NOTIFICATION_MGR_INVOKER = new Invoker, DOMImmutableDataChangeEvent>() { @@ -221,94 +212,26 @@ public class InMemoryDOMDataStore extends TransactionReadyPrototype impl } @Override - protected DOMStoreThreePhaseCommitCohort transactionReady(final SnapshotBackedWriteTransaction tx, final DataTreeModification tree) { - LOG.debug("Tx: {} is submitted. Modifications: {}", tx.getIdentifier(), tree); - return new ThreePhaseCommitImpl(tx, tree); + protected DOMStoreThreePhaseCommitCohort transactionReady(final SnapshotBackedWriteTransaction tx, final DataTreeModification modification) { + LOG.debug("Tx: {} is submitted. Modifications: {}", tx.getIdentifier(), modification); + return new InMemoryDOMStoreThreePhaseCommitCohort(this, tx, modification); } String nextIdentifier() { return name + "-" + txCounter.getAndIncrement(); } - private static void warnDebugContext(final AbstractDOMStoreTransaction transaction) { - final Throwable ctx = transaction.getDebugContext(); - if (ctx != null) { - LOG.warn("Transaction {} has been allocated in the following context", transaction.getIdentifier(), ctx); - } + void validate(final DataTreeModification modification) throws DataValidationFailedException { + dataTree.validate(modification); } - private final class ThreePhaseCommitImpl implements DOMStoreThreePhaseCommitCohort { - private final SnapshotBackedWriteTransaction transaction; - private final DataTreeModification modification; - - private ResolveDataChangeEventsTask listenerResolver; - private DataTreeCandidate candidate; - - public ThreePhaseCommitImpl(final SnapshotBackedWriteTransaction writeTransaction, final DataTreeModification modification) { - this.transaction = writeTransaction; - this.modification = modification; - } - - @Override - public ListenableFuture canCommit() { - try { - dataTree.validate(modification); - LOG.debug("Store Transaction: {} can be committed", transaction.getIdentifier()); - return CAN_COMMIT_FUTURE; - } catch (ConflictingModificationAppliedException e) { - LOG.warn("Store Tx: {} Conflicting modification for {}.", transaction.getIdentifier(), - e.getPath()); - warnDebugContext(transaction); - return Futures.immediateFailedFuture(new OptimisticLockFailedException("Optimistic lock failed.", e)); - } catch (DataValidationFailedException e) { - LOG.warn("Store Tx: {} Data Precondition failed for {}.", transaction.getIdentifier(), - e.getPath(), e); - warnDebugContext(transaction); - - // For debugging purposes, allow dumping of the modification. Coupled with the above - // precondition log, it should allow us to understand what went on. - LOG.trace("Store Tx: {} modifications: {} tree: {}", modification, dataTree); - - return Futures.immediateFailedFuture(new TransactionCommitFailedException("Data did not pass validation.", e)); - } catch (Exception e) { - LOG.warn("Unexpected failure in validation phase", e); - return Futures.immediateFailedFuture(e); - } - } - - @Override - public ListenableFuture preCommit() { - try { - candidate = dataTree.prepare(modification); - listenerResolver = ResolveDataChangeEventsTask.create(candidate, listenerTree); - return SUCCESSFUL_FUTURE; - } catch (Exception e) { - LOG.warn("Unexpected failure in pre-commit phase", e); - return Futures.immediateFailedFuture(e); - } - } - - @Override - public ListenableFuture abort() { - candidate = null; - return SUCCESSFUL_FUTURE; - } - - @Override - public ListenableFuture commit() { - checkState(candidate != null, "Proposed subtree must be computed"); - - /* - * The commit has to occur atomically with regard to listener - * registrations. - */ - synchronized (InMemoryDOMDataStore.this) { - dataTree.commit(candidate); - changePublisher.publishChange(candidate); - listenerResolver.resolve(dataChangeListenerNotificationManager); - } + DataTreeCandidate prepare(final DataTreeModification modification) { + return dataTree.prepare(modification); + } - return SUCCESSFUL_FUTURE; - } + synchronized void commit(final DataTreeCandidate candidate) { + dataTree.commit(candidate); + changePublisher.publishChange(candidate); + ResolveDataChangeEventsTask.create(candidate, listenerTree).resolve(dataChangeListenerNotificationManager); } } diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMStoreThreePhaseCommitCohort.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMStoreThreePhaseCommitCohort.java new file mode 100644 index 0000000000..dba71bff4c --- /dev/null +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMStoreThreePhaseCommitCohort.java @@ -0,0 +1,100 @@ +package org.opendaylight.controller.md.sal.dom.store.impl; + +import static com.google.common.base.Preconditions.checkState; +import com.google.common.base.Preconditions; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import org.opendaylight.controller.md.sal.common.api.data.OptimisticLockFailedException; +import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; +import org.opendaylight.controller.sal.core.spi.data.AbstractDOMStoreTransaction; +import org.opendaylight.controller.sal.core.spi.data.DOMStoreThreePhaseCommitCohort; +import org.opendaylight.controller.sal.core.spi.data.SnapshotBackedWriteTransaction; +import org.opendaylight.yangtools.yang.data.api.schema.tree.ConflictingModificationAppliedException; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +class InMemoryDOMStoreThreePhaseCommitCohort implements DOMStoreThreePhaseCommitCohort { + private static final Logger LOG = LoggerFactory.getLogger(InMemoryDOMStoreThreePhaseCommitCohort.class); + private static final ListenableFuture SUCCESSFUL_FUTURE = Futures.immediateFuture(null); + private static final ListenableFuture CAN_COMMIT_FUTURE = Futures.immediateFuture(Boolean.TRUE); + private final SnapshotBackedWriteTransaction transaction; + private final DataTreeModification modification; + private final InMemoryDOMDataStore store; + private DataTreeCandidate candidate; + + public InMemoryDOMStoreThreePhaseCommitCohort(final InMemoryDOMDataStore store, final SnapshotBackedWriteTransaction writeTransaction, final DataTreeModification modification) { + this.transaction = Preconditions.checkNotNull(writeTransaction); + this.modification = Preconditions.checkNotNull(modification); + this.store = Preconditions.checkNotNull(store); + } + + private static void warnDebugContext(final AbstractDOMStoreTransaction transaction) { + final Throwable ctx = transaction.getDebugContext(); + if (ctx != null) { + LOG.warn("Transaction {} has been allocated in the following context", transaction.getIdentifier(), ctx); + } + } + + @Override + public final ListenableFuture canCommit() { + try { + store.validate(modification); + LOG.debug("Store Transaction: {} can be committed", getTransaction().getIdentifier()); + return CAN_COMMIT_FUTURE; + } catch (ConflictingModificationAppliedException e) { + LOG.warn("Store Tx: {} Conflicting modification for {}.", getTransaction().getIdentifier(), + e.getPath()); + warnDebugContext(getTransaction()); + return Futures.immediateFailedFuture(new OptimisticLockFailedException("Optimistic lock failed.", e)); + } catch (DataValidationFailedException e) { + LOG.warn("Store Tx: {} Data Precondition failed for {}.", getTransaction().getIdentifier(), + e.getPath(), e); + warnDebugContext(getTransaction()); + + // For debugging purposes, allow dumping of the modification. Coupled with the above + // precondition log, it should allow us to understand what went on. + LOG.trace("Store Tx: {} modifications: {} tree: {}", modification, store); + + return Futures.immediateFailedFuture(new TransactionCommitFailedException("Data did not pass validation.", e)); + } catch (Exception e) { + LOG.warn("Unexpected failure in validation phase", e); + return Futures.immediateFailedFuture(e); + } + } + + @Override + public final ListenableFuture preCommit() { + try { + candidate = store.prepare(modification); + return SUCCESSFUL_FUTURE; + } catch (Exception e) { + LOG.warn("Unexpected failure in pre-commit phase", e); + return Futures.immediateFailedFuture(e); + } + } + + @Override + public final ListenableFuture abort() { + candidate = null; + return SUCCESSFUL_FUTURE; + } + + protected final SnapshotBackedWriteTransaction getTransaction() { + return transaction; + } + + @Override + public ListenableFuture commit() { + checkState(candidate != null, "Proposed subtree must be computed"); + + /* + * The commit has to occur atomically with regard to listener + * registrations. + */ + store.commit(candidate); + return SUCCESSFUL_FUTURE; + } +} \ No newline at end of file -- 2.36.6