You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ch...@apache.org on 2014/12/17 18:50:33 UTC

svn commit: r1646300 - in /jackrabbit/oak/branches/1.0: ./ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/ oak-core/src/test/java/org/apache/jackrabbit/oak/plugi...

Author: chetanm
Date: Wed Dec 17 17:50:33 2014
New Revision: 1646300

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

Merging 1607557,1608731

Modified:
    jackrabbit/oak/branches/1.0/   (props changed)
    jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
    jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
    jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
    jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java
    jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
    jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorTest.java

Propchange: jackrabbit/oak/branches/1.0/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Dec 17 17:50:33 2014
@@ -1,2 +1,2 @@
-/jackrabbit/oak/trunk:1584578,1584602,1584614,1584616,1584709,1584781,1584937,1585297,1585304-1585305,1585420,1585424,1585427,1585448,1585465,1585468,1585486,1585497,1585509,1585647,1585655-1585656,1585661,1585665-1585666,1585669-1585670,1585673,1585680,1585719,1585763,1585770,1585896,1585904,1585907,1585940,1585949,1585951,1585956,1585962-1585963,1586287,1586320,1586364,1586372,1586655,1586836,1587130,1587224,1587399,1587408,1587472,1587485,1587488,1587538,1587580,1587807,1588033,1588042,1588046,1588066,1588201,1589025,1589101,1589137,1589141,1589263,1589440,1589442,1589484,1589488,1589661,1589664,1589682,1589708,1589741,1589748,1589789,1589794,1589850,1589864,1590628,1590660,1590684,1590697,1590701,1590980,1590988,1591101,1591226,1591229,1591293,1591314,1591317,1591362,1591374,1591381,1591438,1591467,1591552,1591704,1591713,1591715,1591723,1591874,1592487,1592512,1592658,1592665,1592677,1592742,1592744,1592787,1592809,1592955,1593036,1593048,1593061,1593133,1593210-1593211,1593231
 ,1593245,1593250,1593294,1593304,1593317,1593342,1593554,1594158-1594164,1594166-1594167,1594169,1594237,1594800,1594808,1594835,1594888,1595147,1595457,1595856,1596241,1596474,1596534,1596844,1597569,1597795,1597854,1597860,1598292,1598302,1598352,1598369,1598595,1598631,1598696,1598732,1598797-1598798,1599299,1599332,1599416,1599434,1599671,1600088,1600935,1601309,1601388,1601578,1601676,1601757,1601768,1601814,1601833,1601838,1601853,1601878,1601888,1601922,1602156,1602174,1602179,1602183,1602207,1602227,1602256,1602261,1602796-1602797,1602800,1602809,1602853,1602872,1602914,1603155,1603307,1603401,1603441,1603748,1604166,1605030,1605036,1605038,1605292,1605447,1605526,1605670,1605725,1605831,1605852,1606077,1606079,1606087,1606638,1606641,1606644,1606708,1606711,1607031-1607032,1607077,1607127,1607141,1607152,1607185,1607196,1607331,1607362,1607366,1607392,1607526,1607664,1607737,1608560,1608783,1609064,1609081,1609165,1609488,1610489,1610592,1610603,1610634,1610658,1610664,1611
 021,1611041,1611275,1611277,1611313,1611332,1611584,1612560,1612825,1612993,1613018,1613041,1614265,1614272,1614344-1614345,1614384-1614385,1614397,1614405-1614406,1614574,1614591,1614593,1614596,1614604,1614689,1614807,1614835,1614891,1615417-1615418,1616182,1616236,1616463,1616719,1617417,1617451,1617463,1617711,1618158,1618613,1618624,1618709,1619222,1619411,1619695,1619800,1619808,1619815,1619823-1619824,1620512,1620581,1620585,1620634,1620898,1621115,1621123-1621124,1621168,1621192,1621201,1621706,1621962,1622197,1622201,1622207,1622250,1622479,1623364,1623766,1623827,1623949,1623969,1623973,1624216,1624317,1624551,1624973,1624993-1624994,1625025,1625036,1625158,1625224,1625237,1625299,1625348,1625620,1625916,1625962-1625963,1626021,1626053,1626163,1626168,1626175,1626191,1626265,1626770,1627047,1627052,1627228,1627346,1627470,1627473,1627479,1627503,1627586,1627590,1627715,1627731,1628180,1628198,1628262,1628447,1628608,1629688,1629840,1629917,1630055-1630057,1630156,1630299,1
 630338,1630773,1631283-1631284,1631333-1631334,1631617-1631619,1631630,1631699,1631704,1631711,1631967-1631969,1631986,1631990,1631999,1632002-1632003,1632017,1632258,1632264,1632270,1632293,1632303,1632592,1632605,1633315,1633559-1633560,1633562,1633567,1633571,1633598,1633608,1633641,1633687,1633697,1633768,1633783,1634505,1634513,1634774,1634779,1634781,1634792,1634803,1634814,1634816,1634838,1634841,1634852,1634864,1634896,1634898,1635044-1635045,1635060,1635077,1635089,1635102,1635108,1635218,1635387,1635435,1635518,1635563,1635586,1636336,1636348,1636505,1636585,1636799,1637368,1637382,1637413,1637651,1637815,1638779-1638783,1639260,1639577,1639622,1639963,1639966,1639973,1640134,1640143,1640555-1640556,1640694-1640695,1640715,1640722-1640723,1640728,1640863-1640872,1641340,1641350,1641352,1641541,1641596-1641599,1641601,1641662,1641671,1641695,1641771,1641802,1641811,1641950,1642031,1642056,1642119,1642285,1642648,1642667,1642954,1642959,1643111,1643178,1643186,1643204,164328
 7,1643767,1643774,1643982,1644016,1644106,1644366,1644383,1644397-1644398,1644407,1644479,1644547,1644552,1644554,1644650,1644654,1644689,1644750,1645421,1645424,1645459,1645585,1645611,1645637,1645646,1645660-1645663,1645888,1645901,1645948,1645966,1645970-1645971,1646014,1646164,1646174
