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 ju...@apache.org on 2013/04/05 06:50:08 UTC
svn commit: r1464823 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/index/p2/
main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/
test/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/
Author: jukka
Date: Fri Apr 5 04:50:07 2013
New Revision: 1464823
URL: http://svn.apache.org/r1464823
Log:
OAK-748: ContentMirrorStoreStrategy #insert fails to enforce uniqueness and is slow
The uniqueness constraints should only be checked once the whole subtree has been processed
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexHook.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexHookUpdate.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/IndexStoreStrategy.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategyTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexHook.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexHook.java?rev=1464823&r1=1464822&r2=1464823&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexHook.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexHook.java Fri Apr 5 04:50:07 2013
@@ -47,6 +47,7 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.spi.state.NodeStateUtils;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
/**
* {@link IndexHook} implementation that is responsible for keeping the
@@ -86,6 +87,8 @@ class Property2IndexHook implements Inde
*/
private String path;
+ private final List<Property2IndexHookUpdate> updates = Lists.newArrayList();
+
/**
* The map of known indexes. Key: the property name. Value: the list of
* indexes (it is possible to have multiple indexes for the same property
@@ -199,9 +202,11 @@ class Property2IndexHook implements Inde
}
}
if (!exists) {
- list.add(new Property2IndexHookUpdate(getPath(), node.child(
- INDEX_DEFINITIONS_NAME).child(indexName), store,
- typeNames));
+ Property2IndexHookUpdate update = new Property2IndexHookUpdate(
+ getPath(), node.child(INDEX_DEFINITIONS_NAME).child(indexName),
+ store, typeNames);
+ list.add(update);
+ updates.add(update);
}
}
}
@@ -236,6 +241,9 @@ class Property2IndexHook implements Inde
@Override
public void leave(NodeState before, NodeState after)
throws CommitFailedException {
+ for (Property2IndexHookUpdate update : updates) {
+ update.checkUniqueKeys();
+ }
}
@Override
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexHookUpdate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexHookUpdate.java?rev=1464823&r1=1464822&r2=1464823&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexHookUpdate.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexHookUpdate.java Fri Apr 5 04:50:07 2013
@@ -19,7 +19,9 @@ package org.apache.jackrabbit.oak.plugin
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_PROPERTY_NAME;
import static org.apache.jackrabbit.oak.plugins.index.p2.Property2Index.encode;
+import java.util.Collections;
import java.util.List;
+import java.util.Set;
import javax.jcr.PropertyType;
@@ -29,9 +31,11 @@ import org.apache.jackrabbit.oak.api.Typ
import org.apache.jackrabbit.oak.plugins.index.p2.strategy.IndexStoreStrategy;
import org.apache.jackrabbit.oak.spi.query.PropertyValues;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
/**
* Takes care of applying the updates to the index content.
@@ -64,6 +68,8 @@ class Property2IndexHookUpdate {
private final boolean unique;
+ private final Set<String> modifiedKeys = Sets.newHashSet();
+
public Property2IndexHookUpdate(String path, NodeBuilder node,
IndexStoreStrategy store, List<String> nodeTypeNames) {
this.path = path;
@@ -98,7 +104,8 @@ class Property2IndexHookUpdate {
return;
}
for (String key : encode(PropertyValues.create(value))) {
- store.insert(index, key, unique, ImmutableSet.of(trimm(path)));
+ store.insert(index, key, ImmutableSet.of(trimm(path)));
+ modifiedKeys.add(key);
}
}
@@ -118,6 +125,19 @@ class Property2IndexHookUpdate {
}
for (String key : encode(PropertyValues.create(value))) {
store.remove(index, key, ImmutableSet.of(trimm(path)));
+ modifiedKeys.add(key);
+ }
+ }
+
+ public void checkUniqueKeys() throws CommitFailedException {
+ if (unique && !modifiedKeys.isEmpty()) {
+ NodeState state = index.getNodeState();
+ for (String key : modifiedKeys) {
+ if (store.count(state, Collections.singletonList(key), 2) > 1) {
+ throw new CommitFailedException(
+ "Uniqueness constraint violated for key " + key);
+ }
+ }
}
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java?rev=1464823&r1=1464822&r2=1464823&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java Fri Apr 5 04:50:07 2013
@@ -134,16 +134,9 @@ public class ContentMirrorStoreStrategy
}
@Override
- public void insert(NodeBuilder index, String key, boolean unique,
- Iterable<String> values) throws CommitFailedException {
+ public void insert(NodeBuilder index, String key, Iterable<String> values)
+ throws CommitFailedException {
NodeBuilder child = index.child(key);
- if (unique
- && (child.getProperty("match") != null || child
- .getChildNodeCount() > 0)) {
- throw new CommitFailedException(
- "Uniqueness constraint violated for key " + key);
- }
-
for (String add : values) {
NodeBuilder indexEntry = child;
for (String segment : PathUtils.elements(add)) {
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/IndexStoreStrategy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/IndexStoreStrategy.java?rev=1464823&r1=1464822&r2=1464823&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/IndexStoreStrategy.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/IndexStoreStrategy.java Fri Apr 5 04:50:07 2013
@@ -45,13 +45,11 @@ public interface IndexStoreStrategy {
*
* @param index the index node
* @param key the index key
- * @param unique if the index is defined as unique
- * <b>Note:</b> If the uniqueness constraint is broken, the method will throw a <code>CommitFailedException</code>
* @param values the values to be added to the given key
* @throws CommitFailedException
*/
- void insert(NodeBuilder index, String key, boolean unique,
- Iterable<String> values) throws CommitFailedException;
+ void insert(NodeBuilder index, String key, Iterable<String> values)
+ throws CommitFailedException;
/**
* Search for a given set of values.
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategyTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategyTest.java?rev=1464823&r1=1464822&r2=1464823&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategyTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategyTest.java Fri Apr 5 04:50:07 2013
@@ -18,6 +18,8 @@ package org.apache.jackrabbit.oak.plugin
import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
+import java.util.Collections;
+
import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
@@ -50,7 +52,7 @@ public class ContentMirrorStoreStrategyT
NodeState root = EMPTY_NODE;
NodeBuilder index = root.builder();
- store.insert(index, "key", false,
+ store.insert(index, "key",
Sets.newHashSet("/", "a/b/c", "a/b/d", "b", "d/e", "d/e/f"));
checkPath(index, "key", "", true);
checkPath(index, "key", "a/b/c", true);
@@ -77,7 +79,7 @@ public class ContentMirrorStoreStrategyT
checkPath(index, "key", "a/b/c", true);
// reinsert root and remove everything else
- store.insert(index, "key", false, Sets.newHashSet("/"));
+ store.insert(index, "key", Sets.newHashSet("/"));
store.remove(index, "key", Sets.newHashSet("d/e/f", "b", "a/b/c"));
// remove the root key when the index is empty
@@ -113,17 +115,15 @@ public class ContentMirrorStoreStrategyT
}
@Test
- public void testUnique() {
+ public void testUnique() throws CommitFailedException {
IndexStoreStrategy store = new ContentMirrorStoreStrategy();
NodeState root = EMPTY_NODE;
NodeBuilder index = root.builder();
- try {
- store.insert(index, "key", true, Sets.newHashSet("a"));
- store.insert(index, "key", true, Sets.newHashSet("a"));
- Assert.fail("ContentMirrorStoreStrategy should guarantee uniqueness on insert");
- } catch (CommitFailedException e) {
- // expected
- }
+ store.insert(index, "key", Sets.newHashSet("a"));
+ store.insert(index, "key", Sets.newHashSet("b"));
+ Assert.assertTrue(
+ "ContentMirrorStoreStrategy should guarantee uniqueness on insert",
+ store.count(index.getNodeState(), Collections.singletonList("key"), 2) > 1);
}
}