Author: frm Date: Tue Aug 20 11:26:06 2019 New Revision: 1865531 URL: http://svn.apache.org/viewvc?rev=1865531&view=rev Log: OAK-8066 - Prevent OOME when rebasing nodes with many children
Added: jackrabbit/oak/branches/1.10/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/LargeNumberOfChildNodeUpdatesIT.java - copied, changed from r1854579, jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/LargeNumberOfChildNodeUpdatesIT.java Modified: jackrabbit/oak/branches/1.10/ (props changed) jackrabbit/oak/branches/1.10/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java Propchange: jackrabbit/oak/branches/1.10/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Tue Aug 20 11:26:06 2019 @@ -1,3 +1,3 @@ /jackrabbit/oak/branches/1.0:1665962 -/jackrabbit/oak/trunk:1850874,1850882,1851236,1851253,1851451,1851533-1851535,1851619,1852052,1852084,1852120,1852135,1852451,1852492-1852493,1852528,1852582,1852584,1852601,1852920,1853083,1853141,1853229,1853393,1853429,1853433,1853441,1853866,1853868,1853870,1853893,1853969,1853997,1854034,1854044,1854055,1854058,1854113,1854373,1854377,1854380,1854385,1854401,1854403,1854455,1854461-1854462,1854466,1854468,1854515,1854533,1854539,1854701,1854773-1854774,1854827,1854848,1854859,1854930,1854990-1854991,1855032,1855221,1855477-1855478,1855522,1855776,1855905,1855993,1856049,1856056,1856538,1856545,1857000,1857010,1857104,1857159,1857212,1857221,1857238,1857242,1857247,1857253,1857294,1857314,1857332,1857463,1857480,1857577,1857589,1857592,1857627,1857634-1857635,1857638,1857640,1857687,1857936,1858032,1858053,1858123,1858139,1858385,1858424,1858571,1858578,1858810,1858926,1858931,1859020,1859231,1859292,1859294,1859350-1859351,1859359,1859365,1859533,1859609,1859612,1859619,1859711 ,1859716,1859772,1859776,1859780,1859843,1859854,1859881,1860120,1860131,1860137,1860202,1860278,1860328,1860330,1860355,1860393,1860442,1860548,1860564-1860565,1860931,1861114,1861270,1861626,1861743,1861757,1861763,1861770,1861776,1861780,1862044,1862093,1862266,1862370,1862422,1862447-1862448,1862465,1862499,1862531,1862536-1862537,1862728,1862806,1862817,1862881,1862926-1862927,1862976-1862977,1863076,1863081,1863086,1863369,1863405,1863540,1863633-1863634,1863642,1863645,1864349,1864353 +/jackrabbit/oak/trunk:1850874,1850882,1851236,1851253,1851451,1851533-1851535,1851619,1852052,1852084,1852120,1852135,1852451,1852492-1852493,1852528,1852582,1852584,1852601,1852920,1853083,1853141,1853229,1853393,1853429,1853433,1853441,1853866,1853868,1853870,1853893,1853969,1853997,1854034,1854044,1854055,1854058,1854113,1854373,1854377,1854380,1854385,1854401,1854403,1854455,1854461-1854462,1854466,1854468,1854515,1854533,1854539,1854579-1854580,1854596,1854701,1854773-1854774,1854827,1854848,1854859,1854930,1854990-1854991,1855032,1855221,1855477-1855478,1855522,1855776,1855905,1855993,1856049,1856056,1856538,1856545,1857000,1857010,1857104,1857159,1857212,1857221,1857238,1857242,1857247,1857253,1857294,1857314,1857332,1857463,1857480,1857577,1857589,1857592,1857627,1857634-1857635,1857638,1857640,1857687,1857936,1858032,1858053,1858123,1858139,1858385,1858424,1858571,1858578,1858810,1858926,1858931,1859020,1859231,1859292,1859294,1859350-1859351,1859359,1859365,1859533,1859609 ,1859612,1859619,1859711,1859716,1859772,1859776,1859780,1859843,1859854,1859881,1860120,1860131,1860137,1860202,1860278,1860328,1860330,1860355,1860393,1860442,1860548,1860564-1860565,1860931,1861114,1861270,1861626,1861743,1861757,1861763,1861770,1861776,1861780,1862044,1862093,1862266,1862370,1862422,1862447-1862448,1862465,1862499,1862531,1862536-1862537,1862728,1862806,1862817,1862881,1862926-1862927,1862976-1862977,1863076,1863081,1863086,1863369,1863405,1863540,1863633-1863634,1863642,1863645,1864349,1864353 /jackrabbit/trunk:1345480 Modified: jackrabbit/oak/branches/1.10/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.10/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java?rev=1865531&r1=1865530&r2=1865531&view=diff ============================================================================== --- jackrabbit/oak/branches/1.10/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java (original) +++ jackrabbit/oak/branches/1.10/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java Tue Aug 20 11:26:06 2019 @@ -33,7 +33,6 @@ import static com.google.common.collect. import static com.google.common.collect.Lists.partition; import static com.google.common.collect.Maps.newHashMap; import static com.google.common.io.ByteStreams.read; -import static java.lang.Integer.getInteger; import static java.lang.Long.numberOfLeadingZeros; import static java.lang.Math.min; import static java.util.Arrays.asList; @@ -44,6 +43,7 @@ import static org.apache.jackrabbit.oak. import static org.apache.jackrabbit.oak.api.Type.NAME; import static org.apache.jackrabbit.oak.api.Type.NAMES; import static org.apache.jackrabbit.oak.api.Type.STRING; +import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.compareAgainstEmptyState; import static org.apache.jackrabbit.oak.segment.MapEntry.newModifiedMapEntry; import static org.apache.jackrabbit.oak.segment.MapRecord.BUCKETS_PER_LEVEL; import static org.apache.jackrabbit.oak.segment.RecordWriters.newNodeStateWriter; @@ -70,7 +70,6 @@ import org.apache.jackrabbit.oak.segment import org.apache.jackrabbit.oak.segment.file.tar.GCGeneration; import org.apache.jackrabbit.oak.segment.spi.persistence.Buffer; import org.apache.jackrabbit.oak.spi.blob.BlobStore; -import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry; import org.apache.jackrabbit.oak.spi.state.DefaultNodeStateDiff; import org.apache.jackrabbit.oak.spi.state.NodeState; import org.jetbrains.annotations.NotNull; @@ -89,17 +88,10 @@ public class DefaultSegmentWriter implem private static final Logger LOG = LoggerFactory.getLogger(DefaultSegmentWriter.class); /** - * Default threshold of the number of modified child nodes of a node after which to - * log a warning. + * Number of updates to child nodes before changes are flushed to disk. */ - public static final int CHILD_NODE_COUNT_WARN_THRESHOLD_DEFAULT = 1000000; - - /** - * Threshold of the number of modified child nodes of a node after which to log - * a warning. - */ - private static final int CHILD_NODE_COUNT_WARN_THRESHOLD = - getInteger("oak.segment.childNodeCountWarnThreshold", CHILD_NODE_COUNT_WARN_THRESHOLD_DEFAULT); + private static final int CHILD_NODE_UPDATE_LIMIT = Integer + .getInteger("child.node.update.limit", 10000); @NotNull private final WriterCacheManager cacheManager; @@ -201,7 +193,7 @@ public class DefaultSegmentWriter implem @NotNull public RecordId writeNode(@NotNull final NodeState state, @Nullable final Buffer stableIdBytes) throws IOException { return new SegmentWriteOperation(writeOperationHandler.getGCGeneration()) - .writeNode("/", state, stableIdBytes); + .writeNode(state, stableIdBytes); } /** @@ -219,8 +211,6 @@ public class DefaultSegmentWriter implem private final Cache<String, RecordId> nodeCache; - private int childNodeCountWarnThreshold = CHILD_NODE_COUNT_WARN_THRESHOLD; - SegmentWriteOperation(@NotNull GCGeneration gcGeneration) { int generation = gcGeneration.getGeneration(); this.gcGeneration = gcGeneration; @@ -756,10 +746,7 @@ public class DefaultSegmentWriter implem return tid; } - private RecordId writeNode( - @NotNull String path, - @NotNull NodeState state, - @Nullable Buffer stableIdBytes) + private RecordId writeNode(@NotNull NodeState state, @Nullable Buffer stableIdBytes) throws IOException { RecordId compactedId = deduplicateNode(state); @@ -770,7 +757,7 @@ public class DefaultSegmentWriter implem if (state instanceof SegmentNodeState && stableIdBytes == null) { stableIdBytes = ((SegmentNodeState) state).getStableIdBytes(); } - RecordId recordId = writeNodeUncached(path, state, stableIdBytes); + RecordId recordId = writeNodeUncached(state, stableIdBytes); if (stableIdBytes != null) { // This node state has been rewritten because it is from an older @@ -786,30 +773,14 @@ public class DefaultSegmentWriter implem return (byte) (Byte.MIN_VALUE + 64 - numberOfLeadingZeros(childCount)); } - private void warnOnManyChildren(Map<?, ?> map, String path) { - if (map.size() >= childNodeCountWarnThreshold) { - childNodeCountWarnThreshold += CHILD_NODE_COUNT_WARN_THRESHOLD; - LOG.warn("Large number of modified child nodes: {} @ {}", map.size(), path); - } - } - - private RecordId writeNodeUncached( - @NotNull String path, - @NotNull NodeState state, - @Nullable Buffer stableIdBytes) + private RecordId writeNodeUncached(@NotNull NodeState state, @Nullable Buffer stableIdBytes) throws IOException { - ModifiedNodeState after = null; - - if (state instanceof ModifiedNodeState) { - after = (ModifiedNodeState) state; - } RecordId beforeId = null; - - if (after != null) { + if (state instanceof ModifiedNodeState) { // Pass null to indicate we don't want to update the node write statistics // when deduplicating the base state - beforeId = deduplicateNode(after.getBaseState()); + beforeId = deduplicateNode(((ModifiedNodeState) state).getBaseState()); } SegmentNodeState before = null; @@ -830,26 +801,9 @@ public class DefaultSegmentWriter implem String childName = template.getChildName(); if (childName == Template.MANY_CHILD_NODES) { - MapRecord base; - Map<String, RecordId> childNodes; - if (before != null - && before.getChildNodeCount(2) > 1 - && after.getChildNodeCount(2) > 1) { - base = before.getChildNodeMap(); - childNodes = new ChildNodeCollectorDiff(path).diff(before, after); - } else { - base = null; - childNodes = newHashMap(); - for (ChildNodeEntry entry : state.getChildNodeEntries()) { - childNodes.put( - entry.getName(), - writeNode(path + "/" + entry.getName(), entry.getNodeState(), null)); - warnOnManyChildren(childNodes, path); - } - } - ids.add(writeMap(base, childNodes)); + ids.add(writeChildNodes(before, state)); } else if (childName != Template.ZERO_CHILD_NODES) { - ids.add(writeNode(path + "/" + template.getChildName(), state.getChildNode(template.getChildName()), null)); + ids.add(writeNode(state.getChildNode(template.getChildName()), null)); } List<RecordId> pIds = newArrayList(); @@ -915,6 +869,20 @@ public class DefaultSegmentWriter implem newNodeStateWriter(stableId, ids))); } + @NotNull + private RecordId writeChildNodes(@Nullable SegmentNodeState before, @NotNull NodeState after) + throws IOException { + if (before != null + && before.getChildNodeCount(2) > 1 + && after.getChildNodeCount(2) > 1) { + return new ChildNodeCollectorDiff(before.getChildNodeMap()) + .diff(before, after); + } else { + return new ChildNodeCollectorDiff() + .diff(after); + } + } + /** * Try to deduplicate the passed {@code node}. This succeeds if * the passed node state has already been persisted to this store and @@ -994,30 +962,42 @@ public class DefaultSegmentWriter implem } private class ChildNodeCollectorDiff extends DefaultNodeStateDiff { - private final Map<String, RecordId> childNodes = newHashMap(); - private final String path; + @Nullable + private MapRecord base; private IOException exception; - public ChildNodeCollectorDiff(String path) { - this.path = path; + private ChildNodeCollectorDiff(@Nullable MapRecord base) { + this.base = base; + } + + private ChildNodeCollectorDiff() { + this(null); + } + + public RecordId diff(NodeState after) throws IOException { + compareAgainstEmptyState(after, this); + if (exception != null) { + throw new IOException(exception); + } + return flush(); } - public Map<String, RecordId> diff(SegmentNodeState before, ModifiedNodeState after) throws IOException { + public RecordId diff(NodeState before, NodeState after) + throws IOException { after.compareAgainstBaseState(before, this); if (exception != null) { throw new IOException(exception); - } else { - return childNodes; } + return flush(); } @Override public boolean childNodeAdded(String name, NodeState after) { try { - addChild(name, writeNode(path + "/" + name, after, null)); + onChildNode(name, writeNode(after, null)); } catch (IOException e) { exception = e; return false; @@ -1030,7 +1010,7 @@ public class DefaultSegmentWriter implem String name, NodeState before, NodeState after ) { try { - addChild(name, writeNode(path + "/" + name, after, null)); + onChildNode(name, writeNode(after, null)); } catch (IOException e) { exception = e; return false; @@ -1040,14 +1020,29 @@ public class DefaultSegmentWriter implem @Override public boolean childNodeDeleted(String name, NodeState before) { - addChild(name, null); + try { + onChildNode(name, null); + } catch (IOException e) { + exception = e; + return false; + } return true; } - private void addChild(String name, @Nullable RecordId recordId) { - childNodes.put(name, recordId); - warnOnManyChildren(childNodes, path); + private void onChildNode(String nodeName, @Nullable RecordId recordId) throws IOException { + childNodes.put(nodeName, recordId); + if (childNodes.size() > CHILD_NODE_UPDATE_LIMIT) { + flush(); + } + } + + private RecordId flush() throws IOException { + RecordId mapId = writeMap(base, childNodes); + base = reader.readMap(mapId); + childNodes.clear(); + return mapId; } + } } Copied: jackrabbit/oak/branches/1.10/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/LargeNumberOfChildNodeUpdatesIT.java (from r1854579, jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/LargeNumberOfChildNodeUpdatesIT.java) URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.10/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/LargeNumberOfChildNodeUpdatesIT.java?p2=jackrabbit/oak/branches/1.10/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/LargeNumberOfChildNodeUpdatesIT.java&p1=jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/LargeNumberOfChildNodeUpdatesIT.java&r1=1854579&r2=1865531&rev=1865531&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/LargeNumberOfChildNodeUpdatesIT.java (original) +++ jackrabbit/oak/branches/1.10/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/LargeNumberOfChildNodeUpdatesIT.java Tue Aug 20 11:26:06 2019 @@ -37,9 +37,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; -// *<p>The test is <b>disabled</b> by default, to run it you need to set the {@code SegmentCompactionIT} system property:<br> -// * {@code mvn test -Dtest=SegmentCompactionIT -Dtest.opts.memory=-Xmx4G} - /** * This test asserts that a large number of child nodes can be added in a single * transaction. Due to its long running time the test needs to be explicitly enabled