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/21 09:17:42 UTC
svn commit: r1680748 - in /jackrabbit/oak/branches/1.0/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/prop...
Author: thomasm
Date: Thu May 21 07:17:42 2015
New Revision: 1680748
URL: http://svn.apache.org/r1680748
Log:
OAK-2663 Unique property index can trigger OOM during upgrade of large repository
Modified:
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java
jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java?rev=1680748&r1=1680747&r2=1680748&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java Thu May 21 07:17:42 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;
@@ -84,6 +85,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;
@@ -258,34 +260,78 @@ class PropertyIndexEditor implements Ind
if (!beforeKeys.isEmpty() || !afterKeys.isEmpty()) {
updateCallback.indexUpdate();
NodeBuilder index = definition.child(INDEX_CONTENT_NODE_NAME);
- getStrategy(keysToCheckForUniqueness != null).update(
- index, getPath(), beforeKeys, afterKeys);
- if (keysToCheckForUniqueness != null) {
- keysToCheckForUniqueness.addAll(afterKeys);
+ String properties = definition.getString(PROPERTY_NAMES);
+ boolean uniqueIndex = keysToCheckForUniqueness != null;
+ if (uniqueIndex) {
+ keysToCheckForUniqueness.addAll(
+ getExistingKeys(afterKeys, index));
}
+ getStrategy(uniqueIndex).update(
+ index, getPath(), beforeKeys, afterKeys);
}
}
if (parent == null) {
// 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(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(indexMeta, singleton(key), 2) > 1) {
+ return key;
+ }
+ }
+ return null;
+ }
private static boolean isTypeProperty(String name) {
return JCR_PRIMARYTYPE.equals(name) || JCR_MIXINTYPES.equals(name);
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java?rev=1680748&r1=1680747&r2=1680748&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java Thu May 21 07:17:42 2015
@@ -468,4 +468,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/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java?rev=1680748&r1=1680747&r2=1680748&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java Thu May 21 07:17:42 2015
@@ -39,7 +39,16 @@ public interface IndexStoreStrategy {
void update(
NodeBuilder index, String path,
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/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java?rev=1680748&r1=1680747&r2=1680748&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java Thu May 21 07:17:42 2015
@@ -89,7 +89,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);
@@ -142,6 +144,11 @@ public class UniqueEntryStoreStrategy im
}
@Override
+ public boolean exists(NodeBuilder index, String key) {
+ return index.hasChildNode(key);
+ }
+
+ @Override
public long count(NodeState indexMeta, Set<String> values, int max) {
NodeState index = indexMeta.getChildNode(INDEX_CONTENT_NODE_NAME);
long count = 0;
Modified: jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java?rev=1680748&r1=1680747&r2=1680748&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java Thu May 21 07:17:42 2015
@@ -368,7 +368,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