Author: mreutegg
Date: Thu Jul  3 07:35:38 2014
New Revision: 1607557

URL: http://svn.apache.org/r1607557
Log:
OAK-1794: Keep commit info for local changes in main document

Added:
    
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java
   (with props)
    
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DummyRevisionContext.java
   (with props)
Modified:
    
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
    
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
    
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
    
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java
    
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
    
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorTest.java

Modified: 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1607557&r1=1607556&r2=1607557&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
 Thu Jul  3 07:35:38 2014
@@ -18,9 +18,7 @@ package org.apache.jackrabbit.oak.plugin
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Map;
 import java.util.NavigableMap;
 import java.util.Queue;
@@ -49,16 +47,14 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.collect.Iterables.filter;
 import static com.google.common.collect.Iterables.transform;
-import static java.util.Collections.disjoint;
 import static org.apache.jackrabbit.oak.plugins.document.UpdateOp.Key;
 import static org.apache.jackrabbit.oak.plugins.document.UpdateOp.Operation;
+import static 
org.apache.jackrabbit.oak.plugins.document.util.Utils.isRevisionNewer;
 
 /**
  * A document storing data about a node.
@@ -142,7 +138,7 @@ public final class NodeDocument extends 
      * revision is actually committed. Depth 0 means the commit is in the root 
node,
      * depth 1 means one node below the root, and so on.
      */
-    private static final String COMMIT_ROOT = "_commitRoot";
+    static final String COMMIT_ROOT = "_commitRoot";
 
     /**
      * The number of previous documents (documents that contain old revisions 
of
@@ -181,7 +177,7 @@ public final class NodeDocument extends 
      * "c-" + base revision of the successfully merged branch commit,
      * "b" + base revision of an un-merged branch commit
      */
-    private static final String REVISIONS = "_revisions";
+    static final String REVISIONS = "_revisions";
 
     /**
      * The last revision.
@@ -253,17 +249,35 @@ public final class NodeDocument extends 
          * A split document which contains all types of data. In addition
          * when the split document was created the main document did not had
          * any child.
+         * This type is deprecated because these kind of documents cannot be
+         * garbage collected independently. The main document may still
+         * reference _commitRoot entries in the previous document. See OAK-1794
          */
+        @Deprecated
         DEFAULT_NO_CHILD(20),
         /**
-         * A split document which does not contain REVISIONS history
+         * A split document which does not contain REVISIONS history.
+         * This type is deprecated because these kind of documents cannot be
+         * garbage collected independently. The main document may still
+         * reference _commitRoot entries in the previous document. See OAK-1794
          */
+        @Deprecated
         PROP_COMMIT_ONLY(30),
         /**
          * Its an intermediate split document which only contains version 
ranges
          * and does not contain any other attributes
          */
-        INTERMEDIATE(40)
+        INTERMEDIATE(40),
+        /**
+         * A split document which contains all types of data. In addition
+         * when the split document was created the main document did not had
+         * any child.
+         */
+        DEFAULT_LEAF(50),
+        /**
+         * A split document which does not contain REVISIONS history.
+         */
+        COMMIT_ROOT_ONLY(60),
         ;
 
         final int type;
@@ -293,7 +307,7 @@ public final class NodeDocument extends 
     /**
      * Properties to ignore when a document is split.
      */