+/jackrabbit/oak/trunk:1584578,1584602,1584614,1584616,1584709,1584781,1584937,1585297,1585304-1585305,1585420,1585424,1585427,1585448,1585465,1585468,1585486,1585497,1585509,1585647,1585655-1585656,1585661,1585665-1585666,1585669-1585670,1585673,1585680,1585719,1585763,1585770,1585896,1585904,1585907,1585940,1585949,1585951,1585956,1585962-1585963,1586287,1586320,1586364,1586372,1586655,1586836,1587130,1587224,1587399,1587408,1587472,1587485,1587488,1587538,1587580,1587807,1588033,1588042,1588046,1588066,1588201,1589025,1589101,1589137,1589141,1589263,1589440,1589442,1589484,1589488,1589661,1589664,1589682,1589708,1589741,1589748,1589789,1589794,1589850,1589864,1590628,1590660,1590684,1590697,1590701,1590980,1590988,1591101,1591226,1591229,1591293,1591314,1591317,1591362,1591374,1591381,1591438,1591467,1591552,1591704,1591713,1591715,1591723,1591874,1592487,1592512,1592658,1592665,1592677,1592742,1592744,1592787,1592809,1592955,1593036,1593048,1593061,1593133,1593210-1593211,1593231
 ,1593245,1593250,1593294,1593304,1593317,1593342,1593554,1594158-1594164,1594166-1594167,1594169,1594237,1594800,1594808,1594835,1594888,1595147,1595457,1595856,1596241,1596474,1596534,1596844,1597569,1597795,1597854,1597860,1598292,1598302,1598352,1598369,1598595,1598631,1598696,1598732,1598797-1598798,1599299,1599332,1599416,1599434,1599671,1600088,1600935,1601309,1601388,1601578,1601676,1601757,1601768,1601814,1601833,1601838,1601853,1601878,1601888,1601922,1602156,1602174,1602179,1602183,1602207,1602227,1602256,1602261,1602796-1602797,1602800,1602809,1602853,1602872,1602914,1603155,1603307,1603401,1603441,1603748,1604166,1605030,1605036,1605038,1605292,1605447,1605526,1605670,1605725,1605831,1605852,1606077,1606079,1606087,1606638,1606641,1606644,1606708,1606711,1607031-1607032,1607077,1607127,1607141,1607152,1607185,1607196,1607331,1607362,1607366,1607392,1607526,1607557,1607664,1607737,1608560,1608731,1608783,1609064,1609081,1609165,1609488,1610489,1610592,1610603,1610634,1610
 658,1610664,1611021,1611041,1611275,1611277,1611313,1611332,1611584,1612560,1612825,1612993,1613018,1613041,1614265,1614272,1614344-1614345,1614384-1614385,1614397,1614405-1614406,1614574,1614591,1614593,1614596,1614604,1614689,1614807,1614835,1614891,1615417-1615418,1616182,1616236,1616463,1616719,1617417,1617451,1617463,1617711,1618158,1618613,1618624,1618709,1619222,1619411,1619695,1619800,1619808,1619815,1619823-1619824,1620512,1620581,1620585,1620634,1620898,1621115,1621123-1621124,1621168,1621192,1621201,1621706,1621962,1622197,1622201,1622207,1622250,1622479,1623364,1623766,1623827,1623949,1623969,1623973,1624216,1624317,1624551,1624973,1624993-1624994,1625025,1625036,1625158,1625224,1625237,1625299,1625348,1625620,1625916,1625962-1625963,1626021,1626053,1626163,1626168,1626175,1626191,1626265,1626770,1627047,1627052,1627228,1627346,1627470,1627473,1627479,1627503,1627586,1627590,1627715,1627731,1628180,1628198,1628262,1628447,1628608,1629688,1629840,1629917,1630055-1630057,1
 630156,1630299,1630338,1630773,1631283-1631284,1631333-1631334,1631617-1631619,1631630,1631699,1631704,1631711,1631967-1631969,1631986,1631990,1631999,1632002-1632003,1632017,1632258,1632264,1632270,1632293,1632303,1632592,1632605,1633315,1633559-1633560,1633562,1633567,1633571,1633598,1633608,1633641,1633687,1633697,1633768,1633783,1634505,1634513,1634774,1634779,1634781,1634792,1634803,1634814,1634816,1634838,1634841,1634852,1634864,1634896,1634898,1635044-1635045,1635060,1635077,1635089,1635102,1635108,1635218,1635387,1635435,1635518,1635563,1635586,1636336,1636348,1636505,1636585,1636799,1637368,1637382,1637413,1637651,1637815,1638779-1638783,1639260,1639577,1639622,1639963,1639966,1639973,1640134,1640143,1640555-1640556,1640694-1640695,1640715,1640722-1640723,1640728,1640863-1640872,1641340,1641350,1641352,1641541,1641596-1641599,1641601,1641662,1641671,1641695,1641771,1641802,1641811,1641950,1642031,1642056,1642119,1642285,1642648,1642667,1642954,1642959,1643111,1643178,164318
 6,1643204,1643287,1643767,1643774,1643982,1644016,1644106,1644366,1644383,1644397-1644398,1644407,1644479,1644547,1644552,1644554,1644650,1644654,1644689,1644750,1645421,1645424,1645459,1645585,1645611,1645637,1645646,1645660-1645663,1645888,1645901,1645948,1645966,1645970-1645971,1646014,1646164,1646174
 /jackrabbit/trunk:1345480

Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1646300&r1=1646299&r2=1646300&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java Wed Dec 17 17:50:33 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.Map.Entry;
 import java.util.NavigableMap;
