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 mr...@apache.org on 2016/10/26 15:18:40 UTC
svn commit: r1766692 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/
test/java/org/apache/jackrabbit/oak/plugins/document/
Author: mreutegg
Date: Wed Oct 26 15:18:40 2016
New Revision: 1766692
URL: http://svn.apache.org/viewvc?rev=1766692&view=rev
Log:
OAK-5010: Document split with binary properties too eager
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevs.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.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/VersionGCQueryTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevs.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevs.java?rev=1766692&r1=1766691&r2=1766692&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevs.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevs.java Wed Oct 26 15:18:40 2016
@@ -16,6 +16,7 @@
*/
package org.apache.jackrabbit.oak.plugins.document;
+import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
@@ -39,6 +40,10 @@ final class LastRevs implements Iterable
private Revision branchRev;
+ LastRevs(RevisionVector readRevision) {
+ this(Collections.<Integer, Revision>emptyMap(), readRevision, null);
+ }
+
LastRevs(Map<Integer, Revision> revs,
RevisionVector readRevision,
Branch branch) {
Modified: 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=1766692&r1=1766691&r2=1766692&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java Wed Oct 26 15:18:40 2016
@@ -36,6 +36,8 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.base.Predicate;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
@@ -76,7 +78,8 @@ class SplitOperations {
private Revision high;
private Revision low;
private int numValues;
- private boolean hasBinary;
+ private boolean hasBinaryToSplit;
+ private Supplier<Boolean> nodeExistsAtHeadRevision;
private Map<String, NavigableMap<Revision, String>> committedChanges;
private Set<Revision> changes;
private Map<String, Set<Revision>> garbage;
@@ -86,18 +89,26 @@ class SplitOperations {
private List<UpdateOp> splitOps;
private UpdateOp main;
- private SplitOperations(@Nonnull NodeDocument doc,
- @Nonnull RevisionContext context,
- @Nonnull RevisionVector headRevision,
- @Nonnull Predicate<String> isBinaryValue,
+ private SplitOperations(@Nonnull final NodeDocument doc,
+ @Nonnull final RevisionContext context,
+ @Nonnull final RevisionVector headRev,
+ @Nonnull final Predicate<String> isBinaryValue,
int numRevsThreshold) {
this.doc = checkNotNull(doc);
this.context = checkNotNull(context);
this.isBinaryValue = checkNotNull(isBinaryValue);
this.path = doc.getPath();
this.id = doc.getId();
- this.headRevision = checkNotNull(headRevision).getRevision(context.getClusterId());
+ this.headRevision = checkNotNull(headRev).getRevision(context.getClusterId());
this.numRevsThreshold = numRevsThreshold;
+ this.nodeExistsAtHeadRevision = Suppliers.memoize(new Supplier<Boolean>() {
+ @Override
+ public Boolean get() {
+ return doc.getLiveRevision(context, headRev,
+ Maps.<Revision, String>newHashMap(),
+ new LastRevs(headRev)) != null;
+ }
+ });
}
/**
@@ -200,7 +211,8 @@ class SplitOperations {
Revision r = splitMap.lastKey();
splitMap.remove(r);
splitRevs.addAll(splitMap.keySet());
- hasBinary |= hasBinaryProperty(splitMap.values());
+ hasBinaryToSplit |= hasBinaryProperty(splitMap.values())
+ && nodeExistsAtHeadRevision.get();
mostRecentRevs.add(r);
}
if (splitMap.isEmpty()) {
@@ -328,7 +340,7 @@ class SplitOperations {
if (high != null && low != null
&& (numValues >= numRevsThreshold
|| doc.getMemory() > DOC_SIZE_THRESHOLD
- || hasBinary)) {
+ || hasBinaryToSplit)) {
// enough changes to split off
// move to another document
main = new UpdateOp(id, false);
@@ -363,7 +375,7 @@ class SplitOperations {
// or there are binaries to split off
if (oldDoc.getMemory() > doc.getMemory() * SPLIT_RATIO
|| numValues >= numRevsThreshold
- || hasBinary) {
+ || hasBinaryToSplit) {
splitOps.add(old);
} else {
main = null;
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=1766692&r1=1766691&r2=1766692&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 Wed Oct 26 15:18:40 2016
@@ -921,6 +921,38 @@ public class DocumentSplitTest extends B
assertEquals(9, prevDocs.size());
}
+ @Test
+ public void noBinarySplitWhenRemoved() throws Exception {
+ DocumentStore store = mk.getDocumentStore();
+ DocumentNodeStore ns = mk.getNodeStore();
+ NodeBuilder builder = ns.getRoot().builder();
+ PropertyState binary = binaryProperty("p", "value".getBytes());
+ builder.child("foo").setProperty(binary);
+ merge(ns, builder);
+
+ builder = ns.getRoot().builder();
+ builder.child("foo").remove();
+ merge(ns, builder);
+ ns.runBackgroundOperations();
+
+ // must not create split document in this case. See OAK-5010
+ NodeDocument foo = store.find(NODES, Utils.getIdFromPath("/foo"));
+ assertNotNull(foo);
+ assertEquals(0, foo.getPreviousRanges().size());
+
+ // re-create it
+ builder = ns.getRoot().builder();
+ builder.child("foo");
+ merge(ns, builder);
+ ns.runBackgroundOperations();
+
+ // now the old binary value must be moved to a previous document
+ foo = store.find(NODES, Utils.getIdFromPath("/foo"));
+ assertNotNull(foo);
+ List<NodeDocument> prevDocs = copyOf(foo.getAllPreviousDocs());
+ assertEquals(1, prevDocs.size());
+ }
+
private static class TestRevisionContext implements RevisionContext {
private final RevisionContext rc;
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCQueryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCQueryTest.java?rev=1766692&r1=1766691&r2=1766692&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCQueryTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCQueryTest.java Wed Oct 26 15:18:40 2016
@@ -89,6 +89,15 @@ public class VersionGCQueryTest {
builder.child("test").child("node-" + i).setProperty(p);
}
merge(builder);
+ // overwrite with other binaries to force document splits
+ builder = ns.getRoot().builder();
+ for (int i = 0; i < 10; i++) {
+ InputStream s = new RandomStream(10 * 1024, 17);
+ PropertyState p = new BinaryPropertyState("p", ns.createBlob(s));
+ builder.child("test").child("node-" + i).setProperty(p);
+ }
+ merge(builder);
+ ns.runBackgroundOperations();
builder = ns.getRoot().builder();
builder.child("test").remove();
merge(builder);
@@ -135,7 +144,7 @@ public class VersionGCQueryTest {
assertEquals(1, stats.deletedDocGCCount);
assertEquals(numPrevDocs, stats.splitDocGCCount);
assertEquals(numPrevDocs, prevDocIds.size());
- assertEquals(1, Iterables.size(Utils.getAllDocuments(store)));
+ assertEquals(2, Iterables.size(Utils.getAllDocuments(store)));
}
private NodeState merge(NodeBuilder builder) throws CommitFailedException {