From 2cd9bb89ca2eeff0adffeb4b8501da4e9eb3bca3 Mon Sep 17 00:00:00 2001 From: Jukka Zitting Date: Thu, 24 Apr 2014 12:25:27 -0400 Subject: [PATCH] OAK-1768: DocumentNodeBuilder.setChildNode() runs OOM with large tree --- .../oak/plugins/document/DocumentNodeBuilder.java | 24 +++++++++++++-- .../jackrabbit/oak/upgrade/RepositoryUpgrade.java | 36 ++++------------------ 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeBuilder.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeBuilder.java index 5c525b5..a862567 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeBuilder.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeBuilder.java @@ -22,14 +22,15 @@ import java.io.InputStream; import javax.annotation.Nonnull; import org.apache.jackrabbit.oak.api.Blob; -import org.apache.jackrabbit.oak.commons.PathUtils; +import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeBuilder; import org.apache.jackrabbit.oak.spi.state.ApplyDiff; +import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeState; import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.base.Preconditions.checkState; +import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; import static org.apache.jackrabbit.oak.spi.state.AbstractNodeState.checkValidName; /** @@ -65,6 +66,25 @@ class DocumentNodeBuilder extends MemoryNodeBuilder { return new DocumentNodeBuilder(this, name, root); } + /** + * Sets the named subtree to the given state. To avoid running out + * of memory with large change-sets, the implementation recursively + * copies all properties and child nodes to this builder so that the + * purge mechanism has a chance to periodically flush partial changes + * to the underlying storage database. + */ + @Override + public NodeBuilder setChildNode(String name, NodeState state) { + NodeBuilder builder = super.setChildNode(name, EMPTY_NODE); + for (PropertyState property : state.getProperties()) { + builder.setProperty(property); + } + for (ChildNodeEntry child : state.getChildNodeEntries()) { + builder.setChildNode(child.getName(), child.getNodeState()); + } + return builder; + } + @Override public boolean moveTo(NodeBuilder newParent, String newName) { checkNotNull(newParent); diff --git a/oak-upgrade/src/main/java/org/apache/jackrabbit/oak/upgrade/RepositoryUpgrade.java b/oak-upgrade/src/main/java/org/apache/jackrabbit/oak/upgrade/RepositoryUpgrade.java index 5dff050..10709eb 100644 --- a/oak-upgrade/src/main/java/org/apache/jackrabbit/oak/upgrade/RepositoryUpgrade.java +++ b/oak-upgrade/src/main/java/org/apache/jackrabbit/oak/upgrade/RepositoryUpgrade.java @@ -117,7 +117,6 @@ import org.apache.jackrabbit.oak.plugins.name.NamespaceConstants; import org.apache.jackrabbit.oak.plugins.name.Namespaces; import org.apache.jackrabbit.oak.plugins.nodetype.TypeEditorProvider; import org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent; -import org.apache.jackrabbit.oak.plugins.segment.SegmentNodeBuilder; import org.apache.jackrabbit.oak.security.SecurityProviderImpl; import org.apache.jackrabbit.oak.spi.commit.CommitHook; import org.apache.jackrabbit.oak.spi.commit.CommitInfo; @@ -727,15 +726,15 @@ public class RepositoryUpgrade { source.getInternalVersionManager().getPersistenceManager(); NodeBuilder system = builder.child(JCR_SYSTEM); - copyState(system, JCR_VERSIONSTORAGE, new JackrabbitNodeState( + system.setChildNode(JCR_VERSIONSTORAGE, new JackrabbitNodeState( pm, root, uriToPrefix, VERSION_STORAGE_NODE_ID, "/jcr:system/jcr:versionStorage", copyBinariesByReference)); - copyState(system, "jcr:activities", new JackrabbitNodeState( + system.setChildNode("jcr:activities", new JackrabbitNodeState( pm, root, uriToPrefix, ACTIVITIES_NODE_ID, "/jcr:system/jcr:activities", copyBinariesByReference)); } - private String copyWorkspace( + private void copyWorkspace( NodeBuilder builder, NodeState root, String name, Map uriToPrefix, Map idxToPrefix) throws RepositoryException, IOException { @@ -745,7 +744,8 @@ public class RepositoryUpgrade { source.getWorkspaceInfo(name).getPersistenceManager(); NodeState state = new JackrabbitNodeState( - pm, root, uriToPrefix, ROOT_NODE_ID, "/", copyBinariesByReference); + pm, root, uriToPrefix, ROOT_NODE_ID, + "/", copyBinariesByReference); for (PropertyState property : state.getProperties()) { builder.setProperty(property); @@ -753,33 +753,9 @@ public class RepositoryUpgrade { for (ChildNodeEntry child : state.getChildNodeEntries()) { String childName = child.getName(); if (!JCR_SYSTEM.equals(childName)) { - copyState(builder, childName, child.getNodeState()); + builder.setChildNode(childName, child.getNodeState()); } } - - return name; - } - - private void copyState(NodeBuilder parent, String name, NodeState state) { - if (parent instanceof SegmentNodeBuilder) { - parent.setChildNode(name, state); - } else { - setChildNode(parent, name, state); - } - } - - /** - * NodeState are copied by value by recursing down the complete tree - * This is a temporary approach for OAK-1760 for 1.0 branch. - */ - private void setChildNode(NodeBuilder parent, String name, NodeState state) { - NodeBuilder builder = parent.setChildNode(name); - for (PropertyState property : state.getProperties()) { - builder.setProperty(property); - } - for (ChildNodeEntry child : state.getChildNodeEntries()) { - setChildNode(builder, child.getName(), child.getNodeState()); - } } } -- 1.8.4.msysgit.0