Author: mreutegg
Date: Mon Apr 14 15:26:38 2014
New Revision: 1587224
URL: http://svn.apache.org/r1587224
Log:
OAK-1729: DocumentNodeStore revision GC removes intermediate docs
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/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentStoreFixture.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=1587224&r1=1587223&r2=1587224&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
Mon Apr 14 15:26:38 2014
@@ -51,10 +51,12 @@ 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;
@@ -1339,7 +1341,7 @@ public final class NodeDocument extends
setSplitDocMaxRev(old, maxRev);
SplitDocType type = SplitDocType.DEFAULT;
- if(!mainDoc.hasChildren()){
+ if(!mainDoc.hasChildren() && !referencesOldDocAfterSplit(mainDoc,
oldDoc)){
type = SplitDocType.DEFAULT_NO_CHILD;
} else if (oldDoc.getLocalRevisions().isEmpty()){
type = SplitDocType.PROP_COMMIT_ONLY;
@@ -1354,6 +1356,31 @@ public final class NodeDocument extends
}
/**
+ * 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
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=1587224&r1=1587223&r2=1587224&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
Mon Apr 14 15:26:38 2014
@@ -46,8 +46,7 @@ public class VersionGarbageCollector {
*/
private static final Set<NodeDocument.SplitDocType> GC_TYPES = EnumSet.of(
NodeDocument.SplitDocType.DEFAULT_NO_CHILD,
- NodeDocument.SplitDocType.PROP_COMMIT_ONLY,
- NodeDocument.SplitDocType.INTERMEDIATE);
+ NodeDocument.SplitDocType.PROP_COMMIT_ONLY);
VersionGarbageCollector(DocumentNodeStore nodeStore) {
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentStoreFixture.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentStoreFixture.java?rev=1587224&r1=1587223&r2=1587224&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentStoreFixture.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentStoreFixture.java
Mon Apr 14 15:26:38 2014
@@ -51,8 +51,6 @@ public abstract class DocumentStoreFixtu
public static class MemoryFixture extends DocumentStoreFixture {
- DocumentStore ds = new MemoryDocumentStore();
-
@Override
public String getName() {
return "Memory";
@@ -60,7 +58,7 @@ public abstract class DocumentStoreFixtu
@Override
public DocumentStore createDocumentStore() {
- return ds;
+ return new MemoryDocumentStore();
}
}
@@ -75,7 +73,7 @@ public abstract class DocumentStoreFixtu
DataSource datas = RDBDataSourceFactory.forJdbcUrl(url,
username, passwd);
this.ds = new RDBDocumentStore(datas, new
DocumentMK.Builder());
} catch (Exception ex) {
- LOG.info("Database instance not available at " + url + ",
skipping tests...", ex);
+ LOG.info("Database instance not available at " + url + ",
skipping tests...");
}
}
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=1587224&r1=1587223&r2=1587224&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
Mon Apr 14 15:26:38 2014
@@ -20,15 +20,19 @@
package org.apache.jackrabbit.oak.plugins.document;
import java.io.IOException;
-import java.util.*;
import java.util.Collection;
+import java.util.List;
+import java.util.Map;
import java.util.concurrent.TimeUnit;
-import static org.apache.jackrabbit.oak.plugins.document.Collection.*;
+import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
+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.SplitDocType;
import static
org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.VersionGCStats;
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;
@@ -206,6 +210,53 @@ public class VersionGarbageCollectorTest
//assertTrue(ImmutableList.copyOf(getDoc("/test2/foo").getAllPreviousDocs()).isEmpty());
}
+ // OAK-1729
+ @Test
+ public void gcIntermediateDocs() throws Exception {
+ long maxAge = 1; //hrs
+ long delta = TimeUnit.MINUTES.toMillis(10);
+
+ 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
+ // root document
+ b1.child("foo");
+ store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ // adding test afterwards will use the new test document as the
+ // commit root. this what we want for the test.
+ b1.child("test");
+ store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ assertTrue(!getDoc("/test").getLocalRevisions().isEmpty());
+
+ for (int i = 0; i < PREV_SPLIT_FACTOR + 1; i++) {
+ for (int j = 0; j < NUM_REVS_THRESHOLD; j++) {
+ b1 = store.getRoot().builder();
+ b1.child("test").setProperty("prop", i * NUM_REVS_THRESHOLD +
j);
+ store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ }
+ store.runBackgroundOperations();
+ }
+
+ Map<Revision, Range> prevRanges = getDoc("/test").getPreviousRanges();
+ boolean hasIntermediateDoc = false;
+ for (Map.Entry<Revision, Range> entry : prevRanges.entrySet()) {
+ if (entry.getValue().getHeight() > 0) {
+ hasIntermediateDoc = true;
+ break;
+ }
+ }
+ assertTrue("Test data does not have intermediate previous docs",
+ hasIntermediateDoc);
+
+ clock.waitUntil(clock.getTime() + TimeUnit.HOURS.toMillis(maxAge) +
delta);
+ VersionGCStats stats = gc.gc(maxAge, TimeUnit.HOURS);
+ assertEquals(10, stats.splitDocGCCount);
+
+ DocumentNodeState test = getDoc("/test").getNodeAtRevision(
+ store, store.getHeadRevision(), null);
+ assertNotNull(test);
+ }
+
private NodeDocument getDoc(String path){
return store.getDocumentStore().find(NODES, Utils.getIdFromPath(path),
0);
}
