From c78ba1f70be123609b88f43ce40f8555f1bc2ec7 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Thu, 29 May 2014 00:02:17 +0200 Subject: [PATCH] BUG-509: remove unnecessary Version objects Testing has revealed the datastore instantiates effectively a version per tree node. This turns out to be not needed, as we really just need a single version per commit -- which can be shared between nodes, as the (node,version) tuple remains unique. So let's just instantiate a single version when we start the transaction, which we need to do anyway, and use it for all nodes/subtrees added or modified. This places an upper bound on Version objects retained to the number of transactions committed (not counting no-ops). Change-Id: I40d095ec230eee8b5af2409c1e8ee61a1860a9d3 Signed-off-by: Robert Varga --- .../impl/tree/data/AlwaysFailOperation.java | 2 +- .../tree/data/ModificationApplyOperation.java | 4 +-- ...izedNodeContainerModificationStrategy.java | 28 ++++++------------- .../tree/data/OperationWithModification.java | 4 +-- .../tree/data/SchemaAwareApplyOperation.java | 24 ++++++++-------- .../data/ValueNodeModificationStrategy.java | 10 +++---- 6 files changed, 31 insertions(+), 41 deletions(-) diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/AlwaysFailOperation.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/AlwaysFailOperation.java index 15479be462..c09a1a38b5 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/AlwaysFailOperation.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/AlwaysFailOperation.java @@ -16,7 +16,7 @@ import com.google.common.base.Optional; final class AlwaysFailOperation implements ModificationApplyOperation { @Override public Optional apply(final ModifiedNode modification, - final Optional storeMeta, final Version subtreeVersion) { + final Optional storeMeta, final Version version) { throw new IllegalStateException("Schema Context is not available."); } diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/ModificationApplyOperation.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/ModificationApplyOperation.java index 10f39a8cef..df03db35be 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/ModificationApplyOperation.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/ModificationApplyOperation.java @@ -50,7 +50,7 @@ interface ModificationApplyOperation extends StoreTreeNode apply(ModifiedNode modification, Optional storeMeta, Version subtreeVersion); + Optional apply(ModifiedNode modification, Optional storeMeta, Version version); /** * diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/NormalizedNodeContainerModificationStrategy.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/NormalizedNodeContainerModificationStrategy.java index 3a3af5ecab..b70c5d18dc 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/NormalizedNodeContainerModificationStrategy.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/NormalizedNodeContainerModificationStrategy.java @@ -95,16 +95,9 @@ abstract class NormalizedNodeContainerModificationStrategy extends SchemaAwareAp @Override protected TreeNode applyWrite(final ModifiedNode modification, - final Optional currentMeta, final Version subtreeVersion) { - final Version nodeVersion; - if (currentMeta.isPresent()) { - nodeVersion = currentMeta.get().getVersion().next(); - } else { - nodeVersion = subtreeVersion; - } - + final Optional currentMeta, final Version version) { final NormalizedNode newValue = modification.getWrittenValue(); - final TreeNode newValueMeta = TreeNodeFactory.createTreeNode(newValue, nodeVersion); + final TreeNode newValueMeta = TreeNodeFactory.createTreeNode(newValue, version); if (Iterables.isEmpty(modification.getChildren())) { return newValueMeta; @@ -122,12 +115,12 @@ abstract class NormalizedNodeContainerModificationStrategy extends SchemaAwareAp * and run the common parts on it -- which end with the node being sealed. */ final MutableTreeNode mutable = newValueMeta.mutable(); - mutable.setSubtreeVersion(subtreeVersion); + mutable.setSubtreeVersion(version); @SuppressWarnings("rawtypes") final NormalizedNodeContainerBuilder dataBuilder = createBuilder(newValue); - return mutateChildren(mutable, dataBuilder, nodeVersion, modification.getChildren()); + return mutateChildren(mutable, dataBuilder, version, modification.getChildren()); } @SuppressWarnings({ "rawtypes", "unchecked" }) @@ -155,24 +148,21 @@ abstract class NormalizedNodeContainerModificationStrategy extends SchemaAwareAp @Override protected TreeNode applyMerge(final ModifiedNode modification, final TreeNode currentMeta, - final Version subtreeVersion) { + final Version version) { // For Node Containers - merge is same as subtree change - we only replace children. - return applySubtreeChange(modification, currentMeta, subtreeVersion); + return applySubtreeChange(modification, currentMeta, version); } @Override public TreeNode applySubtreeChange(final ModifiedNode modification, - final TreeNode currentMeta, final Version subtreeVersion) { - // Bump subtree version to its new target - final Version updatedSubtreeVersion = currentMeta.getSubtreeVersion().next(); - + final TreeNode currentMeta, final Version version) { final MutableTreeNode newMeta = currentMeta.mutable(); - newMeta.setSubtreeVersion(updatedSubtreeVersion); + newMeta.setSubtreeVersion(version); @SuppressWarnings("rawtypes") NormalizedNodeContainerBuilder dataBuilder = createBuilder(currentMeta.getData()); - return mutateChildren(newMeta, dataBuilder, updatedSubtreeVersion, modification.getChildren()); + return mutateChildren(newMeta, dataBuilder, version, modification.getChildren()); } @Override diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/OperationWithModification.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/OperationWithModification.java index ff90d57f49..e1cc1a17e5 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/OperationWithModification.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/OperationWithModification.java @@ -44,8 +44,8 @@ final class OperationWithModification { return applyOperation; } - public Optional apply(final Optional data, final Version subtreeVersion) { - return applyOperation.apply(modification, data, subtreeVersion); + public Optional apply(final Optional data, final Version version) { + return applyOperation.apply(modification, data, version); } public static OperationWithModification from(final ModificationApplyOperation operation, diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/SchemaAwareApplyOperation.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/SchemaAwareApplyOperation.java index bdf5667b67..6ef76adacf 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/SchemaAwareApplyOperation.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/SchemaAwareApplyOperation.java @@ -184,7 +184,7 @@ abstract class SchemaAwareApplyOperation implements ModificationApplyOperation { @Override public final Optional apply(final ModifiedNode modification, - final Optional currentMeta, final Version subtreeVersion) { + final Optional currentMeta, final Version version) { switch (modification.getType()) { case DELETE: @@ -193,13 +193,13 @@ abstract class SchemaAwareApplyOperation implements ModificationApplyOperation { Preconditions.checkArgument(currentMeta.isPresent(), "Metadata not available for modification", modification); return modification.storeSnapshot(Optional.of(applySubtreeChange(modification, currentMeta.get(), - subtreeVersion))); + version))); case MERGE: if(currentMeta.isPresent()) { - return modification.storeSnapshot(Optional.of(applyMerge(modification,currentMeta.get(),subtreeVersion))); + return modification.storeSnapshot(Optional.of(applyMerge(modification,currentMeta.get(), version))); } // Fallback to write is intentional - if node is not preexisting merge is same as write case WRITE: - return modification.storeSnapshot(Optional.of(applyWrite(modification, currentMeta, subtreeVersion))); + return modification.storeSnapshot(Optional.of(applyWrite(modification, currentMeta, version))); case UNMODIFIED: return currentMeta; default: @@ -208,13 +208,13 @@ abstract class SchemaAwareApplyOperation implements ModificationApplyOperation { } protected abstract TreeNode applyMerge(ModifiedNode modification, - TreeNode currentMeta, Version subtreeVersion); + TreeNode currentMeta, Version version); protected abstract TreeNode applyWrite(ModifiedNode modification, - Optional currentMeta, Version subtreeVersion); + Optional currentMeta, Version version); protected abstract TreeNode applySubtreeChange(ModifiedNode modification, - TreeNode currentMeta, Version subtreeVersion); + TreeNode currentMeta, Version version); protected abstract void checkSubtreeModificationApplicable(InstanceIdentifier path, final NodeModification modification, final Optional current) throws DataPreconditionFailedException; @@ -231,20 +231,20 @@ abstract class SchemaAwareApplyOperation implements ModificationApplyOperation { @Override protected TreeNode applyMerge(final ModifiedNode modification, final TreeNode currentMeta, - final Version subtreeVersion) { - return applyWrite(modification, Optional.of(currentMeta), subtreeVersion); + final Version version) { + return applyWrite(modification, Optional.of(currentMeta), version); } @Override protected TreeNode applySubtreeChange(final ModifiedNode modification, - final TreeNode currentMeta, final Version subtreeVersion) { + final TreeNode currentMeta, final Version version) { throw new UnsupportedOperationException("UnkeyedList does not support subtree change."); } @Override protected TreeNode applyWrite(final ModifiedNode modification, - final Optional currentMeta, final Version subtreeVersion) { - return TreeNodeFactory.createTreeNode(modification.getWrittenValue(), subtreeVersion); + final Optional currentMeta, final Version version) { + return TreeNodeFactory.createTreeNode(modification.getWrittenValue(), version); } @Override diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/ValueNodeModificationStrategy.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/ValueNodeModificationStrategy.java index 5f68782a2e..a9f69ac953 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/ValueNodeModificationStrategy.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/data/ValueNodeModificationStrategy.java @@ -48,22 +48,22 @@ abstract class ValueNodeModificationStrategy extends S @Override protected TreeNode applySubtreeChange(final ModifiedNode modification, - final TreeNode currentMeta, final Version subtreeVersion) { + final TreeNode currentMeta, final Version version) { throw new UnsupportedOperationException("Node " + schema.getPath() + "is leaf type node. Subtree change is not allowed."); } @Override protected TreeNode applyMerge(final ModifiedNode modification, final TreeNode currentMeta, - final Version subtreeVersion) { + final Version version) { // Just overwrite whatever was there - return applyWrite(modification, null, subtreeVersion); + return applyWrite(modification, null, version); } @Override protected TreeNode applyWrite(final ModifiedNode modification, - final Optional currentMeta, final Version subtreeVersion) { - return TreeNodeFactory.createTreeNode(modification.getWrittenValue(), subtreeVersion); + final Optional currentMeta, final Version version) { + return TreeNodeFactory.createTreeNode(modification.getWrittenValue(), version); } @Override -- 2.36.6