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 th...@apache.org on 2015/05/20 15:21:02 UTC
svn commit: r1680559 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/index/property/
main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/
test/java/org/apache/jackrabbit/oak/plugins/index/property/
Author: thomasm
Date: Wed May 20 13:21:02 2015
New Revision: 1680559
URL: http://svn.apache.org/r1680559
Log:
OAK-2663 Unique property index can trigger OOM during upgrade of large repository
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java?rev=1680559&r1=1680558&r2=1680559&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java Wed May 20 13:21:02 2015
@@ -29,6 +29,7 @@ import static org.apache.jackrabbit.oak.
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.PROPERTY_NAMES;
import static org.apache.jackrabbit.oak.plugins.index.property.PropertyIndex.encode;
+import java.util.Collections;
import java.util.Set;
import javax.annotation.Nonnull;
@@ -87,6 +88,7 @@ class PropertyIndexEditor implements Ind
private final Predicate<NodeState> typePredicate;
/**
+ * This field is only set for unique indexes. Otherwise it is null.
* Keys to check for uniqueness, or {@code null} for no uniqueness checks.
*/
private final Set<String> keysToCheckForUniqueness;
@@ -264,11 +266,13 @@ class PropertyIndexEditor implements Ind
updateCallback.indexUpdate();
NodeBuilder index = definition.child(INDEX_CONTENT_NODE_NAME);
String properties = definition.getString(PROPERTY_NAMES);
- getStrategy(keysToCheckForUniqueness != null).update(
- index, getPath(), properties, definition, beforeKeys, afterKeys);
- if (keysToCheckForUniqueness != null) {
- keysToCheckForUniqueness.addAll(afterKeys);
+ boolean uniqueIndex = keysToCheckForUniqueness != null;
+ if (uniqueIndex) {
+ keysToCheckForUniqueness.addAll(
+ getExistingKeys(afterKeys, index));
}
+ getStrategy(uniqueIndex).update(
+ index, getPath(), properties, definition, beforeKeys, afterKeys);
}
}
@@ -276,21 +280,63 @@ class PropertyIndexEditor implements Ind
// make sure that the index node exist, even with no content
definition.child(INDEX_CONTENT_NODE_NAME);
+ boolean uniqueIndex = keysToCheckForUniqueness != null;
// check uniqueness constraints when leaving the root
- if (keysToCheckForUniqueness != null
- && !keysToCheckForUniqueness.isEmpty()) {
+ if (uniqueIndex &&
+ !keysToCheckForUniqueness.isEmpty()) {
NodeState indexMeta = definition.getNodeState();
- IndexStoreStrategy s = getStrategy(true);
- for (String key : keysToCheckForUniqueness) {
- if (s.count(root, indexMeta, singleton(key), 2) > 1) {
- String msg = String.format("Uniqueness constraint violated at path [%s] for one of the " +
- "property in %s having value %s", getPath(), propertyNames, key);
- throw new CommitFailedException(
- CONSTRAINT, 30, msg);
- }
+ String failed = getFirstDuplicate(
+ keysToCheckForUniqueness, indexMeta);
+ if (failed != null) {
+ String msg = String.format(
+ "Uniqueness constraint violated at path [%s] for one of the "
+ + "property in %s having value %s",
+ getPath(), propertyNames, failed);
+ throw new CommitFailedException(CONSTRAINT, 30, msg);
+ }
+ }
+ }
+ }
+
+ /**
+ * From a set of keys, get those that already exist in the index.
+ *
+ * @param keys the keys
+ * @param index the index
+ * @return the set of keys that already exist in this unique index
+ */
+ private Set<String> getExistingKeys(Set<String> keys, NodeBuilder index) {
+ Set<String> existing = null;
+ IndexStoreStrategy s = getStrategy(true);
+ for (String key : keys) {
+ if (s.exists(index, key)) {
+ if (existing == null) {
+ existing = newHashSet();
}
+ existing.add(key);
+ }
+ }
+ if (existing == null) {
+ existing = Collections.emptySet();
+ }
+ return existing;
+ }
+
+ /**
+ * From a set of keys, get the first that has multiple entries, if any.
+ *
+ * @param keys the keys
+ * @param indexMeta the index configuration
+ * @return the first duplicate, or null if none was found
+ */
+ private String getFirstDuplicate(Set<String> keys, NodeState indexMeta) {
+ IndexStoreStrategy s = getStrategy(true);
+ for (String key : keys) {
+ if (s.count(root, indexMeta, singleton(key), 2) > 1) {
+ return key;
}
}
+ return null;
}
private static boolean isTypeProperty(String name) {
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java?rev=1680559&r1=1680558&r2=1680559&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java Wed May 20 13:21:02 2015
@@ -576,4 +576,14 @@ public class ContentMirrorStoreStrategy
}
}
}
+
+ @Override
+ public boolean exists(NodeBuilder index, String key) {
+ // This is currently not implemented, because there is no test case for it,
+ // and because there is currently no need for this method with this class.
+ // We would need to traverse the tree and search for an entry "match".
+ // See also OAK-2663 for a potential (but untested) implementation.
+ throw new UnsupportedOperationException();
+ }
+
}
\ No newline at end of file
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java?rev=1680559&r1=1680558&r2=1680559&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java Wed May 20 13:21:02 2015
@@ -42,7 +42,16 @@ public interface IndexStoreStrategy {
NodeBuilder index, String path,
String indexName, NodeBuilder indexMeta,
Set<String> beforeKeys, Set<String> afterKeys);
-
+
+ /**
+ * Check whether an entry for the given key exists.
+ *
+ * @param index the index
+ * @param key the key
+ * @return true if at least one entry exists
+ */
+ boolean exists(NodeBuilder index, String key);
+
/**
* Search for a given set of values.
*
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java?rev=1680559&r1=1680558&r2=1680559&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java Wed May 20 13:21:02 2015
@@ -96,7 +96,9 @@ public class UniqueEntryStoreStrategy im
PropertyState s = k.getProperty("entry");
for (int i = 0; i < s.count(); i++) {
String r = s.getValue(Type.STRING, i);
- list.add(r);
+ if (!list.contains(r)) {
+ list.add(r);
+ }
}
}
PropertyState s2 = MultiStringPropertyState.stringProperty("entry", list);
@@ -149,6 +151,11 @@ public class UniqueEntryStoreStrategy im
}
@Override
+ public boolean exists(NodeBuilder index, String key) {
+ return index.hasChildNode(key);
+ }
+
+ @Override
public long count(NodeState root, NodeState indexMeta, Set<String> values, int max) {
NodeState index = indexMeta.getChildNode(INDEX_CONTENT_NODE_NAME);
long count = 0;
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java?rev=1680559&r1=1680558&r2=1680559&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java Wed May 20 13:21:02 2015
@@ -459,7 +459,33 @@ public class PropertyIndexTest {
Type.STRINGS);
NodeState after = builder.getNodeState();
- HOOK.processCommit(before, after, CommitInfo.EMPTY); // should throw
+ // should throw
+ HOOK.processCommit(before, after, CommitInfo.EMPTY);
+ }
+
+ @Test
+ public void testUpdateUnique() throws Exception {
+ NodeState root = EMPTY_NODE;
+
+ NodeBuilder builder = root.builder();
+ createIndexDefinition(
+ builder.child(INDEX_DEFINITIONS_NAME),
+ "fooIndex", true, true, ImmutableSet.of("foo"), null);
+ NodeState before = builder.getNodeState();
+ builder.child("a").setProperty("foo", "abc");
+ NodeState after = builder.getNodeState();
+ NodeState done = HOOK.processCommit(before, after, CommitInfo.EMPTY);
+
+ // remove, and then re-add the same node
+ builder = done.builder();
+ builder.child("a").setProperty("foo", "abc");
+ after = builder.getNodeState();
+
+ // apply the changes to the state before adding the entries
+ done = HOOK.processCommit(before, after, CommitInfo.EMPTY);
+
+ // re-apply the changes
+ done = HOOK.processCommit(done, after, CommitInfo.EMPTY);
}
@Test