LOG.debug("Tx {} - Creating local component for shard {} using factory {}",
parent.getIdentifier(), shardName, local);
}
- return createLocalTransactionContext(local, parent);
+
+ try {
+ return createLocalTransactionContext(local, parent);
+ } catch(Exception e) {
+ return new NoOpTransactionContext(e, parent.getIdentifier());
+ }
}
return null;
updateShardInfo(shardName, primaryShardInfo);
- TransactionContext localContext = maybeCreateLocalTransactionContext(parent, shardName);
- if(localContext != null) {
- transactionContextWrapper.executePriorTransactionOperations(localContext);
- } else {
- RemoteTransactionContextSupport remote = new RemoteTransactionContextSupport(transactionContextWrapper,
- parent, shardName);
- remote.setPrimaryShard(primaryShardInfo.getPrimaryShardActor(), primaryShardInfo.getPrimaryShardVersion());
+ try {
+ TransactionContext localContext = maybeCreateLocalTransactionContext(parent, shardName);
+ if(localContext != null) {
+ transactionContextWrapper.executePriorTransactionOperations(localContext);
+ } else {
+ RemoteTransactionContextSupport remote = new RemoteTransactionContextSupport(transactionContextWrapper,
+ parent, shardName);
+ remote.setPrimaryShard(primaryShardInfo.getPrimaryShardActor(), primaryShardInfo.getPrimaryShardVersion());
+ }
+ } finally {
+ onTransactionContextCreated(parent.getIdentifier());
}
}
- private static void onFindPrimaryShardFailure(Throwable failure, TransactionProxy parent,
+ private void onFindPrimaryShardFailure(Throwable failure, TransactionProxy parent,
String shardName, TransactionContextWrapper transactionContextWrapper) {
LOG.debug("Tx {}: Find primary for shard {} failed", parent.getIdentifier(), shardName, failure);
- transactionContextWrapper.executePriorTransactionOperations(new NoOpTransactionContext(failure,
- parent.getIdentifier()));
+ try {
+ transactionContextWrapper.executePriorTransactionOperations(new NoOpTransactionContext(failure,
+ parent.getIdentifier()));
+ } finally {
+ onTransactionContextCreated(parent.getIdentifier());
+ }
}
final TransactionContextWrapper newTransactionContextWrapper(final TransactionProxy parent, final String shardName) {
final TransactionContextWrapper transactionContextWrapper =
new TransactionContextWrapper(parent.getIdentifier(), actorContext);
- Future<PrimaryShardInfo> findPrimaryFuture = findPrimaryShard(shardName, parent.getIdentifier().toString());
+ Future<PrimaryShardInfo> findPrimaryFuture = findPrimaryShard(shardName, parent.getIdentifier());
if(findPrimaryFuture.isCompleted()) {
Try<PrimaryShardInfo> maybe = findPrimaryFuture.value().get();
if(maybe.isSuccess()) {
* @param shardName Shard name
* @return Future containing shard information.
*/
- protected abstract Future<PrimaryShardInfo> findPrimaryShard(String shardName, String txId);
+ protected abstract Future<PrimaryShardInfo> findPrimaryShard(@Nonnull String shardName,
+ @Nonnull TransactionIdentifier txId);
/**
* Create local transaction factory for specified shard, backed by specified shard leader
*/
protected abstract <T> void onTransactionReady(@Nonnull TransactionIdentifier transaction, @Nonnull Collection<Future<T>> cohortFutures);
+ /**
+ * Callback invoked when the internal TransactionContext has been created for a transaction.
+ *
+ * @param transactionId the ID of the transaction.
+ */
+ protected abstract void onTransactionContextCreated(@Nonnull TransactionIdentifier transactionId);
+
private static TransactionContext createLocalTransactionContext(final LocalTransactionFactory factory,
final TransactionProxy parent) {
package org.opendaylight.controller.cluster.datastore;
import akka.actor.ActorSelection;
+import akka.dispatch.Futures;
import akka.dispatch.OnComplete;
import com.google.common.base.Preconditions;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.List;
+import java.util.Map.Entry;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
+import javax.annotation.Nonnull;
import org.opendaylight.controller.cluster.datastore.identifiers.TransactionChainIdentifier;
import org.opendaylight.controller.cluster.datastore.identifiers.TransactionIdentifier;
import org.opendaylight.controller.cluster.datastore.messages.CloseTransactionChain;
private final TransactionContextFactory parent;
private volatile State currentState = IDLE_STATE;
+ /**
+ * This map holds Promise instances for each read-only tx. It is used to maintain ordering of tx creates
+ * wrt to read-only tx's between this class and a LocalTransactionChain since they're bridged by
+ * asynchronous futures. Otherwise, in the following scenario, eg:
+ *
+ * 1) Create write tx1 on chain
+ * 2) do write and submit
+ * 3) Create read-only tx2 on chain and issue read
+ * 4) Create write tx3 on chain, do write but do not submit
+ *
+ * if the sequence/timing is right, tx3 may create its local tx on the LocalTransactionChain before tx2,
+ * which results in tx2 failing b/c tx3 isn't ready yet. So maintaining ordering prevents this issue
+ * (see Bug 4774).
+ * <p>
+ * A Promise is added via newReadOnlyTransaction. When the parent class completes the primary shard
+ * lookup and creates the TransactionContext (either success or failure), onTransactionContextCreated is
+ * called which completes the Promise. A write tx that is created prior to completion will wait on the
+ * Promise's Future via findPrimaryShard.
+ */
+ private final ConcurrentMap<TransactionIdentifier, Promise<Object>> priorReadOnlyTxPromises = new ConcurrentHashMap<>();
+
TransactionChainProxy(final TransactionContextFactory parent) {
super(parent.getActorContext());
@Override
public DOMStoreReadTransaction newReadOnlyTransaction() {
currentState.checkReady();
- return new TransactionProxy(this, TransactionType.READ_ONLY);
+ TransactionProxy transactionProxy = new TransactionProxy(this, TransactionType.READ_ONLY);
+ priorReadOnlyTxPromises.put(transactionProxy.getIdentifier(), Futures.<Object>promise());
+ return transactionProxy;
}
@Override
* before we initiate the next Tx in the chain to avoid creation failures if the
* previous Tx's ready operations haven't completed yet.
*/
+ @SuppressWarnings({ "unchecked", "rawtypes" })
@Override
- protected Future<PrimaryShardInfo> findPrimaryShard(final String shardName, final String txId) {
+ protected Future<PrimaryShardInfo> findPrimaryShard(final String shardName, final TransactionIdentifier txId) {
// Read current state atomically
final State localState = currentState;
// There are no outstanding futures, shortcut
- final Future<?> previous = localState.previousFuture();
+ Future<?> previous = localState.previousFuture();
if (previous == null) {
- return parent.findPrimaryShard(shardName, txId);
+ return combineFutureWithPossiblePriorReadOnlyTxFutures(parent.findPrimaryShard(shardName, txId), txId);
}
final String previousTransactionId;
LOG.debug("Waiting for ready futures on chain {}", getTransactionChainId());
}
+ previous = combineFutureWithPossiblePriorReadOnlyTxFutures(previous, txId);
+
// Add a callback for completion of the combined Futures.
- final Promise<PrimaryShardInfo> returnPromise = akka.dispatch.Futures.promise();
+ final Promise<PrimaryShardInfo> returnPromise = Futures.promise();
final OnComplete onComplete = new OnComplete() {
@Override
return returnPromise.future();
}
+ private <T> Future<T> combineFutureWithPossiblePriorReadOnlyTxFutures(final Future<T> future,
+ final TransactionIdentifier txId) {
+ if(!priorReadOnlyTxPromises.containsKey(txId) && !priorReadOnlyTxPromises.isEmpty()) {
+ Collection<Entry<TransactionIdentifier, Promise<Object>>> priorReadOnlyTxPromiseEntries =
+ new ArrayList<>(priorReadOnlyTxPromises.entrySet());
+ if(priorReadOnlyTxPromiseEntries.isEmpty()) {
+ return future;
+ }
+
+ List<Future<Object>> priorReadOnlyTxFutures = new ArrayList<>(priorReadOnlyTxPromiseEntries.size());
+ for(Entry<TransactionIdentifier, Promise<Object>> entry: priorReadOnlyTxPromiseEntries) {
+ LOG.debug("Tx: {} - waiting on future for prior read-only Tx {}", txId, entry.getKey());
+ priorReadOnlyTxFutures.add(entry.getValue().future());
+ }
+
+ Future<Iterable<Object>> combinedFutures = Futures.sequence(priorReadOnlyTxFutures,
+ getActorContext().getClientDispatcher());
+
+ final Promise<T> returnPromise = Futures.promise();
+ final OnComplete<Iterable<Object>> onComplete = new OnComplete<Iterable<Object>>() {
+ @Override
+ public void onComplete(final Throwable failure, final Iterable<Object> notUsed) {
+ LOG.debug("Tx: {} - prior read-only Tx futures complete", txId);
+
+ // Complete the returned Promise with the original Future.
+ returnPromise.completeWith(future);
+ }
+ };
+
+ combinedFutures.onComplete(onComplete, getActorContext().getClientDispatcher());
+ return returnPromise.future();
+ } else {
+ return future;
+ }
+ }
+
@Override
protected <T> void onTransactionReady(final TransactionIdentifier transaction, final Collection<Future<T>> cohortFutures) {
final State localState = currentState;
}
// Combine the ready Futures into 1
- final Future<Iterable<T>> combined = akka.dispatch.Futures.sequence(
- cohortFutures, getActorContext().getClientDispatcher());
+ final Future<Iterable<T>> combined = Futures.sequence(cohortFutures, getActorContext().getClientDispatcher());
// Record the we have outstanding futures
final State newState = new Submitted(transaction, combined);
}, getActorContext().getClientDispatcher());
}
+ @Override
+ protected void onTransactionContextCreated(@Nonnull TransactionIdentifier transactionId) {
+ Promise<Object> promise = priorReadOnlyTxPromises.remove(transactionId);
+ if(promise != null) {
+ promise.success(null);
+ }
+ }
+
@Override
protected TransactionIdentifier nextIdentifier() {
return transactionChainId.newTransactionIdentifier();
}};
}
+ @Test
+ public void testChainWithReadOnlyTxAfterPreviousReady() throws Throwable {
+ new IntegrationTestKit(getSystem(), datastoreContextBuilder) {{
+ DistributedDataStore dataStore = setupDistributedDataStore(
+ "testChainWithReadOnlyTxAfterPreviousReady", "test-1");
+
+ final DOMStoreTransactionChain txChain = dataStore.createTransactionChain();
+
+ // Create a write tx and submit.
+
+ DOMStoreWriteTransaction writeTx = txChain.newWriteOnlyTransaction();
+ writeTx.write(TestModel.TEST_PATH, ImmutableNodes.containerNode(TestModel.TEST_QNAME));
+ DOMStoreThreePhaseCommitCohort cohort1 = writeTx.ready();
+
+ // Create read-only tx's and issue a read.
+
+ CheckedFuture<Optional<NormalizedNode<?, ?>>, ReadFailedException> readFuture1 =
+ txChain.newReadOnlyTransaction().read(TestModel.TEST_PATH);
+
+ CheckedFuture<Optional<NormalizedNode<?, ?>>, ReadFailedException> readFuture2 =
+ txChain.newReadOnlyTransaction().read(TestModel.TEST_PATH);
+
+ // Create another write tx and issue the write.
+
+ DOMStoreWriteTransaction writeTx2 = txChain.newWriteOnlyTransaction();
+ writeTx2.write(TestModel.OUTER_LIST_PATH,
+ ImmutableNodes.mapNodeBuilder(TestModel.OUTER_LIST_QNAME).build());
+
+ // Ensure the reads succeed.
+
+ assertEquals("isPresent", true, readFuture1.checkedGet(5, TimeUnit.SECONDS).isPresent());
+ assertEquals("isPresent", true, readFuture2.checkedGet(5, TimeUnit.SECONDS).isPresent());
+
+ // Ensure the writes succeed.
+
+ DOMStoreThreePhaseCommitCohort cohort2 = writeTx2.ready();
+
+ doCommit(cohort1);
+ doCommit(cohort2);
+
+ assertEquals("isPresent", true, txChain.newReadOnlyTransaction().read(TestModel.OUTER_LIST_PATH).
+ checkedGet(5, TimeUnit.SECONDS).isPresent());
+ }};
+ }
+
@Test
public void testChainedTransactionFailureWithSingleShard() throws Exception{
new IntegrationTestKit(getSystem(), datastoreContextBuilder) {{