From 8edd92f999e5ff4588d284c3a1ae553549b84fb4 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Wed, 1 Apr 2015 11:56:55 +0200 Subject: [PATCH] Move AbstractDOMStoreTransaction into SPI It is a useful base for DOMStoreTransaction implementations, with some debugging capabilities. Publish it as a beta SPI. Change-Id: Iea3551720b2a8df7c268ce003147fc3a734f3871 Signed-off-by: Robert Varga --- .../cluster/datastore/TransactionProxy.java | 65 +++++++++---------- .../data}/AbstractDOMStoreTransaction.java | 42 ++++++++---- .../dom/store/impl/InMemoryDOMDataStore.java | 12 +++- .../impl/SnapshotBackedReadTransaction.java | 5 +- .../impl/SnapshotBackedWriteTransaction.java | 3 +- 5 files changed, 73 insertions(+), 54 deletions(-) rename opendaylight/md-sal/{sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl => sal-dom-spi/src/main/java/org/opendaylight/controller/sal/core/spi/data}/AbstractDOMStoreTransaction.java (55%) diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionProxy.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionProxy.java index f1ba4eabb9..a7effbcf37 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionProxy.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionProxy.java @@ -41,6 +41,7 @@ import org.opendaylight.controller.cluster.datastore.messages.CreateTransactionR import org.opendaylight.controller.cluster.datastore.shardstrategy.ShardStrategyFactory; import org.opendaylight.controller.cluster.datastore.utils.ActorContext; import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException; +import org.opendaylight.controller.sal.core.spi.data.AbstractDOMStoreTransaction; import org.opendaylight.controller.sal.core.spi.data.DOMStoreReadWriteTransaction; import org.opendaylight.controller.sal.core.spi.data.DOMStoreThreePhaseCommitCohort; import org.opendaylight.yangtools.util.concurrent.MappingCheckedFuture; @@ -65,7 +66,7 @@ import scala.concurrent.duration.FiniteDuration; * shards will be executed. *

