From: Robert Varga Date: Tue, 17 May 2016 21:21:58 +0000 (+0200) Subject: BUG-5280: eliminate ShardTransactionIdentifier X-Git-Tag: release/boron~152 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=4062f5241a2a6f58ffb83dd1e9939ee66122d217 BUG-5280: eliminate ShardTransactionIdentifier This is very simple wrapper class which has two functions: - carries a String transaction identifier (making it available) - prepends it with 'shard-' in its toString() method Analyzing the actual uses of this class revealed that the toString() is called only when instantiating an actor, at which point either the transaction identifier is already present, or a transaction identifier is actually available through AbstractShardDataTreeTransaction. The second case is actually not entirely accurate, as for the case of remotely-created transactions we will actually use the actor name for the transaction name, hence adding to the overall string-induced confusion. Moving the toString() method towards actor instantiation removes the need to pass this class around and we can pass down normal transaction identifiers. This newfound unification allows us to eliminate duplication in the Shard*Transaction classes, which can pick the identifier from AbstractShardDataTreeTransaction, eliminating surface where inconsistencies may be introduced. Change-Id: If05f1bf2e76434d07feab115239addbb4b4bfb91 Signed-off-by: Robert Varga --- diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java index 250e3f854f..b76c0fac34 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java @@ -29,7 +29,6 @@ import org.opendaylight.controller.cluster.common.actor.MessageTracker.Error; import org.opendaylight.controller.cluster.common.actor.MeteringBehavior; import org.opendaylight.controller.cluster.datastore.exceptions.NoShardLeaderException; import org.opendaylight.controller.cluster.datastore.identifiers.ShardIdentifier; -import org.opendaylight.controller.cluster.datastore.identifiers.ShardTransactionIdentifier; import org.opendaylight.controller.cluster.datastore.jmx.mbeans.shard.ShardMBeanFactory; import org.opendaylight.controller.cluster.datastore.jmx.mbeans.shard.ShardStats; import org.opendaylight.controller.cluster.datastore.messages.AbortTransaction; @@ -602,8 +601,7 @@ public class Shard extends RaftActor { } } - private ActorRef createTransaction(int transactionType, String remoteTransactionId, String transactionChainId) { - ShardTransactionIdentifier transactionId = new ShardTransactionIdentifier(remoteTransactionId); + private ActorRef createTransaction(int transactionType, String transactionId, String transactionChainId) { LOG.debug("{}: Creating transaction : {} ", persistenceId(), transactionId); return transactionActorFactory.newShardTransaction(TransactionType.fromInt(transactionType), transactionId, transactionChainId); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardReadTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardReadTransaction.java index e65b8f415e..d28278afa9 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardReadTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardReadTransaction.java @@ -28,8 +28,8 @@ public class ShardReadTransaction extends ShardTransaction { private final AbstractShardDataTreeTransaction transaction; public ShardReadTransaction(AbstractShardDataTreeTransaction transaction, ActorRef shardActor, - ShardStats shardStats, String transactionID) { - super(shardActor, shardStats, transactionID); + ShardStats shardStats) { + super(shardActor, shardStats, transaction.getId()); this.transaction = transaction; } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardReadWriteTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardReadWriteTransaction.java index 2cc23c90e8..46a5039012 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardReadWriteTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardReadWriteTransaction.java @@ -19,8 +19,8 @@ import org.opendaylight.controller.cluster.datastore.messages.ReadData; */ public class ShardReadWriteTransaction extends ShardWriteTransaction { public ShardReadWriteTransaction(ReadWriteShardDataTreeTransaction transaction, ActorRef shardActor, - ShardStats shardStats, String transactionID) { - super(transaction, shardActor, shardStats, transactionID); + ShardStats shardStats) { + super(transaction, shardActor, shardStats); } @Override diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardSnapshotCohort.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardSnapshotCohort.java index e49aa1e4f3..7cdd7c6b98 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardSnapshotCohort.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardSnapshotCohort.java @@ -10,7 +10,6 @@ package org.opendaylight.controller.cluster.datastore; import akka.actor.ActorRef; import com.google.common.base.Preconditions; import java.util.concurrent.ExecutionException; -import org.opendaylight.controller.cluster.datastore.identifiers.ShardTransactionIdentifier; import org.opendaylight.controller.cluster.datastore.messages.CreateSnapshot; import org.opendaylight.controller.cluster.datastore.utils.SerializationUtils; import org.opendaylight.controller.cluster.raft.RaftActorSnapshotCohort; @@ -45,11 +44,8 @@ class ShardSnapshotCohort implements RaftActorSnapshotCohort { // so that this actor does not get block building the snapshot. THe transaction actor will // after processing the CreateSnapshot message. - ShardTransactionIdentifier transactionID = new ShardTransactionIdentifier( - "createSnapshot" + ++createSnapshotTransactionCounter); - ActorRef createSnapshotTransaction = transactionActorFactory.newShardTransaction( - TransactionType.READ_ONLY, transactionID, ""); + TransactionType.READ_ONLY, "createSnapshot" + ++createSnapshotTransactionCounter, ""); createSnapshotTransaction.tell(CreateSnapshot.INSTANCE, actorRef); } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransaction.java index c2a05a6d61..ee38d1918e 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransaction.java @@ -52,9 +52,8 @@ public abstract class ShardTransaction extends AbstractUntypedActorWithMetering } public static Props props(TransactionType type, AbstractShardDataTreeTransaction transaction, ActorRef shardActor, - DatastoreContext datastoreContext, ShardStats shardStats, String transactionID) { - return Props.create(new ShardTransactionCreator(type, transaction, shardActor, - datastoreContext, shardStats, transactionID)); + DatastoreContext datastoreContext, ShardStats shardStats) { + return Props.create(new ShardTransactionCreator(type, transaction, shardActor, datastoreContext, shardStats)); } protected abstract AbstractShardDataTreeTransaction getDOMStoreTransaction(); @@ -131,16 +130,14 @@ public abstract class ShardTransaction extends AbstractUntypedActorWithMetering final ActorRef shardActor; final DatastoreContext datastoreContext; final ShardStats shardStats; - final String transactionID; final TransactionType type; ShardTransactionCreator(TransactionType type, AbstractShardDataTreeTransaction transaction, ActorRef shardActor, - DatastoreContext datastoreContext, ShardStats shardStats, String transactionID) { + DatastoreContext datastoreContext, ShardStats shardStats) { this.transaction = Preconditions.checkNotNull(transaction); this.shardActor = shardActor; this.shardStats = shardStats; this.datastoreContext = datastoreContext; - this.transactionID = Preconditions.checkNotNull(transactionID); this.type = type; } @@ -149,16 +146,13 @@ public abstract class ShardTransaction extends AbstractUntypedActorWithMetering final ShardTransaction tx; switch (type) { case READ_ONLY: - tx = new ShardReadTransaction(transaction, shardActor, - shardStats, transactionID); + tx = new ShardReadTransaction(transaction, shardActor, shardStats); break; case READ_WRITE: - tx = new ShardReadWriteTransaction((ReadWriteShardDataTreeTransaction)transaction, - shardActor, shardStats, transactionID); + tx = new ShardReadWriteTransaction((ReadWriteShardDataTreeTransaction)transaction, shardActor, shardStats); break; case WRITE_ONLY: - tx = new ShardWriteTransaction((ReadWriteShardDataTreeTransaction)transaction, - shardActor, shardStats, transactionID); + tx = new ShardWriteTransaction((ReadWriteShardDataTreeTransaction)transaction, shardActor, shardStats); break; default: throw new IllegalArgumentException("Unhandled transaction type " + type); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransactionFactory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransactionFactory.java index 76838dd2f8..3c03871159 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransactionFactory.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransactionFactory.java @@ -10,7 +10,6 @@ package org.opendaylight.controller.cluster.datastore; import akka.actor.ActorRef; import akka.actor.UntypedActorContext; import com.google.common.base.Preconditions; -import org.opendaylight.controller.cluster.datastore.identifiers.ShardTransactionIdentifier; import org.opendaylight.controller.cluster.datastore.jmx.mbeans.shard.ShardStats; /** @@ -37,28 +36,26 @@ class ShardTransactionActorFactory { this.shardActor = shardActor; } - ActorRef newShardTransaction(TransactionType type, ShardTransactionIdentifier transactionID, - String transactionChainID) { + ActorRef newShardTransaction(TransactionType type, String transactionID, String transactionChainID) { final AbstractShardDataTreeTransaction transaction; switch (type) { case READ_ONLY: - transaction = dataTree.newReadOnlyTransaction(transactionID.toString(), transactionChainID); + transaction = dataTree.newReadOnlyTransaction(transactionID, transactionChainID); shardMBean.incrementReadOnlyTransactionCount(); break; case READ_WRITE: - transaction = dataTree.newReadWriteTransaction(transactionID.toString(), transactionChainID); + transaction = dataTree.newReadWriteTransaction(transactionID, transactionChainID); shardMBean.incrementReadWriteTransactionCount(); break; case WRITE_ONLY: - transaction = dataTree.newReadWriteTransaction(transactionID.toString(), transactionChainID); + transaction = dataTree.newReadWriteTransaction(transactionID, transactionChainID); shardMBean.incrementWriteOnlyTransactionCount(); break; default: throw new IllegalArgumentException("Unsupported transaction type " + type); } - return actorContext.actorOf(ShardTransaction.props(type, transaction, shardActor, datastoreContext, shardMBean, - transactionID.getRemoteTransactionId()).withDispatcher(txnDispatcherPath), - transactionID.toString()); + return actorContext.actorOf(ShardTransaction.props(type, transaction, shardActor, datastoreContext, shardMBean) + .withDispatcher(txnDispatcherPath), "shard-" + transactionID); } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardWriteTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardWriteTransaction.java index 2ad2488d6d..4ae54d3e5f 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardWriteTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardWriteTransaction.java @@ -30,8 +30,8 @@ public class ShardWriteTransaction extends ShardTransaction { private final ReadWriteShardDataTreeTransaction transaction; public ShardWriteTransaction(ReadWriteShardDataTreeTransaction transaction, ActorRef shardActor, - ShardStats shardStats, String transactionID) { - super(shardActor, shardStats, transactionID); + ShardStats shardStats) { + super(shardActor, shardStats, transaction.getId()); this.transaction = transaction; } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/ShardTransactionIdentifier.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/ShardTransactionIdentifier.java deleted file mode 100644 index fa1525c574..0000000000 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/ShardTransactionIdentifier.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright (c) 2014 Cisco Systems, Inc. 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.controller.cluster.datastore.identifiers; - -import com.google.common.base.Preconditions; - -public class ShardTransactionIdentifier { - private final String remoteTransactionId; - private final String stringRepresentation; - - public ShardTransactionIdentifier(String remoteTransactionId) { - this.remoteTransactionId = Preconditions.checkNotNull(remoteTransactionId, - "remoteTransactionId should not be null"); - - stringRepresentation = new StringBuilder(remoteTransactionId.length() + 6).append("shard-"). - append(remoteTransactionId).toString(); - } - - public String getRemoteTransactionId() { - return remoteTransactionId; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - ShardTransactionIdentifier that = (ShardTransactionIdentifier) o; - - if (!remoteTransactionId.equals(that.remoteTransactionId)) { - return false; - } - - return true; - } - - @Override - public int hashCode() { - return remoteTransactionId.hashCode(); - } - - @Override public String toString() { - return stringRepresentation; - } - -} diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionFailureTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionFailureTest.java index 75fdc36146..cc15e1a0e2 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionFailureTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionFailureTest.java @@ -60,7 +60,7 @@ public class ShardTransactionFailureTest extends AbstractActorTest { final ActorRef shard = createShard(); final Props props = ShardTransaction.props(RO, store.newReadOnlyTransaction("test-txn", null), shard, - datastoreContext, shardStats, "txn"); + datastoreContext, shardStats); final TestActorRef subject = TestActorRef.create(getSystem(), props, "testNegativeReadWithReadOnlyTransactionClosed"); @@ -82,7 +82,7 @@ public class ShardTransactionFailureTest extends AbstractActorTest { final ActorRef shard = createShard(); final Props props = ShardTransaction.props(RW, store.newReadWriteTransaction("test-txn", null), shard, - datastoreContext, shardStats, "txn"); + datastoreContext, shardStats); final TestActorRef subject = TestActorRef.create(getSystem(), props, "testNegativeReadWithReadWriteTransactionClosed"); @@ -103,7 +103,7 @@ public class ShardTransactionFailureTest extends AbstractActorTest { final ActorRef shard = createShard(); final Props props = ShardTransaction.props(RW, store.newReadWriteTransaction("test-txn", null), shard, - datastoreContext, shardStats, "txn"); + datastoreContext, shardStats); final TestActorRef subject = TestActorRef.create(getSystem(), props, "testNegativeExistsWithReadWriteTransactionClosed"); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionTest.java index 95b26817b9..ad5a7a4535 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionTest.java @@ -86,7 +86,7 @@ public class ShardTransactionTest extends AbstractActorTest { private ActorRef newTransactionActor(TransactionType type, AbstractShardDataTreeTransaction transaction, ActorRef shard, String name) { Props props = ShardTransaction.props(type, transaction, shard != null ? shard : createShard(), - datastoreContext, shardStats, "txn"); + datastoreContext, shardStats); return getSystem().actorOf(props, name); } @@ -420,7 +420,7 @@ public class ShardTransactionTest extends AbstractActorTest { public void testNegativePerformingWriteOperationOnReadTransaction() throws Exception { final ActorRef shard = createShard(); final Props props = ShardTransaction.props(TransactionType.READ_ONLY, readOnlyTransaction(), shard, - datastoreContext, shardStats, "txn"); + datastoreContext, shardStats); final TestActorRef transaction = TestActorRef.apply(props,getSystem()); transaction.receive(new BatchedModifications("tx1", DataStoreVersions.CURRENT_VERSION, null),