-    private static final Set<String> IGNORE_ON_SPLIT = ImmutableSet.of(
+    static final Set<String> IGNORE_ON_SPLIT = ImmutableSet.of(
             ID, MOD_COUNT, MODIFIED_IN_SECS, PREVIOUS, LAST_REV, CHILDREN_FLAG,
             HAS_BINARY_FLAG, PATH, DELETED_ONCE, COLLISIONS);
 
@@ -885,156 +899,7 @@ public final class NodeDocument extends 
      */
     @Nonnull
     public Iterable<UpdateOp> split(@Nonnull RevisionContext context) {
-        SortedMap<Revision, Range> previous = getPreviousRanges();
-        // only consider if there are enough commits,
-        // unless document is really big
-        if (getLocalRevisions().size() + getLocalCommitRoot().size() <= 
NUM_REVS_THRESHOLD
-                && getMemory() < DOC_SIZE_THRESHOLD
-                && previous.size() < PREV_SPLIT_FACTOR) {
-            return Collections.emptyList();
-        }
-        String path = getPath();
-        String id = getId();
-        if (id == null) {
-            throw new IllegalStateException("document does not have an id: " + 
this);
-        }
-        // collect ranges and create a histogram of the height
-        Map<Integer, List<Range>> prevHisto = Maps.newHashMap();
-        for (Map.Entry<Revision, Range> entry : previous.entrySet()) {
-            Revision rev = entry.getKey();
-            if (rev.getClusterId() != context.getClusterId()) {
-                continue;
-            }
-            Range r = entry.getValue();
-            List<Range> list = prevHisto.get(r.getHeight());
-            if (list == null) {
-                list = new ArrayList<Range>();
-                prevHisto.put(r.getHeight(), list);
-            }
-            list.add(r);
-        }
-        Map<String, NavigableMap<Revision, String>> splitValues
-                = new HashMap<String, NavigableMap<Revision, String>>();
-        for (String property : data.keySet()) {
-            if (IGNORE_ON_SPLIT.contains(property)) {
-                continue;
-            }
-            NavigableMap<Revision, String> splitMap
-                    = new TreeMap<Revision, 
String>(context.getRevisionComparator());
-            splitValues.put(property, splitMap);
-            Map<Revision, String> valueMap = getLocalMap(property);
-            // collect committed changes of this cluster node after the
-            // most recent previous split revision
-            for (Map.Entry<Revision, String> entry : valueMap.entrySet()) {
-                Revision rev = entry.getKey();
-                if (rev.getClusterId() != context.getClusterId()) {
-                    continue;
-                }
-                if (isCommitted(rev)) {
-                    splitMap.put(rev, entry.getValue());
-                }
-            }
-        }
-
-        List<UpdateOp> splitOps = Lists.newArrayList();
-        int numValues = 0;
-        Revision high = null;
-        Revision low = null;
-        for (NavigableMap<Revision, String> splitMap : splitValues.values()) {
-            // keep the most recent in the main document
-            if (!splitMap.isEmpty()) {
-                splitMap.remove(splitMap.lastKey());
-            }
-            if (splitMap.isEmpty()) {
-                continue;
-            }
-            // remember highest / lowest revision
-            if (high == null || isRevisionNewer(context, splitMap.lastKey(), 
high)) {
-                high = splitMap.lastKey();
-            }
-            if (low == null || isRevisionNewer(context, low, 
splitMap.firstKey())) {
-                low = splitMap.firstKey();
-            }
-            numValues += splitMap.size();
-        }
-        UpdateOp main = null;
-        if (high != null && low != null
-                && (numValues >= NUM_REVS_THRESHOLD
-                    || getMemory() > DOC_SIZE_THRESHOLD)) {
-            // enough revisions to split off
-            // move to another document
-            main = new UpdateOp(id, false);
-            setPrevious(main, new Range(high, low, 0));
-            String oldPath = Utils.getPreviousPathFor(path, high, 0);
-            UpdateOp old = new UpdateOp(Utils.getIdFromPath(oldPath), true);
-            old.set(ID, old.getId());
-            if (Utils.isLongPath(oldPath)) {
-                old.set(PATH, oldPath);
-            }
-            for (String property : splitValues.keySet()) {
-                NavigableMap<Revision, String> splitMap = 
splitValues.get(property);
-                for (Map.Entry<Revision, String> entry : splitMap.entrySet()) {
-                    Revision r = entry.getKey();
-                    main.removeMapEntry(property, r);
-                    old.setMapEntry(property, r, entry.getValue());
-                }
-            }
-            // check size of old document
-            NodeDocument oldDoc = new NodeDocument(store);
-            UpdateUtils.applyChanges(oldDoc, old, 
context.getRevisionComparator());
-            setSplitDocProps(this, oldDoc, old, high);
-            // only split if enough of the data can be moved to old document
-            if (oldDoc.getMemory() > getMemory() * SPLIT_RATIO
-                    || numValues >= NUM_REVS_THRESHOLD) {
-                splitOps.add(old);
-            } else {
-                main = null;
-            }
-        }
-
-        // check if we need to create intermediate previous documents
-        for (Map.Entry<Integer, List<Range>> entry : prevHisto.entrySet()) {
-            if (entry.getValue().size() >= PREV_SPLIT_FACTOR) {
-                if (main == null) {
-                    main = new UpdateOp(id, false);
-                }
-                // calculate range new range
-                Revision h = null;
-                Revision l = null;
-                for (Range r : entry.getValue()) {
-                    if (h == null || isRevisionNewer(context, r.high, h)) {
-                        h = r.high;
-                    }
-                    if (l == null || isRevisionNewer(context, l, r.low)) {
-                        l = r.low;
-                    }
-                    removePrevious(main, r);
-                }
-                if (h == null || l == null) {
-                    throw new IllegalStateException();
-                }
-                String prevPath = Utils.getPreviousPathFor(path, h, 
entry.getKey() + 1);
-                String prevId = Utils.getIdFromPath(prevPath);
-                UpdateOp intermediate = new UpdateOp(prevId, true);
-                intermediate.set(ID, prevId);
-                if (Utils.isLongPath(prevPath)) {
-                    intermediate.set(PATH, prevPath);
-                }
-                setPrevious(main, new Range(h, l, entry.getKey() + 1));
-                for (Range r : entry.getValue()) {
-                    setPrevious(intermediate, r);
-                }
-                setIntermediateDocProps(intermediate, h);
-                splitOps.add(intermediate);
-            }
-        }
-
-        // main document must be updated last
-        if (main != null && !splitOps.isEmpty()) {
-            splitOps.add(main);
-        }
-
-        return splitOps;
+        return SplitOperations.forDocument(this, context);
     }
 
     /**
@@ -1224,6 +1089,10 @@ public final class NodeDocument extends 
         return REVISIONS.equals(name);
     }
 
+    public static boolean isCommitRootEntry(String name) {
+        return COMMIT_ROOT.equals(name);
+    }
+
     public static void removeRevision(@Nonnull UpdateOp op,
                                       @Nonnull Revision revision) {
         checkNotNull(op).removeMapEntry(REVISIONS, checkNotNull(revision));
@@ -1301,18 +1170,6 @@ public final class NodeDocument extends 
         checkNotNull(op).set(HAS_BINARY_FLAG, HAS_BINARY_VAL);
     }
 
-    //----------------------------< internal modifiers 
>------------------------
-
-    private static void setSplitDocType(@Nonnull UpdateOp op,
-                                        @Nonnull SplitDocType type) {
-        checkNotNull(op).set(SD_TYPE, type.type);
-    }
-
-    private static void setSplitDocMaxRev(@Nonnull UpdateOp op,
-                                          @Nonnull Revision maxRev) {
-        checkNotNull(op).set(SD_MAX_REV_TIME_IN_SECS, 
getModifiedInSecs(maxRev.getTimestamp()));
-    }
-
     //----------------------------< internal 
>----------------------------------
 
     /**
@@ -1363,82 +1220,6 @@ public final class NodeDocument extends 
     }
 
     /**
-     * Set various split document related flag/properties
-     *
-     * @param mainDoc main document from which split document is being created
-     * @param old updateOp of the old document created via split
-     * @param oldDoc old document created via split
-     * @param maxRev max revision stored in the split document oldDoc
-     */
-    private static void setSplitDocProps(NodeDocument mainDoc, NodeDocument 
oldDoc,
-                                         UpdateOp old, Revision maxRev) {
-        setSplitDocMaxRev(old, maxRev);
-
-        SplitDocType type = SplitDocType.DEFAULT;
-        if(!mainDoc.hasChildren() && !referencesOldDocAfterSplit(mainDoc, 
oldDoc)){
-            type = SplitDocType.DEFAULT_NO_CHILD;
-        } else if (oldDoc.getLocalRevisions().isEmpty()){
-            type = SplitDocType.PROP_COMMIT_ONLY;
-        }
-
-        //Copy over the hasBinary flag
-        if(mainDoc.hasBinary()){
-            setHasBinary(old);
-        }
-
-        setSplitDocType(old,type);
-    }
-
-    /**
-     * Checks if the main document has changes referencing {@code oldDoc} after
-     * the split.
-     *
-     * @param mainDoc the main document before the split.
-     * @param oldDoc  the old document created by the split.
-     * @return {@code true} if the main document contains references to the
-     *         old document after the split; {@code false} otherwise.
-     */
-    private static boolean referencesOldDocAfterSplit(NodeDocument mainDoc,
-                                                      NodeDocument oldDoc) {
-        Set<Revision> revs = oldDoc.getLocalRevisions().keySet();
-        for (String property : mainDoc.data.keySet()) {
-            if (IGNORE_ON_SPLIT.contains(property)) {
-                continue;
-            }
-            Set<Revision> changes = 
Sets.newHashSet(mainDoc.getLocalMap(property).keySet());
-            changes.removeAll(oldDoc.getLocalMap(property).keySet());
-            if (!disjoint(changes, revs)) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    /**
-     * Set various properties for intermediate split document
-     *
-     * @param intermediate updateOp of the intermediate doc getting created
-     * @param maxRev max revision stored in the intermediate
-     */
-    private static void setIntermediateDocProps(UpdateOp intermediate, 
Revision maxRev) {
-        setSplitDocMaxRev(intermediate, maxRev);
-        setSplitDocType(intermediate,SplitDocType.INTERMEDIATE);
-    }
-
-    /**
-     * Checks that revision x is newer than another revision.
-     *
-     * @param x the revision to check
-     * @param previous the presumed earlier revision
-     * @return true if x is newer
-     */
-    private static boolean isRevisionNewer(@Nonnull RevisionContext context,
-                                           @Nonnull Revision x,
-                                           @Nonnull Revision previous) {
-        return context.getRevisionComparator().compare(x, previous) > 0;
-    }
-
-    /**
      * Returns <code>true</code> if the given revision
      * {@link Utils#isCommitted(String)} in the revisions map (including
      * revisions split off to previous documents) and is visible from the

Added: 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java?rev=1607557&view=auto
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java
 (added)
+++ 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java
 Thu Jul  3 07:35:38 2014
@@ -0,0 +1,433 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.document;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.NavigableMap;
+import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
+
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+
+import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.util.Utils;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.COMMIT_ROOT;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.DOC_SIZE_THRESHOLD;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.IGNORE_ON_SPLIT;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.NUM_REVS_THRESHOLD;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.PREV_SPLIT_FACTOR;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.REVISIONS;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.SPLIT_RATIO;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.isCommitRootEntry;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.isRevisionsEntry;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.removePrevious;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.setHasBinary;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.setPrevious;
+import static 
org.apache.jackrabbit.oak.plugins.document.util.Utils.isRevisionNewer;
+
+/**
+ * Utility class to create document split operations.
+ */
+class SplitOperations {
+
+    private static final DocumentStore STORE = new MemoryDocumentStore();
+
+    private final NodeDocument doc;
+    private final String path;
+    private final String id;
+    private final RevisionContext context;
+    private Revision high;
+    private Revision low;
+    private int numValues;
+    private Map<String, NavigableMap<Revision, String>> committedChanges;
+    private Set<Revision> mostRecentRevs;
+    private Set<Revision> splitRevs;
+    private List<UpdateOp> splitOps;
+    private UpdateOp main;
+
+    private SplitOperations(@Nonnull NodeDocument doc,
+                            @Nonnull RevisionContext context) {
+        this.doc = checkNotNull(doc);
+        this.context = checkNotNull(context);
+        this.path = doc.getPath();
+        this.id = doc.getId();
+    }
+
+    /**
+     * Creates a list of update operations in case the given document requires
+     * a split.
+     *
+     * @param doc a main document.
+     * @param context the revision context.
+     * @return list of update operations. An empty list indicates the document
+     *          does not require a split.
+     * @throws IllegalArgumentException if the given document is a split
+     *                                  document.
+     */
+    @Nonnull
+    static List<UpdateOp> forDocument(@Nonnull NodeDocument doc,
+                                      @Nonnull RevisionContext context) {
+        if (doc.isSplitDocument()) {
+            throw new IllegalArgumentException(
+                    "Not a main document: " + doc.getId());
+        }
+        return new SplitOperations(doc, context).create();
+
+    }
+
+    private List<UpdateOp> create() {
+        if (!considerSplit()) {
+            return Collections.emptyList();
+        }
+        splitOps = Lists.newArrayList();
+        mostRecentRevs = Sets.newHashSet();
+        splitRevs = Sets.newHashSet();
+        committedChanges = getCommittedLocalChanges();
+
+        // revisions of the most recent committed changes on this document
+        // these are kept in the main document. _revisions and _commitRoot
+        // entries with these revisions are retained in the main document
+        populateSplitRevs();
+
+        // collect _revisions and _commitRoot entries for split document
+        collectRevisionsAndCommitRoot();
+
+        // create split ops out of the split values
+        main = createSplitOps();
+
+        // create intermediate docs if needed
+        createIntermediateDocs();
+
+        // main document must be updated last
+        if (main != null && !splitOps.isEmpty()) {
+            splitOps.add(main);
+        }
+
+        return splitOps;
+    }
+
+    private boolean considerSplit() {
+        SortedMap<Revision, Range> previous = doc.getPreviousRanges();
+        // only consider if there are enough commits,
+        // unless document is really big
+        return doc.getLocalRevisions().size() + 
doc.getLocalCommitRoot().size() > NUM_REVS_THRESHOLD
+                || doc.getMemory() >= DOC_SIZE_THRESHOLD
+                || previous.size() >= PREV_SPLIT_FACTOR;
+    }
+
+    /**
+     * Populate the {@link #splitRevs} with the revisions of the committed
+     * changes that will be moved to a previous document. For each property,
+     * all but the most recent change will be moved.
+     */
+    private void populateSplitRevs() {
+        for (NavigableMap<Revision, String> splitMap : 
committedChanges.values()) {
+            // keep the most recent changes in the main document
+            if (!splitMap.isEmpty()) {
+                Revision r = splitMap.lastKey();
+                splitMap.remove(r);
+                splitRevs.addAll(splitMap.keySet());
+                mostRecentRevs.add(r);
+            }
+            if (splitMap.isEmpty()) {
+                continue;
+            }
+            // remember highest / lowest revision
+            trackHigh(splitMap.lastKey());
+            trackLow(splitMap.firstKey());
+            numValues += splitMap.size();
+        }
+    }
+
+    /**
+     * Collect _revisions and _commitRoot entries that can be moved to a
+     * previous document.
+     */
+    private void collectRevisionsAndCommitRoot() {
+        NavigableMap<Revision, String> revisions =
+                new TreeMap<Revision, String>(context.getRevisionComparator());
+        for (Map.Entry<Revision, String> entry : 
doc.getLocalRevisions().entrySet()) {
+            if (splitRevs.contains(entry.getKey())) {
+                revisions.put(entry.getKey(), entry.getValue());
+                numValues++;
+            } else {
+                // move _revisions entries that act as commit root without
+                // local changes
+                if (context.getClusterId() != entry.getKey().getClusterId()) {
+                    // only consider local changes
+                    continue;
+                }
+                if (doc.isCommitted(entry.getKey())
+                        && !mostRecentRevs.contains(entry.getKey())) {
+                    // this is a commit root for changes in other documents
+                    revisions.put(entry.getKey(), entry.getValue());
+                    numValues++;
+                    trackHigh(entry.getKey());
+                    trackLow(entry.getKey());
+                }
+            }
+        }
+        committedChanges.put(REVISIONS, revisions);
+        NavigableMap<Revision, String> commitRoot =
+                new TreeMap<Revision, String>(context.getRevisionComparator());
+        for (Map.Entry<Revision, String> entry : 
doc.getLocalCommitRoot().entrySet()) {
+            if (splitRevs.contains(entry.getKey())) {
+                commitRoot.put(entry.getKey(), entry.getValue());
+                numValues++;
+            }
+        }
+        committedChanges.put(COMMIT_ROOT, commitRoot);
+    }
+
+    /**
+     * Creates {@link UpdateOp}s for intermediate documents if necessary.
+     */
+    private void createIntermediateDocs() {
+        // collect ranges and create a histogram of the height
+        Map<Integer, List<Range>> prevHisto = getPreviousDocsHistogram();
+        // check if we need to create intermediate previous documents
+        for (Map.Entry<Integer, List<Range>> entry : prevHisto.entrySet()) {
+            if (entry.getValue().size() >= PREV_SPLIT_FACTOR) {
+                if (main == null) {
+                    main = new UpdateOp(id, false);
+                }
+                // calculate range
+                Revision h = null;
+                Revision l = null;
+                for (Range r : entry.getValue()) {
+                    if (h == null || isRevisionNewer(context, r.high, h)) {
+                        h = r.high;
+                    }
+                    if (l == null || isRevisionNewer(context, l, r.low)) {
+                        l = r.low;
+                    }
+                    removePrevious(main, r);
+                }
+                if (h == null || l == null) {
+                    throw new IllegalStateException();
+                }
+                String prevPath = Utils.getPreviousPathFor(path, h, 
entry.getKey() + 1);
+                String prevId = Utils.getIdFromPath(prevPath);
+                UpdateOp intermediate = new UpdateOp(prevId, true);
+                intermediate.set(Document.ID, prevId);
+                if (Utils.isLongPath(prevPath)) {
+                    intermediate.set(NodeDocument.PATH, prevPath);
+                }
+                setPrevious(main, new Range(h, l, entry.getKey() + 1));
+                for (Range r : entry.getValue()) {
+                    setPrevious(intermediate, r);
+                }
+                setIntermediateDocProps(intermediate, h);
+                splitOps.add(intermediate);
+            }
+        }
+    }
+
+    /**
+     * Creates split {@link UpdateOp} if there is enough data to split off. The
+     * {@link UpdateOp} for the new previous document is placed into the list 
of
+     * {@link #splitOps}. The {@link UpdateOp} for the main document is not
+     * added to the list but rather returned.
+     *
+     * @return the UpdateOp for the main document or {@code null} if there is
+     *          not enough data to split.
+     */
+    @CheckForNull
+    private UpdateOp createSplitOps() {
+        UpdateOp main = null;
+        // check if we have enough data to split off
+        if (high != null && low != null
+                && (numValues >= NUM_REVS_THRESHOLD
+                || doc.getMemory() > DOC_SIZE_THRESHOLD)) {
+            // enough changes to split off
+            // move to another document
+            main = new UpdateOp(id, false);
+            setPrevious(main, new Range(high, low, 0));
+            String oldPath = Utils.getPreviousPathFor(path, high, 0);
+            UpdateOp old = new UpdateOp(Utils.getIdFromPath(oldPath), true);
+            old.set(Document.ID, old.getId());
+            if (Utils.isLongPath(oldPath)) {
+                old.set(NodeDocument.PATH, oldPath);
+            }
+            for (String property : committedChanges.keySet()) {
+                NavigableMap<Revision, String> splitMap = 
committedChanges.get(property);
+                for (Map.Entry<Revision, String> entry : splitMap.entrySet()) {
+                    Revision r = entry.getKey();
+                    if (isRevisionsEntry(property) || 
isCommitRootEntry(property)) {
+                        // only remove from main document if it is not
+                        // referenced anymore from from most recent changes
+                        if (!mostRecentRevs.contains(r)) {
+                            main.removeMapEntry(property, r);
+                        }
+                    } else {
+                        main.removeMapEntry(property, r);
+                    }
+                    old.setMapEntry(property, r, entry.getValue());
+                }
+            }
+            // check size of old document
+            NodeDocument oldDoc = new NodeDocument(STORE);
+            UpdateUtils.applyChanges(oldDoc, old, 
context.getRevisionComparator());
+            setSplitDocProps(doc, oldDoc, old, high);
+            // only split if enough of the data can be moved to old document
+            if (oldDoc.getMemory() > doc.getMemory() * SPLIT_RATIO
+                    || numValues >= NUM_REVS_THRESHOLD) {
+                splitOps.add(old);
+            } else {
+                main = null;
+            }
+        }
+        return main;
+    }
+
+    /**
+     * Returns a histogram of the height of the previous documents referenced
+     * by this document. This only includes direct references and not 
indirectly
+     * referenced previous documents through intermediate previous docs.
+     *
+     * @return histogram of the height of the previous documents.
+     */
+    private Map<Integer, List<Range>> getPreviousDocsHistogram() {
+        Map<Integer, List<Range>> prevHisto = Maps.newHashMap();
+        for (Map.Entry<Revision, Range> entry : 
doc.getPreviousRanges().entrySet()) {
+            Revision rev = entry.getKey();
+            if (rev.getClusterId() != context.getClusterId()) {
+                continue;
+            }
+            Range r = entry.getValue();
+            List<Range> list = prevHisto.get(r.getHeight());
+            if (list == null) {
+                list = new ArrayList<Range>();
+                prevHisto.put(r.getHeight(), list);
+            }
+            list.add(r);
+        }
+        return prevHisto;
+    }
+
+    /**
+     * Returns a map of all local property changes committed by the current
+     * cluster node.
+     *
+     * @return local changes committed by the current cluster node.
+     */
+    @Nonnull
+    private Map<String, NavigableMap<Revision, String>> 
getCommittedLocalChanges() {
+        Map<String, NavigableMap<Revision, String>> committedLocally
+                = new HashMap<String, NavigableMap<Revision, String>>();
+        for (String property : doc.keySet()) {
+            if (IGNORE_ON_SPLIT.contains(property)
+                    || isRevisionsEntry(property)
+                    || isCommitRootEntry(property)) {
+                continue;
+            }
+            NavigableMap<Revision, String> splitMap
+                    = new TreeMap<Revision, 
String>(context.getRevisionComparator());
+            committedLocally.put(property, splitMap);
+            Map<Revision, String> valueMap = doc.getLocalMap(property);
+            // collect committed changes of this cluster node
+            for (Map.Entry<Revision, String> entry : valueMap.entrySet()) {
+                Revision rev = entry.getKey();
+                if (rev.getClusterId() != context.getClusterId()) {
+                    continue;
+                }
+                if (doc.isCommitted(rev)) {
+                    splitMap.put(rev, entry.getValue());
+                }
+            }
+        }
+        return committedLocally;
+    }
+
+    private void trackHigh(Revision r) {
+        if (high == null || isRevisionNewer(context, r, high)) {
+            high = r;
+        }
+    }
+
+    private void trackLow(Revision r) {
+        if (low == null || isRevisionNewer(context, low, r)) {
+            low = r;
+        }
+    }
+
+    /**
+     * Set various split document related flag/properties
+     *
+     * @param mainDoc main document from which split document is being created
+     * @param old updateOp of the old document created via split
+     * @param oldDoc old document created via split
+     * @param maxRev max revision stored in the split document oldDoc
+     */
+    private static void setSplitDocProps(NodeDocument mainDoc, NodeDocument 
oldDoc,
+                                         UpdateOp old, Revision maxRev) {
+        setSplitDocMaxRev(old, maxRev);
+
+        SplitDocType type = SplitDocType.DEFAULT;
+        if (!mainDoc.hasChildren()) {
+            type = SplitDocType.DEFAULT_LEAF;
+        } else if (oldDoc.getLocalRevisions().isEmpty()) {
+            type = SplitDocType.COMMIT_ROOT_ONLY;
+        }
+
+        // Copy over the hasBinary flag
+        if (mainDoc.hasBinary()) {
+            setHasBinary(old);
+        }
+
+        setSplitDocType(old, type);
+    }
+
+    /**
+     * Set various properties for intermediate split document
+     *
+     * @param intermediate updateOp of the intermediate doc getting created
+     * @param maxRev max revision stored in the intermediate
+     */
+    private static void setIntermediateDocProps(UpdateOp intermediate, 
Revision maxRev) {
+        setSplitDocMaxRev(intermediate, maxRev);
+        setSplitDocType(intermediate, SplitDocType.INTERMEDIATE);
+    }
+
+    //----------------------------< internal modifiers 
>------------------------
+
+    private static void setSplitDocType(@Nonnull UpdateOp op,
+                                        @Nonnull SplitDocType type) {
+        checkNotNull(op).set(NodeDocument.SD_TYPE, type.type);
+    }
+
+    private static void setSplitDocMaxRev(@Nonnull UpdateOp op,
+                                          @Nonnull Revision maxRev) {
+        checkNotNull(op).set(NodeDocument.SD_MAX_REV_TIME_IN_SECS, 
NodeDocument.getModifiedInSecs(maxRev.getTimestamp()));
+    }
+
+}

Propchange: 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java?rev=1607557&r1=1607556&r2=1607557&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
 Thu Jul  3 07:35:38 2014
@@ -20,7 +20,7 @@
 package org.apache.jackrabbit.oak.plugins.document;
 
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.EnumSet;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
@@ -35,6 +35,9 @@ import org.apache.jackrabbit.oak.plugins
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType.COMMIT_ROOT_ONLY;
+import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType.DEFAULT_LEAF;
+
 public class VersionGarbageCollector {
     private final DocumentNodeStore nodeStore;
     private final VersionGCSupport versionStore;
@@ -42,11 +45,10 @@ public class VersionGarbageCollector {
     private final Logger log = LoggerFactory.getLogger(getClass());
 
     /**
-     * Split document types which can be safely Garbage Collected
-     * OAK-1793: SplitDocType.DEFAULT_NO_CHILD and 
SplitDocType.PROP_COMMIT_ONLY
-     * have been removed, but should be added again when OAK-1794 is fixed.
+     * Split document types which can be safely garbage collected
      */
-    private static final Set<NodeDocument.SplitDocType> GC_TYPES = 
Collections.emptySet();
+    private static final Set<NodeDocument.SplitDocType> GC_TYPES = EnumSet.of(
+            DEFAULT_LEAF, COMMIT_ROOT_ONLY);
 
     VersionGarbageCollector(DocumentNodeStore nodeStore) {
         this.nodeStore = nodeStore;
@@ -81,8 +83,7 @@ public class VersionGarbageCollector {
         }
 
         collectDeletedDocuments(stats, headRevision, oldestRevTimeStamp);
-        // FIXME: OAK-1793 and OAK-1794
-        // collectSplitDocuments(stats, oldestRevTimeStamp);
+        collectSplitDocuments(stats, oldestRevTimeStamp);
 
         sw.stop();
         log.info("Version garbage collected in {}. {}", sw, stats);

Modified: 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java?rev=1607557&r1=1607556&r2=1607557&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
 Thu Jul  3 07:35:38 2014
@@ -37,6 +37,7 @@ import com.mongodb.BasicDBObject;
 import org.apache.commons.codec.binary.Hex;
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.plugins.document.Revision;
+import org.apache.jackrabbit.oak.plugins.document.RevisionContext;
 import org.bson.types.ObjectId;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -447,4 +448,18 @@ public class Utils {
     public static String timestampToString(long timestamp){
         return (new Timestamp(timestamp) + "00").substring(0, 23);
     }
+
+    /**
+     * Checks that revision x is newer than another revision.
+     *
+     * @param x the revision to check
+     * @param previous the presumed earlier revision
+     * @return true if x is newer
+     */
+    public static boolean isRevisionNewer(@Nonnull RevisionContext context,
+                                          @Nonnull Revision x,
+                                          @Nonnull Revision previous) {
+        return context.getRevisionComparator().compare(x, previous) > 0;
+    }
+
 }

Modified: 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java?rev=1607557&r1=1607556&r2=1607557&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java
 Thu Jul  3 07:35:38 2014
@@ -35,6 +35,7 @@ import org.apache.jackrabbit.oak.plugins
 import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
 import org.apache.jackrabbit.oak.spi.commit.EmptyHook;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.apache.jackrabbit.oak.spi.state.NodeStore;
 import org.junit.Test;
 
 import com.google.common.collect.Iterables;
@@ -45,11 +46,11 @@ import static org.apache.jackrabbit.oak.
 import static 
org.apache.jackrabbit.oak.plugins.document.MongoBlobGCTest.randomStream;
 import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.NUM_REVS_THRESHOLD;
 import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.PREV_SPLIT_FACTOR;
-import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.SPLIT_RATIO;
 import static 
org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType;
 import static 
org.apache.jackrabbit.oak.plugins.document.UpdateOp.Operation.Type.REMOVE_MAP_ENTRY;
 import static 
org.apache.jackrabbit.oak.plugins.document.UpdateOp.Operation.Type.SET_MAP_ENTRY;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -152,8 +153,10 @@ public class DocumentSplitTest extends B
         doc = store.find(NODES, Utils.getIdFromPath("/foo"));
         assertNotNull(doc);
         Map<Revision, String> commits = doc.getLocalCommitRoot();
-        // one remaining in the local commit root map
-        assertEquals(1, commits.size());
+        // two remaining in the local commit root map
+        // the first _commitRoot entry for the _deleted when the node was 
created
+        // the second _commitRoot entry for the most recent prop change
+        assertEquals(2, commits.size());
         for (Revision rev : commitRoots) {
             assertTrue(doc.isCommitted(rev));
         }
@@ -332,7 +335,7 @@ public class DocumentSplitTest extends B
         NodeDocument doc = store.find(NODES, Utils.getIdFromPath("/test/foo"));
         List<NodeDocument> prevDocs = 
ImmutableList.copyOf(doc.getAllPreviousDocs());
         assertEquals(1, prevDocs.size());
-        assertEquals(SplitDocType.DEFAULT_NO_CHILD, 
prevDocs.get(0).getSplitDocType());
+        assertEquals(SplitDocType.DEFAULT_LEAF, 
prevDocs.get(0).getSplitDocType());
     }
 
     @Test
@@ -355,7 +358,7 @@ public class DocumentSplitTest extends B
         NodeDocument doc = store.find(NODES, Utils.getIdFromPath("/test/foo"));
         List<NodeDocument> prevDocs = 
ImmutableList.copyOf(doc.getAllPreviousDocs());
         assertEquals(1, prevDocs.size());
-        assertEquals(SplitDocType.PROP_COMMIT_ONLY, 
prevDocs.get(0).getSplitDocType());
+        assertEquals(SplitDocType.COMMIT_ROOT_ONLY, 
prevDocs.get(0).getSplitDocType());
     }
 
     @Test
@@ -587,6 +590,78 @@ public class DocumentSplitTest extends B
         assertEquals(id, splitOps.get(1).getId());
     }
 
+    // OAK-1794
+    @Test
+    public void keepRevisionsForMostRecentChanges() throws Exception {
+        DocumentStore store = mk.getDocumentStore();
+        NodeStore ns = mk.getNodeStore();
+        NodeBuilder builder = ns.getRoot().builder();
+        builder.setProperty("foo", -1);
+        builder.setProperty("bar", -1);
+        ns.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        for (int i = 0; i < NUM_REVS_THRESHOLD; i++) {
+            builder = ns.getRoot().builder();
+            builder.setProperty("foo", i);
+            ns.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        }
+        mk.runBackgroundOperations();
+        NodeDocument doc = store.find(NODES, Utils.getIdFromPath("/"));
+        assertNotNull(doc);
+        // the local _revisions map must still contain the entry for
+        // the initial 'bar' property
+        Map<Revision, String> valueMap = doc.getValueMap("bar");
+        assertFalse(valueMap.isEmpty());
+        Revision r = valueMap.keySet().iterator().next();
+        assertTrue(doc.getLocalRevisions().containsKey(r));
+        // but also the previous document must contain the revision
+        List<NodeDocument> prevDocs = 
Lists.newArrayList(doc.getAllPreviousDocs());
+        assertEquals(1, prevDocs.size());
+        NodeDocument prev = prevDocs.get(0);
+        assertTrue(prev.getLocalRevisions().containsKey(r));
+    }
+
+    // OAK-1794
+    @Test
+    public void keepCommitRootForMostRecentChanges() throws Exception {
+        DocumentStore store = mk.getDocumentStore();
+        NodeStore ns = mk.getNodeStore();
+        NodeBuilder builder = ns.getRoot().builder();
+        builder.setProperty("p", -1);
+        NodeBuilder test = builder.child("test");
+        test.setProperty("foo", -1);
+        test.setProperty("bar", -1);
+        ns.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        for (int i = 0; i < NUM_REVS_THRESHOLD; i++) {
+            builder = ns.getRoot().builder();
+            builder.setProperty("p", i);
+            test = builder.child("test");
+            test.setProperty("foo", i);
+            ns.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        }
+        mk.runBackgroundOperations();
+        NodeDocument doc = store.find(NODES, Utils.getIdFromPath("/test"));
+        assertNotNull(doc);
+        // the local _commitRoot map must still contain the entry for
+        // the initial 'bar' property
+        Map<Revision, String> valueMap = doc.getValueMap("bar");
+        assertFalse(valueMap.isEmpty());
+        Revision r = valueMap.keySet().iterator().next();
+        assertTrue(doc.getLocalCommitRoot().containsKey(r));
+        // but also the previous document must contain the commitRoot entry
+        List<NodeDocument> prevDocs = 
Lists.newArrayList(doc.getAllPreviousDocs());
+        assertEquals(1, prevDocs.size());
+        NodeDocument prev = prevDocs.get(0);
+        assertTrue(prev.getLocalCommitRoot().containsKey(r));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void splitPreviousDocument() {
+        NodeDocument doc = new NodeDocument(mk.getDocumentStore());
+        doc.put(NodeDocument.ID, Utils.getIdFromPath("/test"));
+        doc.put(NodeDocument.SD_TYPE, NodeDocument.SplitDocType.DEFAULT.type);
+        SplitOperations.forDocument(doc, DummyRevisionContext.INSTANCE);
+    }
+
     private void syncMKs(List<DocumentMK> mks, int idx) {
         mks.get(idx).runBackgroundOperations();
         for (int i = 0; i < mks.size(); i++) {

Added: 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DummyRevisionContext.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DummyRevisionContext.java?rev=1607557&view=auto
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DummyRevisionContext.java
 (added)
+++ 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DummyRevisionContext.java
 Thu Jul  3 07:35:38 2014
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.document;
+
+import java.util.Comparator;
+
+/**
+ * A revision context for tests.
+ */
+public class DummyRevisionContext implements RevisionContext {
+
+    static final RevisionContext INSTANCE = new DummyRevisionContext();
+
+    private final Comparator<Revision> comparator
+            = StableRevisionComparator.INSTANCE;
+
+    @Override
+    public UnmergedBranches getBranches() {
+        return new UnmergedBranches(comparator);
+    }
+
+    @Override
+    public UnsavedModifications getPendingModifications() {
+        return new UnsavedModifications();
+    }
+
+    @Override
+    public Comparator<Revision> getRevisionComparator() {
+        return comparator;
+    }
+
+    @Override
+    public int getClusterId() {
+        return 1;
+    }
+}

Propchange: 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DummyRevisionContext.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java?rev=1607557&r1=1607556&r2=1607557&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
 Thu Jul  3 07:35:38 2014
@@ -16,8 +16,6 @@
  */
 package org.apache.jackrabbit.oak.plugins.document;
 
-import java.util.Comparator;
-
 import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
 import org.apache.jackrabbit.oak.plugins.document.util.Utils;
 import org.junit.Test;
@@ -40,32 +38,6 @@ public class NodeDocumentTest {
             NodeDocument.addCollision(op, r);
         }
         UpdateUtils.applyChanges(doc, op, StableRevisionComparator.INSTANCE);
-        doc.split(CONTEXT);
+        doc.split(DummyRevisionContext.INSTANCE);
     }
-
-    private static final RevisionContext CONTEXT = new RevisionContext() {
-
-        private final Comparator<Revision> comparator
-                = StableRevisionComparator.INSTANCE;
-
-        @Override
-        public UnmergedBranches getBranches() {
-            return new UnmergedBranches(comparator);
-        }
-
-        @Override
-        public UnsavedModifications getPendingModifications() {
-            return new UnsavedModifications();
-        }
-
-        @Override
-        public Comparator<Revision> getRevisionComparator() {
-            return comparator;
-        }
-
-        @Override
-        public int getClusterId() {
-            return 1;
-        }
-    };
 }

Modified: 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorTest.java?rev=1607557&r1=1607556&r2=1607557&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorTest.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorTest.java
 Thu Jul  3 07:35:38 2014
@@ -49,7 +49,6 @@ import org.apache.jackrabbit.oak.spi.sta
 import org.apache.jackrabbit.oak.stats.Clock;
 import org.junit.After;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -167,7 +166,6 @@ public class VersionGarbageCollectorTest
 
     }
 
-    @Ignore("OAK-1794")
     @Test
     public void gcSplitDocs() throws Exception{
         long maxAge = 1; //hrs
@@ -200,8 +198,8 @@ public class VersionGarbageCollectorTest
         assertEquals(1, previousDocTestFoo.size());
         assertEquals(1, previousDocTestFoo2.size());
 
-        assertEquals(SplitDocType.PROP_COMMIT_ONLY, 
previousDocTestFoo.get(0).getSplitDocType());
-        assertEquals(SplitDocType.DEFAULT_NO_CHILD, 
previousDocTestFoo2.get(0).getSplitDocType());
+        assertEquals(SplitDocType.COMMIT_ROOT_ONLY, 
previousDocTestFoo.get(0).getSplitDocType());
+        assertEquals(SplitDocType.DEFAULT_LEAF, 
previousDocTestFoo2.get(0).getSplitDocType());
 
         clock.waitUntil(clock.getTime() + TimeUnit.HOURS.toMillis(maxAge) + 
delta);
         VersionGCStats stats = gc.gc(maxAge, TimeUnit.HOURS);
@@ -217,7 +215,6 @@ public class VersionGarbageCollectorTest
     }
 
     // OAK-1729
-    @Ignore("OAK-1794")
     @Test
     public void gcIntermediateDocs() throws Exception {
         long maxAge = 1; //hrs
@@ -225,7 +222,7 @@ public class VersionGarbageCollectorTest
 
         NodeBuilder b1 = store.getRoot().builder();
         // adding the foo node will cause the commit root to be placed
-        // on the rood document, because the children flag is set on the
+        // on the root document, because the children flag is set on the
         // root document
         b1.child("foo");
         store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
@@ -235,7 +232,7 @@ public class VersionGarbageCollectorTest
         store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
         assertTrue(!getDoc("/test").getLocalRevisions().isEmpty());
 
-        for (int i = 0; i < PREV_SPLIT_FACTOR + 1; i++) {
+        for (int i = 0; i < PREV_SPLIT_FACTOR; i++) {
             for (int j = 0; j < NUM_REVS_THRESHOLD; j++) {
                 b1 = store.getRoot().builder();
                 b1.child("test").setProperty("prop", i * NUM_REVS_THRESHOLD + 
j);
@@ -243,6 +240,10 @@ public class VersionGarbageCollectorTest
             }
             store.runBackgroundOperations();
         }
+        // trigger another split, now that we have 10 previous docs
+        // this will create an intermediate previous doc
+        store.addSplitCandidate(Utils.getIdFromPath("/test"));
+        store.runBackgroundOperations();
 
         Map<Revision, Range> prevRanges = getDoc("/test").getPreviousRanges();
         boolean hasIntermediateDoc = false;
@@ -319,8 +320,7 @@ public class VersionGarbageCollectorTest
         clock.waitUntil(clock.getTime() + TimeUnit.HOURS.toMillis(maxAge) + 
delta);
 
         VersionGCStats stats = gc.gc(maxAge, TimeUnit.HOURS);
-        // TODO: uncomment once OAK-1794 is fixed
-        // assertEquals(2, stats.splitDocGCCount);
+        assertEquals(2, stats.splitDocGCCount);
 
         NodeDocument doc = getDoc("/foo");
         assertNotNull(doc);


Reply via email to