@@ -54,16 +52,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.
@@ -147,7 +143,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
@@ -258,17 +254,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;
@@ -298,7 +312,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);
 
@@ -896,7 +910,8 @@ public final class NodeDocument extends
             value = getLatestValue(context, getDeleted(),
                     null, maxRev, validRevisions);
         }
-        return value != null && value.value.equals("false") ? value.revision : null;
+
+        return value != null && "false".equals(value.value) ? value.revision : null;
     }
 
     /**
@@ -961,156 +976,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);
     }
 
     /**
@@ -1300,6 +1166,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));
@@ -1377,18 +1247,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 >----------------------------------
 
     /**
@@ -1496,82 +1354,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
@@ -1670,7 +1452,10 @@ public final class NodeDocument extends
 
     /**
      * Get the latest property value that is larger or equal the min revision,
-     * and smaller or equal the readRevision revision.
+     * and smaller or equal the readRevision revision. A {@code null} return
+     * value indicates that the property was not set or removed within the given
+     * range. A non-null value means the the property was either set or removed
+     * depending on {@link Value#value}.
      *
      * @param valueMap the sorted revision-value map
      * @param min the minimum revision (null meaning unlimited)
@@ -1685,8 +1470,6 @@ public final class NodeDocument extends
                                  @Nullable Revision min,
                                  @Nonnull Revision readRevision,
                                  @Nonnull Map<Revision, String> validRevisions) {
-        String value = null;
-        Revision latestRev = null;
         for (Map.Entry<Revision, String> entry : valueMap.entrySet()) {
             Revision propRev = entry.getKey();
             // ignore revisions newer than readRevision
@@ -1712,12 +1495,12 @@ public final class NodeDocument extends
             }
             if (isValidRevision(context, propRev, commitValue, readRevision, validRevisions)) {
                 // TODO: need to check older revisions as well?
-                latestRev = Utils.resolveCommitRevision(propRev, commitValue);
-                value = entry.getValue();
-                break;
+                return new Value(
+                        Utils.resolveCommitRevision(propRev, commitValue),
+                        entry.getValue());
             }
         }
-        return value != null ? new Value(value, latestRev) : null;
+        return null;
     }
 
     @Override
@@ -1901,12 +1684,16 @@ public final class NodeDocument extends
      */
     private static final class Value {
 
-        final String value;
         final Revision revision;
+        /**
+         * The value of a property at the given revision. A {@code null} value
+         * indicates the property was removed.
+         */
+        final String value;
 
-        Value(@Nonnull String value, @Nonnull Revision revision) {
-            this.value = checkNotNull(value);
+        Value(@Nonnull Revision revision, @Nullable String value) {
             this.revision = checkNotNull(revision);
+            this.value = value;
         }
     }
 }

Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java?rev=1646300&r1=1646299&r2=1646300&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java Wed Dec 17 17:50:33 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/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java?rev=1646300&r1=1646299&r2=1646300&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java Wed Dec 17 17:50:33 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/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java?rev=1646300&r1=1646299&r2=1646300&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java Wed Dec 17 17:50:33 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,126 @@ 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);
+    }
+
+    @Test
+    public void readLocalCommitInfo() throws Exception {
+        final Set<String> readSet = Sets.newHashSet();
+        DocumentStore store = new MemoryDocumentStore() {
+            @Override
+            public <T extends Document> T find(Collection<T> collection,
+                                               String key,
+                                               int maxCacheAge) {
+                readSet.add(key);
+                return super.find(collection, key, maxCacheAge);
+            }
+        };
+        DocumentNodeStore ns = new DocumentMK.Builder()
+                .setDocumentStore(store).setAsyncDelay(0).getNodeStore();
+        NodeBuilder builder = ns.getRoot().builder();
+        builder.child("test");
+        ns.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        for (int i = 0; i < NUM_REVS_THRESHOLD; i++) {
+            builder = ns.getRoot().builder();
+            builder.setProperty("p", i);
+            builder.child("test").setProperty("p", i);
+            builder.child("test").setProperty("q", i);
+            ns.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        }
+        builder = ns.getRoot().builder();
+        builder.child("test").removeProperty("q");
+        ns.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+        ns.runBackgroundOperations();
+
+        NodeDocument doc = store.find(NODES, Utils.getIdFromPath("/test"));
+        assertNotNull(doc);
+
+        readSet.clear();
+
+        // must not access previous document of /test
+        doc.getNodeAtRevision(ns, ns.getHeadRevision(), null);
+        for (String id : Sets.newHashSet(readSet)) {
+            doc = store.find(NODES, id);
+            assertNotNull(doc);
+            if (doc.isSplitDocument() && !doc.getMainPath().equals("/")) {
+                fail("must not access previous document: " + id);
+            }
+        }
+
+        ns.dispose();
+    }
+
     private void syncMKs(List<DocumentMK> mks, int idx) {
         mks.get(idx).runBackgroundOperations();
         for (int i = 0; i < mks.size(); i++) {

Modified: jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java?rev=1646300&r1=1646299&r2=1646300&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java Wed Dec 17 17:50:33 2014
@@ -16,17 +16,10 @@
  */
 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;
 
-import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.revisionAreAmbiguous;
-import static org.apache.jackrabbit.oak.plugins.document.Revision.RevisionComparator;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
 /**
  * Tests for {@link NodeDocument}.
  */
@@ -47,69 +40,4 @@ public class NodeDocumentTest {
         UpdateUtils.applyChanges(doc, op, StableRevisionComparator.INSTANCE);
         doc.split(DummyRevisionContext.INSTANCE);
     }
-
-    private static 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;
-        }
-    }
-
-    @Test
-    public void ambiguousRevisions() {
-        // revisions from same cluster node are not ambiguous
-        RevisionContext context = DummyRevisionContext.INSTANCE;
-        Revision r1 = new Revision(1, 0, 1);
-        Revision r2 = new Revision(2, 0, 1);
-        assertFalse(revisionAreAmbiguous(context, r1, r1));
-        assertFalse(revisionAreAmbiguous(context, r1, r2));
-        assertFalse(revisionAreAmbiguous(context, r2, r1));
-
-        // revisions from different cluster nodes are not ambiguous
-        // if seen with stable revision comparator
-        r1 = new Revision(1, 0, 2);
-        r2 = new Revision(2, 0, 1);
-        assertFalse(revisionAreAmbiguous(context, r1, r1));
-        assertFalse(revisionAreAmbiguous(context, r1, r2));
-        assertFalse(revisionAreAmbiguous(context, r2, r1));
-
-        // now use a revision comparator with seen-at support
-        final RevisionComparator comparator = new RevisionComparator(1);
-        context = new DummyRevisionContext() {
-            @Override
-            public Comparator<Revision> getRevisionComparator() {
-                return comparator;
-            }
-        };
-        r1 = new Revision(1, 0, 2);
-        r2 = new Revision(2, 0, 1);
-        // add revision to comparator in reverse time order
-        comparator.add(r2, new Revision(2, 0, 0));
-        comparator.add(r1, new Revision(3, 0, 0)); // r1 seen after r2
-        assertFalse(revisionAreAmbiguous(context, r1, r1));
-        assertFalse(revisionAreAmbiguous(context, r2, r2));
-        assertTrue(revisionAreAmbiguous(context, r1, r2));
-        assertTrue(revisionAreAmbiguous(context, r2, r1));
-    }
 }

Modified: jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorTest.java?rev=1646300&r1=1646299&r2=1646300&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorTest.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorTest.java Wed Dec 17 17:50:33 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 test 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("test");
         store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
@@ -237,7 +234,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);
@@ -245,6 +242,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;
@@ -322,8 +323,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);