*/ -public class TransactionProxy implements DOMStoreReadWriteTransaction { +public class TransactionProxy extends AbstractDOMStoreTransaction implements DOMStoreReadWriteTransaction { public static enum TransactionType { READ_ONLY, @@ -147,7 +148,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { remoteTransactionActors = referent.remoteTransactionActors; remoteTransactionActorsMB = referent.remoteTransactionActorsMB; actorContext = referent.actorContext; - identifier = referent.identifier; + identifier = referent.getIdentifier(); } @Override @@ -186,7 +187,6 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { private final TransactionType transactionType; private final ActorContext actorContext; - private final TransactionIdentifier identifier; private final String transactionChainId; private final SchemaContext schemaContext; private boolean inReadyState; @@ -199,8 +199,8 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { this(actorContext, transactionType, ""); } - public TransactionProxy(ActorContext actorContext, TransactionType transactionType, - String transactionChainId) { + public TransactionProxy(ActorContext actorContext, TransactionType transactionType, String transactionChainId) { + super(createIdentifier(actorContext)); this.actorContext = Preconditions.checkNotNull(actorContext, "actorContext should not be null"); this.transactionType = Preconditions.checkNotNull(transactionType, @@ -209,14 +209,16 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { "schemaContext should not be null"); this.transactionChainId = transactionChainId; + LOG.debug("Created txn {} of type {} on chain {}", getIdentifier(), transactionType, transactionChainId); + } + + private static TransactionIdentifier createIdentifier(ActorContext actorContext) { String memberName = actorContext.getCurrentMemberName(); - if(memberName == null){ + if (memberName == null) { memberName = "UNKNOWN-MEMBER"; } - this.identifier = new TransactionIdentifier(memberName, counter.getAndIncrement()); - - LOG.debug("Created txn {} of type {} on chain {}", identifier, transactionType, transactionChainId); + return new TransactionIdentifier(memberName, counter.getAndIncrement()); } @VisibleForTesting @@ -250,7 +252,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { Preconditions.checkState(transactionType != TransactionType.WRITE_ONLY, "Read operation on write-only transaction is not allowed"); - LOG.debug("Tx {} read {}", identifier, path); + LOG.debug("Tx {} read {}", getIdentifier(), path); throttleOperation(); @@ -273,7 +275,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { Preconditions.checkState(transactionType != TransactionType.WRITE_ONLY, "Exists operation on write-only transaction is not allowed"); - LOG.debug("Tx {} exists {}", identifier, path); + LOG.debug("Tx {} exists {}", getIdentifier(), path); throttleOperation(); @@ -332,7 +334,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { checkModificationState(); - LOG.debug("Tx {} write {}", identifier, path); + LOG.debug("Tx {} write {}", getIdentifier(), path); throttleOperation(); @@ -350,7 +352,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { checkModificationState(); - LOG.debug("Tx {} merge {}", identifier, path); + LOG.debug("Tx {} merge {}", getIdentifier(), path); throttleOperation(); @@ -368,7 +370,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { checkModificationState(); - LOG.debug("Tx {} delete {}", identifier, path); + LOG.debug("Tx {} delete {}", getIdentifier(), path); throttleOperation(); @@ -388,7 +390,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { inReadyState = true; - LOG.debug("Tx {} Readying {} transactions for commit", identifier, + LOG.debug("Tx {} Readying {} transactions for commit", getIdentifier(), txFutureCallbackMap.size()); if(txFutureCallbackMap.size() == 0) { @@ -403,7 +405,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { for(TransactionFutureCallback txFutureCallback : txFutureCallbackMap.values()) { - LOG.debug("Tx {} Readying transaction for shard {} chain {}", identifier, + LOG.debug("Tx {} Readying transaction for shard {} chain {}", getIdentifier(), txFutureCallback.getShardName(), transactionChainId); final TransactionContext transactionContext = txFutureCallback.getTransactionContext(); @@ -428,7 +430,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { onTransactionReady(cohortFutures); return new ThreePhaseCommitCohortProxy(actorContext, cohortFutures, - identifier.toString()); + getIdentifier().toString()); } /** @@ -439,11 +441,6 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { protected void onTransactionReady(List> cohortFutures) { } - @Override - public Object getIdentifier() { - return this.identifier; - } - @Override public void close() { for (TransactionFutureCallback txFutureCallback : txFutureCallbackMap.values()) { @@ -568,7 +565,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { if(transactionType == TransactionType.WRITE_ONLY && actorContext.getDatastoreContext().isWriteOnlyTransactionOptimizationsEnabled()) { LOG.debug("Tx {} Primary shard {} found - creating WRITE_ONLY transaction context", - identifier, primaryShard); + getIdentifier(), primaryShard); // For write-only Tx's we prepare the transaction modifications directly on the shard actor // to avoid the overhead of creating a separate transaction actor. @@ -587,7 +584,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { boolean invokeOperation = true; synchronized(txOperationsOnComplete) { if(transactionContext == null) { - LOG.debug("Tx {} Adding operation on complete", identifier); + LOG.debug("Tx {} Adding operation on complete", getIdentifier()); invokeOperation = false; txOperationsOnComplete.add(operation); @@ -615,10 +612,10 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { */ private void tryCreateTransaction() { if(LOG.isDebugEnabled()) { - LOG.debug("Tx {} Primary shard {} found - trying create transaction", identifier, primaryShard); + LOG.debug("Tx {} Primary shard {} found - trying create transaction", getIdentifier(), primaryShard); } - Object serializedCreateMessage = new CreateTransaction(identifier.toString(), + Object serializedCreateMessage = new CreateTransaction(getIdentifier().toString(), TransactionProxy.this.transactionType.ordinal(), getTransactionChainId()).toSerializable(); @@ -636,7 +633,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { // is ok. if(--createTxTries > 0) { LOG.debug("Tx {} Shard {} has no leader yet - scheduling create Tx retry", - identifier, shardName); + getIdentifier(), shardName); actorContext.getActorSystem().scheduler().scheduleOnce(CREATE_TX_TRY_INTERVAL, new Runnable() { @@ -668,9 +665,9 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { // TransactionContext until after we've executed all cached TransactionOperations. TransactionContext localTransactionContext; if(failure != null) { - LOG.debug("Tx {} Creating NoOpTransaction because of error", identifier, failure); + LOG.debug("Tx {} Creating NoOpTransaction because of error", getIdentifier(), failure); - localTransactionContext = new NoOpTransactionContext(failure, identifier, operationLimiter); + localTransactionContext = new NoOpTransactionContext(failure, getIdentifier(), operationLimiter); } else if (response.getClass().equals(CreateTransactionReply.SERIALIZABLE_CLASS)) { localTransactionContext = createValidTransactionContext( CreateTransactionReply.fromSerializable(response)); @@ -678,7 +675,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { IllegalArgumentException exception = new IllegalArgumentException(String.format( "Invalid reply type %s for CreateTransaction", response.getClass())); - localTransactionContext = new NoOpTransactionContext(exception, identifier, operationLimiter); + localTransactionContext = new NoOpTransactionContext(exception, getIdentifier(), operationLimiter); } executeTxOperatonsOnComplete(localTransactionContext); @@ -718,7 +715,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { } private TransactionContext createValidTransactionContext(CreateTransactionReply reply) { - LOG.debug("Tx {} Received {}", identifier, reply); + LOG.debug("Tx {} Received {}", getIdentifier(), reply); return createValidTransactionContext(actorContext.actorSelection(reply.getTransactionPath()), reply.getTransactionPath(), reply.getVersion()); @@ -755,15 +752,15 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { boolean isTxActorLocal = actorContext.isPathLocal(transactionPath); if(remoteTransactionVersion < DataStoreVersions.LITHIUM_VERSION) { - return new PreLithiumTransactionContextImpl(transactionPath, transactionActor, identifier, + return new PreLithiumTransactionContextImpl(transactionPath, transactionActor, getIdentifier(), transactionChainId, actorContext, schemaContext, isTxActorLocal, remoteTransactionVersion, operationCompleter); } else if (transactionType == TransactionType.WRITE_ONLY && actorContext.getDatastoreContext().isWriteOnlyTransactionOptimizationsEnabled()) { - return new WriteOnlyTransactionContextImpl(transactionActor, identifier, transactionChainId, + return new WriteOnlyTransactionContextImpl(transactionActor, getIdentifier(), transactionChainId, actorContext, schemaContext, isTxActorLocal, remoteTransactionVersion, operationCompleter); } else { - return new TransactionContextImpl(transactionActor, identifier, transactionChainId, + return new TransactionContextImpl(transactionActor, getIdentifier(), transactionChainId, actorContext, schemaContext, isTxActorLocal, remoteTransactionVersion, operationCompleter); } } diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/AbstractDOMStoreTransaction.java b/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/sal/core/spi/data/AbstractDOMStoreTransaction.java similarity index 55% rename from opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/AbstractDOMStoreTransaction.java rename to opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/sal/core/spi/data/AbstractDOMStoreTransaction.java index 8d040f612e..f043d2cb84 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/AbstractDOMStoreTransaction.java +++ b/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/sal/core/spi/data/AbstractDOMStoreTransaction.java @@ -5,38 +5,52 @@ * 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.controller.md.sal.dom.store.impl; +package org.opendaylight.controller.sal.core.spi.data; +import com.google.common.annotations.Beta; import com.google.common.base.MoreObjects; import com.google.common.base.MoreObjects.ToStringHelper; import com.google.common.base.Preconditions; -import org.opendaylight.controller.sal.core.spi.data.DOMStoreTransaction; -import org.slf4j.Logger; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** - * Abstract DOM Store Transaction + * Abstract DOM Store Transaction. * * Convenience super implementation of DOM Store transaction which provides * common implementation of {@link #toString()} and {@link #getIdentifier()}. + * + * It can optionally capture the context where it was allocated. + * + * identifier type */ -abstract class AbstractDOMStoreTransaction implements DOMStoreTransaction { +@Beta +public abstract class AbstractDOMStoreTransaction implements DOMStoreTransaction { private final Throwable debugContext; - private final Object identifier; + private final T identifier; + + protected AbstractDOMStoreTransaction(@Nonnull final T identifier) { + this(identifier, false); + } - protected AbstractDOMStoreTransaction(final Object identifier, final boolean debug) { + protected AbstractDOMStoreTransaction(@Nonnull final T identifier, final boolean debug) { this.identifier = Preconditions.checkNotNull(identifier, "Identifier must not be null."); this.debugContext = debug ? new Throwable().fillInStackTrace() : null; } @Override - public final Object getIdentifier() { + public final T getIdentifier() { return identifier; } - protected final void warnDebugContext(final Logger logger) { - if (debugContext != null) { - logger.warn("Transaction {} has been allocated in the following context", identifier, debugContext); - } + /** + * Return the context in which this transaction was allocated. + * + * @return The context in which this transaction was allocated, or null + * if the context was not recorded. + */ + @Nullable public final Throwable getDebugContext() { + return debugContext; } @Override @@ -51,7 +65,7 @@ abstract class AbstractDOMStoreTransaction implements DOMStoreTransaction { * ToStringHelper instance * @return ToStringHelper instance which was passed in */ - protected ToStringHelper addToStringAttributes(final ToStringHelper toStringHelper) { + protected ToStringHelper addToStringAttributes(@Nonnull final ToStringHelper toStringHelper) { return toStringHelper.add("id", identifier); } -} \ 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/InMemoryDOMDataStore.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java index 1f85b473fe..b617a8087f 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 @@ -22,6 +22,7 @@ import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFaile import org.opendaylight.controller.md.sal.dom.api.DOMDataTreeChangeListener; import org.opendaylight.controller.md.sal.dom.store.impl.SnapshotBackedWriteTransaction.TransactionReadyPrototype; 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; @@ -223,6 +224,13 @@ public class InMemoryDOMDataStore extends TransactionReadyPrototype implements D return name + "-" + txCounter.getAndIncrement(); } + private static void warnDebugContext(AbstractDOMStoreTransaction transaction) { + final Throwable ctx = transaction.getDebugContext(); + if (ctx != null) { + LOG.warn("Transaction {} has been allocated in the following context", transaction.getIdentifier(), ctx); + } + } + private final class ThreePhaseCommitImpl implements DOMStoreThreePhaseCommitCohort { private final SnapshotBackedWriteTransaction transaction; private final DataTreeModification modification; @@ -244,12 +252,12 @@ public class InMemoryDOMDataStore extends TransactionReadyPrototype implements D } catch (ConflictingModificationAppliedException e) { LOG.warn("Store Tx: {} Conflicting modification for {}.", transaction.getIdentifier(), e.getPath()); - transaction.warnDebugContext(LOG); + 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); - transaction.warnDebugContext(LOG); + 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. diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/SnapshotBackedReadTransaction.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/SnapshotBackedReadTransaction.java index ed95796499..8b18be432a 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/SnapshotBackedReadTransaction.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/SnapshotBackedReadTransaction.java @@ -8,13 +8,12 @@ package org.opendaylight.controller.md.sal.dom.store.impl; import static com.google.common.base.Preconditions.checkNotNull; - import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; - import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException; +import org.opendaylight.controller.sal.core.spi.data.AbstractDOMStoreTransaction; import org.opendaylight.controller.sal.core.spi.data.DOMStoreReadTransaction; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; @@ -30,7 +29,7 @@ import org.slf4j.LoggerFactory; * which delegates most of its calls to similar methods provided by underlying snapshot. * */ -final class SnapshotBackedReadTransaction extends AbstractDOMStoreTransaction +final class SnapshotBackedReadTransaction extends AbstractDOMStoreTransaction implements DOMStoreReadTransaction { private static final Logger LOG = LoggerFactory.getLogger(SnapshotBackedReadTransaction.class); diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/SnapshotBackedWriteTransaction.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/SnapshotBackedWriteTransaction.java index faddbae850..10e3a7df10 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/SnapshotBackedWriteTransaction.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/SnapshotBackedWriteTransaction.java @@ -13,6 +13,7 @@ import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.base.Throwables; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; +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.DOMStoreWriteTransaction; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; @@ -28,7 +29,7 @@ import org.slf4j.LoggerFactory; * {@link org.opendaylight.controller.md.sal.dom.store.impl.SnapshotBackedWriteTransaction.TransactionReadyPrototype}. * */ -class SnapshotBackedWriteTransaction extends AbstractDOMStoreTransaction implements DOMStoreWriteTransaction { +class SnapshotBackedWriteTransaction extends AbstractDOMStoreTransaction implements DOMStoreWriteTransaction { private static final Logger LOG = LoggerFactory.getLogger(SnapshotBackedWriteTransaction.class); private static final AtomicReferenceFieldUpdater READY_UPDATER = AtomicReferenceFieldUpdater.newUpdater(SnapshotBackedWriteTransaction.class, TransactionReadyPrototype.class, "readyImpl"); -- 2.36.6