From: Robert Varga Date: Fri, 23 Oct 2015 12:24:39 +0000 (+0200) Subject: Reduce ShardDataTree#getDataTree() callsites X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=94603c85193862f85bf9d9aa51d5062d9f84e979 Reduce ShardDataTree#getDataTree() callsites A lot of these callsites perform a specific function, expose those functions without leaking the DataTree. This is needed to handle asynchronous persistence and optimistic transaction commit. Change-Id: I330cb4172349e0d1d8daacc3aafce7dad64cd8b2 Signed-off-by: Robert Varga --- diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java index 18f0cddf17..abe106170b 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java @@ -25,8 +25,10 @@ import org.opendaylight.yangtools.concepts.ListenerRegistration; 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.DataTreeCandidate; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateTip; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidates; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeSnapshot; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException; import org.opendaylight.yangtools.yang.data.api.schema.tree.TipProducingDataTree; import org.opendaylight.yangtools.yang.data.impl.schema.tree.InMemoryDataTreeFactory; @@ -196,4 +198,24 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { snapshot.ready(); return new SimpleShardDataTreeCohort(this, snapshot, transaction.getId()); } + + public Optional> readNode(YangInstanceIdentifier path) { + return dataTree.takeSnapshot().readNode(path); + } + + public DataTreeSnapshot takeSnapshot() { + return dataTree.takeSnapshot(); + } + + public DataTreeModification newModification() { + return dataTree.takeSnapshot().newModification(); + } + + public DataTreeCandidate commit(DataTreeModification modification) throws DataValidationFailedException { + modification.ready(); + dataTree.validate(modification); + DataTreeCandidateTip candidate = dataTree.prepare(modification); + dataTree.commit(candidate); + return candidate; + } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java index 183c2192e4..5c377d5ff5 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java @@ -37,7 +37,7 @@ final class ShardDataTreeTransactionChain extends ShardDataTreeTransactionParent Preconditions.checkState(openTransaction == null, "Transaction %s is open", openTransaction); if (previousTx == null) { - return dataTree.getDataTree().takeSnapshot(); + return dataTree.takeSnapshot(); } else { return previousTx.getSnapshot(); } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinator.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinator.java index 5c9f0d11c6..f8c1db9879 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinator.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinator.java @@ -22,9 +22,7 @@ import org.opendaylight.controller.cluster.raft.protobuff.client.messages.Compos import org.opendaylight.controller.cluster.raft.protobuff.client.messages.Payload; 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.DataTree; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidates; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException; import org.opendaylight.yangtools.yang.model.api.SchemaContext; import org.slf4j.Logger; @@ -40,7 +38,7 @@ import org.slf4j.Logger; */ class ShardRecoveryCoordinator implements RaftActorRecoveryCohort { private static final YangInstanceIdentifier ROOT = YangInstanceIdentifier.builder().build(); - private final DataTree store; + private final ShardDataTree store; private final String shardName; private final Logger log; private final Set validNamespaces; @@ -48,7 +46,7 @@ class ShardRecoveryCoordinator implements RaftActorRecoveryCohort { private int size; ShardRecoveryCoordinator(ShardDataTree store, SchemaContext schemaContext, String shardName, Logger log) { - this.store = store.getDataTree(); + this.store = Preconditions.checkNotNull(store); this.shardName = shardName; this.log = log; this.validNamespaces = NormalizedNodePruner.namespaces(schemaContext); @@ -57,7 +55,7 @@ class ShardRecoveryCoordinator implements RaftActorRecoveryCohort { @Override public void startLogRecoveryBatch(int maxBatchSize) { log.debug("{}: starting log recovery batch with max size {}", shardName, maxBatchSize); - transaction = new PruningDataTreeModification(store.takeSnapshot().newModification(), validNamespaces); + transaction = new PruningDataTreeModification(store.newModification(), validNamespaces); size = 0; } @@ -90,10 +88,7 @@ class ShardRecoveryCoordinator implements RaftActorRecoveryCohort { } private void commitTransaction(PruningDataTreeModification tx) throws DataValidationFailedException { - DataTreeModification delegate = tx.getDelegate(); - delegate.ready(); - store.validate(delegate); - store.commit(store.prepare(delegate)); + store.commit(tx.getDelegate()); } /** @@ -122,7 +117,7 @@ class ShardRecoveryCoordinator implements RaftActorRecoveryCohort { log.debug("{}: Applying recovered snapshot", shardName); final NormalizedNode node = SerializationUtils.deserializeNormalizedNode(snapshotBytes); - final PruningDataTreeModification tx = new PruningDataTreeModification(store.takeSnapshot().newModification(), validNamespaces); + final PruningDataTreeModification tx = new PruningDataTreeModification(store.newModification(), validNamespaces); tx.write(ROOT, node); try { commitTransaction(tx); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShard.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShard.java index 1641b668c3..3d48271c54 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShard.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShard.java @@ -59,7 +59,6 @@ import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild; import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode; import org.opendaylight.yangtools.yang.data.api.schema.MapNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeSnapshot; import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; import org.opendaylight.yangtools.yang.model.api.SchemaContext; import scala.concurrent.Future; @@ -343,13 +342,12 @@ class EntityOwnershipShard extends Shard { commitCoordinator.commitModifications(modifications, this); } - private boolean hasCandidate(MapEntryNode entity, String candidateName) { + private static boolean hasCandidate(MapEntryNode entity, String candidateName) { return ((MapNode)entity.getChild(CANDIDATE_NODE_ID).get()).getChild(candidateNodeKey(candidateName)).isPresent(); } private void searchForEntitiesOwnedBy(final String owner, final EntityWalker walker) { - DataTreeSnapshot snapshot = getDataStore().getDataTree().takeSnapshot(); - Optional> possibleEntityTypes = snapshot.readNode(ENTITY_TYPES_PATH); + Optional> possibleEntityTypes = getDataStore().readNode(ENTITY_TYPES_PATH); if(!possibleEntityTypes.isPresent()) { return; } @@ -369,8 +367,7 @@ class EntityOwnershipShard extends Shard { } private void searchForEntities(EntityWalker walker) { - DataTreeSnapshot snapshot = getDataStore().getDataTree().takeSnapshot(); - Optional> possibleEntityTypes = snapshot.readNode(ENTITY_TYPES_PATH); + Optional> possibleEntityTypes = getDataStore().readNode(ENTITY_TYPES_PATH); if(!possibleEntityTypes.isPresent()) { return; } @@ -388,7 +385,7 @@ class EntityOwnershipShard extends Shard { } } - private Collection getCandidateNames(MapEntryNode entity) { + private static Collection getCandidateNames(MapEntryNode entity) { Collection candidates = ((MapNode)entity.getChild(CANDIDATE_NODE_ID).get()).getValue(); Collection candidateNames = new ArrayList<>(candidates.size()); for(MapEntryNode candidate: candidates) { @@ -416,8 +413,7 @@ class EntityOwnershipShard extends Shard { } private String getCurrentOwner(YangInstanceIdentifier entityId) { - DataTreeSnapshot snapshot = getDataStore().getDataTree().takeSnapshot(); - Optional> optionalEntityOwner = snapshot.readNode(entityId.node(ENTITY_OWNER_QNAME)); + Optional> optionalEntityOwner = getDataStore().readNode(entityId.node(ENTITY_OWNER_QNAME)); if(optionalEntityOwner.isPresent()){ return optionalEntityOwner.get().getValue().toString(); } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/AbstractShardTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/AbstractShardTest.java index 3f0a1bfc86..382c17dd5a 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/AbstractShardTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/AbstractShardTest.java @@ -50,7 +50,6 @@ 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.DataTreeCandidateTip; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeSnapshot; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException; import org.opendaylight.yangtools.yang.model.api.SchemaContext; @@ -243,16 +242,11 @@ public abstract class AbstractShardTest extends AbstractActorTest{ public static NormalizedNode readStore(final TestActorRef shard, final YangInstanceIdentifier id) throws ExecutionException, InterruptedException { - return readStore(shard.underlyingActor().getDataStore().getDataTree(), id); + return shard.underlyingActor().getDataStore().readNode(id).orNull(); } public static NormalizedNode readStore(final DataTree store, final YangInstanceIdentifier id) { - final DataTreeSnapshot transaction = store.takeSnapshot(); - - final Optional> optional = transaction.readNode(id); - final NormalizedNode node = optional.isPresent()? optional.get() : null; - - return node; + return store.takeSnapshot().readNode(id).orNull(); } public static void writeToStore(final TestActorRef shard, final YangInstanceIdentifier id, diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinatorTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinatorTest.java index 4778c569b4..a3c9a6cca4 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinatorTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinatorTest.java @@ -142,29 +142,21 @@ public class ShardRecoveryCoordinatorTest { private Optional> readCars(final ShardDataTree shardDataTree){ final TipProducingDataTree dataTree = shardDataTree.getDataTree(); + // FIXME: this should not be called here dataTree.setSchemaContext(peopleSchemaContext); - final DataTreeSnapshot snapshot = dataTree.takeSnapshot(); - - final DataTreeModification modification = snapshot.newModification(); - - return modification.readNode(CarsModel.BASE_PATH); + return shardDataTree.readNode(CarsModel.BASE_PATH); } private Optional> readPeople(final ShardDataTree shardDataTree){ final TipProducingDataTree dataTree = shardDataTree.getDataTree(); + // FIXME: this should not be called here dataTree.setSchemaContext(peopleSchemaContext); - final DataTreeSnapshot snapshot = dataTree.takeSnapshot(); - - final DataTreeModification modification = snapshot.newModification(); - - return modification.readNode(PeopleModel.BASE_PATH); + return shardDataTree.readNode(PeopleModel.BASE_PATH); } - - - private byte[] createSnapshot(){ + private static byte[] createSnapshot(){ final TipProducingDataTree dataTree = InMemoryDataTreeFactory.getInstance().create(); dataTree.setSchemaContext(SchemaContextHelper.select(SchemaContextHelper.CARS_YANG, SchemaContextHelper.PEOPLE_YANG)); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java index 2833018587..144f0f5c9f 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java @@ -1182,7 +1182,7 @@ public class ShardTest extends AbstractShardTest { final ShardDataTree dataStore = shard.underlyingActor().getDataStore(); - final DataTreeModification modification = dataStore.getDataTree().takeSnapshot().newModification(); + final DataTreeModification modification = dataStore.newModification(); final ContainerNode writeData = ImmutableNodes.containerNode(TestModel.TEST_QNAME); new WriteModification(TestModel.TEST_PATH, writeData).apply(modification); @@ -1215,7 +1215,7 @@ public class ShardTest extends AbstractShardTest { final ShardDataTree dataStore = shard.underlyingActor().getDataStore(); - final DataTreeModification modification = dataStore.getDataTree().takeSnapshot().newModification(); + final DataTreeModification modification = dataStore.newModification(); final ContainerNode writeData = ImmutableNodes.containerNode(TestModel.TEST_QNAME); new WriteModification(TestModel.TEST_PATH, writeData).apply(modification); @@ -2085,7 +2085,7 @@ public class ShardTest extends AbstractShardTest { // Ready the third Tx. final String transactionID3 = "tx3"; - final DataTreeModification modification3 = dataStore.getDataTree().takeSnapshot().newModification(); + final DataTreeModification modification3 = dataStore.newModification(); new WriteModification(TestModel.TEST2_PATH, ImmutableNodes.containerNode(TestModel.TEST2_QNAME)) .apply(modification3); modification3.ready(); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/AbstractEntityOwnershipTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/AbstractEntityOwnershipTest.java index ea7648efb9..9315cc2dde 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/AbstractEntityOwnershipTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/AbstractEntityOwnershipTest.java @@ -45,7 +45,6 @@ import org.opendaylight.yangtools.yang.data.api.schema.DataContainerNode; import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode; import org.opendaylight.yangtools.yang.data.api.schema.MapNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateTip; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException; @@ -156,26 +155,21 @@ public class AbstractEntityOwnershipTest extends AbstractActorTest { static void writeNode(YangInstanceIdentifier path, NormalizedNode node, ShardDataTree shardDataTree) throws DataValidationFailedException { - DataTreeModification modification = shardDataTree.getDataTree().takeSnapshot().newModification(); + DataTreeModification modification = shardDataTree.newModification(); modification.merge(path, node); commit(shardDataTree, modification); } static void deleteNode(YangInstanceIdentifier path, ShardDataTree shardDataTree) throws DataValidationFailedException { - DataTreeModification modification = shardDataTree.getDataTree().takeSnapshot().newModification(); + DataTreeModification modification = shardDataTree.newModification(); modification.delete(path); commit(shardDataTree, modification); } static void commit(ShardDataTree shardDataTree, DataTreeModification modification) throws DataValidationFailedException { - modification.ready(); - - shardDataTree.getDataTree().validate(modification); - DataTreeCandidateTip candidate = shardDataTree.getDataTree().prepare(modification); - shardDataTree.getDataTree().commit(candidate); - shardDataTree.notifyListeners(candidate); + shardDataTree.notifyListeners(shardDataTree.commit(modification)); } static EntityOwnershipChange ownershipChange(final Entity expEntity, final boolean expWasOwner,