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 <rovarga@cisco.com>

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 18f0cddf179b2ed010123c6eaf9f65f320314049..abe106170b46c97fd48e04f40d7d8d00d3e316ab 100644 (file)
--- 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<NormalizedNode<?, ?>> 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 183c2192e405e2a48a52c664ef90033f5da38597..5c377d5ff526ac21fbc828f0261498a7f0eafac6 100644 (file)
--- 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 5c9f0d11c6584378e4c3a9ff4d46fabe597a80ef..f8c1db987912e44e487088abb9dd3d903b0c8fee 100644 (file)
--- 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<URI> 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 1641b668c325804e1680cd3089f95a83e91c2d7b..3d48271c54f9178685260c30841d1437caa4a62e 100644 (file)
--- 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<NormalizedNode<?, ?>> possibleEntityTypes = snapshot.readNode(ENTITY_TYPES_PATH);
+        Optional<NormalizedNode<?, ?>> 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<NormalizedNode<?, ?>> possibleEntityTypes = snapshot.readNode(ENTITY_TYPES_PATH);
+        Optional<NormalizedNode<?, ?>> possibleEntityTypes = getDataStore().readNode(ENTITY_TYPES_PATH);
         if(!possibleEntityTypes.isPresent()) {
             return;
         }
@@ -388,7 +385,7 @@ class EntityOwnershipShard extends Shard {
         }
     }
 
-    private Collection<String> getCandidateNames(MapEntryNode entity) {
+    private static Collection<String> getCandidateNames(MapEntryNode entity) {
         Collection<MapEntryNode> candidates = ((MapNode)entity.getChild(CANDIDATE_NODE_ID).get()).getValue();
         Collection<String> 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<NormalizedNode<?, ?>> optionalEntityOwner = snapshot.readNode(entityId.node(ENTITY_OWNER_QNAME));
+        Optional<NormalizedNode<?, ?>> 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 3f0a1bfc861ab34b9499bc47a2da7af345f4f136..382c17dd5a3b2253b2aeb0b34fefc58274e750a3 100644 (file)
--- 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<? extends Shard> 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<NormalizedNode<?, ?>> 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> 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 4778c569b4d9f55781cc337444469c9651ce9020..a3c9a6cca4bd569c501797e492c09a21bcc4db83 100644 (file)
--- 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<NormalizedNode<?,?>> 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<NormalizedNode<?,?>> 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 28330185872b3231be213b6115696a75ddc7b64f..144f0f5c9fc825917d139ca01b3ff0e989c39ab1 100644 (file)
--- 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 ea7648efb9df918bd034f858f6d26eebd5a48afd..9315cc2ddef123a21690be402f8e6b55b5b9e8cd 100644 (file)
--- 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,