Bug 1572: Fixed duplicate and inconsistent allocation of Version

After switching from numeric version objects to non-numeric Version
modifications based on modifications was broken due to duplicate
allocation of versions - one when derived modification
was allocated and other one when modification was commited,
this make all writes effectivelly conflicting, since
same node was seen with two different versions.

Change-Id: I366f77a96b74db3aa13a2f374d9437e7cfeb2da5
Signed-off-by: Tony Tkacik <ttkacik@cisco.com>

diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTree.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTree.java
index 02e85a46a0bca643c3f2315d71730417ea3ac875..0f0b4a12da7e96f594a5d7a8f78c1a0dc79bf8c4 100644 (file)
--- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTree.java
+++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTree.java
@@ -103,7 +103,7 @@ final class InMemoryDataTree implements DataTree {
         rwLock.writeLock().lock();
         try {
             final Optional<TreeNode> newRoot = m.getStrategy().apply(m.getRootModification(),
-                    Optional.<TreeNode>of(rootNode), rootNode.getSubtreeVersion().next());
+                    Optional.<TreeNode>of(rootNode), m.getVersion());
             Preconditions.checkState(newRoot.isPresent(), "Apply strategy failed to produce root node");
             return new InMemoryDataTreeCandidate(PUBLIC_ROOT_PATH, root, rootNode, newRoot.get());
         } finally {

diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java
index 73d3ac36c03980d86d3235764755ea924ad38a10..e26c32ee5b03bfc01edd2ec64698a66911797bd0 100644 (file)
--- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java
+++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java
@@ -18,6 +18,7 @@ import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.TreeNode;
+import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.Version;
 import org.opendaylight.yangtools.yang.data.impl.schema.NormalizedNodeUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -30,6 +31,7 @@ final class InMemoryDataTreeModification implements DataTreeModification {
     private final RootModificationApplyOperation strategyTree;
     private final InMemoryDataTreeSnapshot snapshot;
     private final ModifiedNode rootNode;
+    private final Version version;
 
     @GuardedBy("this")
     private boolean sealed = false;
@@ -38,6 +40,17 @@ final class InMemoryDataTreeModification implements DataTreeModification {
         this.snapshot = Preconditions.checkNotNull(snapshot);
         this.strategyTree = Preconditions.checkNotNull(resolver).snapshot();
         this.rootNode = ModifiedNode.createUnmodified(snapshot.getRootNode());
+        /*
+         * We could allocate version beforehand, since Version contract
+         * states two allocated version must be allways different.
+         * 
+         * Preallocating version simplifies scenarios such as
+         * chaining of modifications, since version for particular
+         * node in modification and in data tree (if successfully
+         * commited) will be same and will not change.
+         * 
+         */
+        this.version = snapshot.getRootNode().getSubtreeVersion().next();
     }
 
     ModifiedNode getRootModification() {
@@ -108,7 +121,7 @@ final class InMemoryDataTreeModification implements DataTreeModification {
 
         try {
             return resolveModificationStrategy(path).apply(modification, modification.getOriginal(),
-                    snapshot.getRootNode().getSubtreeVersion().next());
+                    version);
         } catch (Exception e) {
             LOG.error("Could not create snapshot for {}:{}", path,modification,e);
             throw e;
@@ -160,22 +173,17 @@ final class InMemoryDataTreeModification implements DataTreeModification {
         }
 
         /*
-         *  FIXME: Add advanced transaction chaining for modification of not rebased
-         *  modification.
-         *
-         *  Current computation of tempRoot may yeld incorrect subtree versions
-         *  if there are multiple concurrent transactions, which may break
-         *  versioning preconditions for modification of previously occured write,
-         *  directly nested under parent node, since node version is derived from
-         *  subtree version.
-         *
-         *  For deeper nodes subtree version is derived from their respective metadata
-         *  nodes, so this incorrect root subtree version is not affecting us.
+         *  We will use preallocated version, this means returned snapshot will 
+         *  have same version each time this method is called.
          */
         TreeNode originalSnapshotRoot = snapshot.getRootNode();
-        Optional<TreeNode> tempRoot = strategyTree.apply(rootNode, Optional.of(originalSnapshotRoot), originalSnapshotRoot.getSubtreeVersion().next());
+        Optional<TreeNode> tempRoot = strategyTree.apply(rootNode, Optional.of(originalSnapshotRoot), version);
 
         InMemoryDataTreeSnapshot tempTree = new InMemoryDataTreeSnapshot(snapshot.getSchemaContext(), tempRoot.get(), strategyTree);
         return tempTree.newModification();
     }
+
+    Version getVersion() {
+        return version;
+    }
 }