diff --git a/oak-benchmarks/run_concurrent_addmembers.sh b/oak-benchmarks/run_concurrent_addmembers.sh index d462f45604..e2dce1d008 100755 --- a/oak-benchmarks/run_concurrent_addmembers.sh +++ b/oak-benchmarks/run_concurrent_addmembers.sh @@ -17,13 +17,13 @@ # TITLE=AddMembersTest_besteffort BENCH="AddMembersTest" -BATCH_SIZE="10 50 100 500" +BATCH_SIZE="1 10" # 10 50 100 500 IMPORT_BEHAVIOR="besteffort" # ignore abort" -MEMBERS_CNT="1 10 100 500 1000 5000 10000" -RUNTIME=5 +MEMBERS_CNT="50 100 500 1000" # 1 10 100 500 1000 5000 10000 +RUNTIME=60 FIXS="Oak-Segment-Tar" -THREADS="1,10,20,50" #"1,2,4,8,10,15,20,50" -PROFILE=true +THREADS="1" #"1,2,4,8,10,15,20,50" +PROFILE=false LOG=$TITLE"_$(date +'%Y%m%d_%H%M%S').csv" echo "Benchmarks: $BENCH" > $LOG @@ -45,11 +45,17 @@ for bm in $BENCH for noMembers in $MEMBERS_CNT do echo "Executing benchmarks with $noMembers members with batchsize $batchsize on $importBehavior" | tee -a $LOG - echo "-----------------------------------------------------------" | tee -a $LOG + echo "-----------------------------------------------------------" | tee -a $LOG rm -rf target/Jackrabbit-* target/Oak-Tar-* - cmd="java -Xmx2048m -Dprofile=$PROFILE -Druntime=$RUNTIME -Dwarmup=1 -jar target/oak-benchmarks-*-SNAPSHOT.jar benchmark --batchSize $batchsize --importBehavior $IMPORT_BEHAVIOR --numberOfUsers $noMembers --csvFile $LOG --concurrency $THREADS --report false $bm $FIXS" + cmd="java -Xmx2048m -Druntime=$RUNTIME -jar target/oak-benchmarks-*-SNAPSHOT.jar benchmark --batchSize $batchsize --importBehavior $IMPORT_BEHAVIOR --numberOfUsers $noMembers --csvFile $LOG --concurrency $THREADS --report false $bm $FIXS" echo $cmd $cmd + + rm -rf target/Jackrabbit-* target/Oak-Tar-* + cmd2="java -Xmx2048m -DGroupImpl.useNew=true -Druntime=$RUNTIME -jar target/oak-benchmarks-*-SNAPSHOT.jar benchmark --batchSize $batchsize --importBehavior $IMPORT_BEHAVIOR --numberOfUsers $noMembers --csvFile $LOG --concurrency $THREADS --report false $bm $FIXS" + echo $cmd2 + $cmd2 + done done done diff --git a/oak-benchmarks/run_concurrent_addmembers_unique.sh b/oak-benchmarks/run_concurrent_addmembers_unique.sh new file mode 100755 index 0000000000..10a50e32dd --- /dev/null +++ b/oak-benchmarks/run_concurrent_addmembers_unique.sh @@ -0,0 +1,64 @@ +#!/bin/sh +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +TITLE=AddUniqueMembersTest_besteffort +BENCH="AddUniqueMembersTest" +BATCH_SIZE="1 10" # 10 50 100 500 +IMPORT_BEHAVIOR="besteffort" # ignore abort" +MEMBERS_CNT="50 100 500 1000" # 1 10 100 500 1000 5000 10000 +RUNTIME=60 +FIXS="Oak-Segment-Tar" +THREADS="1" #"1,2,4,8,10,15,20,50" +PROFILE=false + +LOG=$TITLE"_$(date +'%Y%m%d_%H%M%S').csv" +echo "Benchmarks: $BENCH" > $LOG +echo "Fixture: $FIXS" >> $LOG +echo "Runtime: $RUNTIME" >> $LOG +echo "Concurrency: $THREADS" >> $LOG +echo "Profiling: $PROFILE" >> $LOG + +echo "Batch Size: $BATCH_SIZE" >> $LOG +echo "Import Behavior: $IMPORT_BEHAVIORS" >> $LOG +echo "Number of Members: $MEMBERS_CNT" >> $LOG + +echo "--------------------------------------" >> $LOG + +for bm in $BENCH + do + for batchsize in $BATCH_SIZE + do + for noMembers in $MEMBERS_CNT + do + echo "Executing benchmarks with $noMembers members with batchsize $batchsize on $importBehavior" | tee -a $LOG + echo "-----------------------------------------------------------" | tee -a $LOG + rm -rf target/Jackrabbit-* target/Oak-Tar-* + cmd="java -Xmx2048m -Druntime=$RUNTIME -jar target/oak-benchmarks-*-SNAPSHOT.jar benchmark --batchSize $batchsize --importBehavior $IMPORT_BEHAVIOR --numberOfUsers $noMembers --csvFile $LOG --concurrency $THREADS --report false $bm $FIXS" + echo $cmd + $cmd + + rm -rf target/Jackrabbit-* target/Oak-Tar-* + cmd2="java -Xmx2048m -DGroupImpl.useNew=true -Druntime=$RUNTIME -jar target/oak-benchmarks-*-SNAPSHOT.jar benchmark --batchSize $batchsize --importBehavior $IMPORT_BEHAVIOR --numberOfUsers $noMembers --csvFile $LOG --concurrency $THREADS --report false $bm $FIXS" + echo $cmd2 + $cmd2 + + done + done +done +echo "-----------------------------------------" +echo "Benchmark completed. see $LOG for details:" +cat $LOG diff --git a/oak-benchmarks/run_concurrent_rmmembers.sh b/oak-benchmarks/run_concurrent_rmmembers.sh new file mode 100755 index 0000000000..9789378852 --- /dev/null +++ b/oak-benchmarks/run_concurrent_rmmembers.sh @@ -0,0 +1,63 @@ +#!/bin/sh +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +TITLE=RemoveMembersTest +BENCH="RemoveMembersTest" +BATCH_SIZE="1 10" # 10 50 100 500 +MEMBERS_CNT="50 100" # 1 10 100 500 1000 5000 10000 +RUNTIME=60 +FIXS="Oak-Segment-Tar" +THREADS="1" #"1,2,4,8,10,15,20,50" +PROFILE=false + +LOG=$TITLE"_$(date +'%Y%m%d_%H%M%S').csv" +echo "Benchmarks: $BENCH" > $LOG +echo "Fixture: $FIXS" >> $LOG +echo "Runtime: $RUNTIME" >> $LOG +echo "Concurrency: $THREADS" >> $LOG +echo "Profiling: $PROFILE" >> $LOG + +echo "Batch Size: $BATCH_SIZE" >> $LOG +echo "Import Behavior: $IMPORT_BEHAVIORS" >> $LOG +echo "Number of Members: $MEMBERS_CNT" >> $LOG + +echo "--------------------------------------" >> $LOG + +for bm in $BENCH + do + for batchsize in $BATCH_SIZE + do + for noMembers in $MEMBERS_CNT + do + echo "Executing benchmarks with $noMembers members with batchsize $batchsize on $importBehavior" | tee -a $LOG + echo "-----------------------------------------------------------" | tee -a $LOG + rm -rf target/Jackrabbit-* target/Oak-Tar-* + cmd="java -Xmx2048m -Druntime=$RUNTIME -jar target/oak-benchmarks-*-SNAPSHOT.jar benchmark --batchSize $batchsize --numberOfUsers $noMembers --csvFile $LOG --concurrency $THREADS --report false $bm $FIXS" + echo $cmd + $cmd + + rm -rf target/Jackrabbit-* target/Oak-Tar-* + cmd2="java -Xmx2048m -DGroupImpl.useNew=true -Druntime=$RUNTIME -jar target/oak-benchmarks-*-SNAPSHOT.jar benchmark --batchSize $batchsize --numberOfUsers $noMembers --csvFile $LOG --concurrency $THREADS --report false $bm $FIXS" + echo $cmd2 + $cmd2 + + done + done +done +echo "-----------------------------------------" +echo "Benchmark completed. see $LOG for details:" +cat $LOG diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java index d61cfe124e..6e5a46373c 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java @@ -48,6 +48,14 @@ class GroupImpl extends AuthorizableImpl implements Group { private static final Logger log = LoggerFactory.getLogger(GroupImpl.class); + static boolean USE_NEW = Boolean.getBoolean("GroupImpl.useNew"); + static { + if (USE_NEW) { + log.info("'GroupImpl.useNew' flag enabled."); + System.err.println("'GroupImpl.useNew' flag enabled."); + } + } + GroupImpl(String id, Tree tree, UserManagerImpl userManager) throws RepositoryException { super(id, tree, userManager); } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipProvider.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipProvider.java index f2ab9292cd..954ec82338 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipProvider.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipProvider.java @@ -16,7 +16,9 @@ */ package org.apache.jackrabbit.oak.security.user; +import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Deque; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -107,7 +109,7 @@ class MembershipProvider extends AuthorizableBaseProvider { private static final Logger log = LoggerFactory.getLogger(MembershipProvider.class); - private final MembershipWriter writer = new MembershipWriter(); + private final MembershipWriter writer = new MembershipWriter(GroupImpl.USE_NEW); /** * Creates a new membership provider @@ -387,14 +389,25 @@ class MembershipProvider extends AuthorizableBaseProvider { */ private abstract class MemberReferenceIterator extends AbstractLazyIterator { - private final Iterator trees; + private final Deque trees; + private Tree tree; private Iterator propertyValues; private MemberReferenceIterator(@Nonnull Tree groupTree) { - this.trees = Iterators.concat( - Iterators.singletonIterator(groupTree), - groupTree.getChild(REP_MEMBERS_LIST).getChildren().iterator() - ); + this.trees = new ArrayDeque<>(); + trees.push(groupTree); + } + + private Tree getTree() { + if (tree == null) { + tree = trees.poll(); + if (tree != null) { + for (Tree c : tree.getChildren()) { + trees.push(c); + } + } + } + return tree; } @Override @@ -403,17 +416,21 @@ class MembershipProvider extends AuthorizableBaseProvider { while (next == null) { if (propertyValues == null) { // check if there are more trees that can provide a rep:members property - if (!trees.hasNext()) { + Tree t = getTree(); + if (t == null) { // if not, we're done break; } - PropertyState property = trees.next().getProperty(REP_MEMBERS); + PropertyState property = t.getProperty(REP_MEMBERS); if (property != null) { propertyValues = property.getValue(Type.STRINGS).iterator(); + } else { + tree = null; } } else if (!propertyValues.hasNext()) { // if there are no more values left, reset the iterator propertyValues = null; + tree = null; } else { String value = propertyValues.next(); if (hasProcessedReference(value)) { diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipWriter.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipWriter.java index 05a2888bd3..791685b287 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipWriter.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipWriter.java @@ -16,26 +16,34 @@ */ package org.apache.jackrabbit.oak.security.user; +import static org.apache.jackrabbit.oak.api.Type.NAME; + +import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; +import java.util.function.Function; import javax.annotation.Nonnull; import javax.jcr.RepositoryException; -import com.google.common.collect.Maps; -import com.google.common.collect.Sets; import org.apache.jackrabbit.JcrConstants; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.api.Type; -import org.apache.jackrabbit.oak.spi.security.user.UserConstants; import org.apache.jackrabbit.oak.plugins.memory.PropertyBuilder; +import org.apache.jackrabbit.oak.plugins.tree.TreeUtil; +import org.apache.jackrabbit.oak.spi.security.user.UserConstants; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterators; - -import static org.apache.jackrabbit.oak.api.Type.NAME; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; /** * @see MembershipProvider to more details. @@ -44,13 +52,21 @@ public class MembershipWriter { public static final int DEFAULT_MEMBERSHIP_THRESHOLD = 100; + private final WriterStrategy writer; + + public MembershipWriter(boolean useTreeWriter) { + if (useTreeWriter) { + writer = new TreeWriter(); + } else { + writer = new ListWriter(); + } + } + /** - * size of the membership threshold after which a new overflow node is created. + * Sets the size of the membership threshold after which a new overflow node is created. */ - private int membershipSizeThreshold = DEFAULT_MEMBERSHIP_THRESHOLD; - public void setMembershipSizeThreshold(int membershipSizeThreshold) { - this.membershipSizeThreshold = membershipSizeThreshold; + writer.setMembershipSizeThreshold(membershipSizeThreshold); } /** @@ -76,162 +92,479 @@ public class MembershipWriter { * @throws RepositoryException if an error occurs */ Set addMembers(@Nonnull Tree groupTree, @Nonnull Map memberIds) throws RepositoryException { - // check all possible rep:members properties for the new member and also find the one with the least values - Tree membersList = groupTree.getChild(UserConstants.REP_MEMBERS_LIST); - Iterator trees = Iterators.concat( - Iterators.singletonIterator(groupTree), - membersList.getChildren().iterator() - ); - - Set failed = new HashSet(memberIds.size()); - int bestCount = membershipSizeThreshold; - PropertyState bestProperty = null; - Tree bestTree = null; - - // remove existing memberIds from the map and find best-matching tree - // for the insertion of the new members. - while (trees.hasNext() && !memberIds.isEmpty()) { - Tree t = trees.next(); - PropertyState refs = t.getProperty(UserConstants.REP_MEMBERS); - if (refs != null) { - int numRefs = 0; - for (String ref : refs.getValue(Type.WEAKREFERENCES)) { - String id = memberIds.remove(ref); - if (id != null) { - failed.add(id); - if (memberIds.isEmpty()) { - break; + return writer.addMembers(groupTree, memberIds); + } + + /** + * Removes the member from the given group. + * + * @param groupTree group to remove the member from + * @param memberContentId member to remove + * @return {@code true} if the member was removed. + */ + boolean removeMember(@Nonnull Tree groupTree, @Nonnull String memberContentId) { + Map m = Maps.newHashMapWithExpectedSize(1); + m.put(memberContentId, "-"); + return removeMembers(groupTree, m).isEmpty(); + } + + /** + * Removes the members from the given group. + * + * @param groupTree group to remove the member from + * @param memberIds Map of 'contentId':'memberId' of all members that need to be removed. + * @return the set of member IDs that was not successfully processed. + */ + Set removeMembers(@Nonnull Tree groupTree, @Nonnull Map memberIds) { + return writer.removeMembers(groupTree, memberIds); + } + + private static interface WriterStrategy { + + void setMembershipSizeThreshold(int membershipSizeThreshold); + + Set addMembers(@Nonnull Tree groupTree, @Nonnull Map memberIds) + throws RepositoryException; + + Set removeMembers(@Nonnull Tree groupTree, @Nonnull Map memberIds); + + } + + private static class TreeWriterLeaf { + + Tree t; + int level; + boolean load; + + TreeWriterLeaf(Tree t, boolean load, int level) { + this.t = t; + this.level = level; + this.load = load; + } + + @Override + public String toString() { + return "TreeWriterLeaf [t=" + t + ", level=" + level + "]"; + } + + } + + static class TreeWriter implements WriterStrategy { + + private int membershipSizeThreshold = DEFAULT_MEMBERSHIP_THRESHOLD; + + // a132cbbd-6a2c-3981-965a-239e22fba6c7 + private static int MAX_LEVEL = 35; + + public void setMembershipSizeThreshold(int membershipSizeThreshold) { + this.membershipSizeThreshold = membershipSizeThreshold; + } + + @Override + public Set addMembers(@Nonnull Tree groupTree, final @Nonnull Map memberIds) + throws RepositoryException { + Set failed = new HashSet(); + + List keys = Lists.newArrayList(memberIds.keySet()); + if (keys.size() > 1) { + Collections.sort(keys); + } + + TreeWriterLeaf location = new TreeWriterLeaf(groupTree, keys.size() > 1, -1); + for (String key : keys) { + String mid = memberIds.remove(key); + if (mid == null) { + continue; + } + if (location.level >= 0) { + location = walkUp(location, key); + } + + Function onProperty = new Function() { + + @Override + public Boolean apply(String k) { + String v = memberIds.remove(k); + if (v != null) { + failed.add(v); } + return !memberIds.isEmpty(); } - numRefs++; + }; + + if (!addMember(key, location, membershipSizeThreshold, MAX_LEVEL, onProperty)) { + failed.add(mid); } - if (numRefs < bestCount) { - bestCount = numRefs; - bestProperty = refs; - bestTree = t; + if (memberIds.isEmpty()) { + break; } } + return failed; } - // update member content structure by starting inserting new member IDs - // with the best-matching property and create new member-ref-nodes as needed. - if (!memberIds.isEmpty()) { - PropertyBuilder propertyBuilder; - int propCnt; - if (bestProperty == null) { - // we don't have a good candidate to store the new members. - // so there are no members at all or all are full - if (!groupTree.hasProperty(UserConstants.REP_MEMBERS)) { - bestTree = groupTree; + static TreeWriterLeaf walkUp(TreeWriterLeaf location, String key) { + Tree t = location.t; + int level = location.level; + + if (level >= 0 && !idToKey(key, level).equals(t.getName())) { + if (level == 0) { + t = t.getParent(); + } + TreeWriterLeaf nl = new TreeWriterLeaf(t.getParent(), true, level - 1); + return walkUp(nl, key); + } + return location; + } + + static boolean addMember(String uuid, TreeWriterLeaf location, int threshold, int maxLevel, + Function onProperty) { + Tree t = location.t; + int level = location.level; + + // if property exists, we're sure this is a leaf + PropertyState refs = t.getProperty(UserConstants.REP_MEMBERS); + if (refs != null) { + + Set newVals = getValues(refs.getValue(Type.WEAKREFERENCES), uuid, onProperty, location); + if (newVals == null) { + return false; + } + newVals.add(uuid); + + if (newVals.size() <= threshold || level >= maxLevel) { + setRepMembers(t, newVals); + return true; + } else { - bestTree = createMemberRefTree(groupTree, membersList); + // merge & split + t.removeProperty(UserConstants.REP_MEMBERS); + if (level == -1) { + t = t.addChild(UserConstants.REP_MEMBERS_LIST); + level++; + } + // change node type from 'refs' to 'ref list' + t.setProperty(JcrConstants.JCR_PRIMARYTYPE, UserConstants.NT_REP_MEMBER_REFERENCES_LIST, NAME); + + Map> groupped = groupByKey(newVals, level); + for (Entry> e : groupped.entrySet()) { + Tree c = t.addChild(e.getKey()); + c.setProperty(JcrConstants.JCR_PRIMARYTYPE, UserConstants.NT_REP_MEMBER_REFERENCES, NAME); + // this can overflow the threshold + setRepMembers(c, e.getValue()); + } + location.t = t.getChild(idToKey(uuid, level)); + location.level = level; + return true; } - propertyBuilder = PropertyBuilder.array(Type.WEAKREFERENCE, UserConstants.REP_MEMBERS); - propCnt = 0; + + } else if (isLeafType(t, level)) { + setRepMembers(t, ImmutableSet.of(uuid)); + location.load = false; + return true; + } else { - propertyBuilder = PropertyBuilder.copy(Type.WEAKREFERENCE, bestProperty); - propCnt = bestCount; + // continue going down the tree + if (level == -1) { + t = getOrAdd(t, UserConstants.REP_MEMBERS_LIST, UserConstants.NT_REP_MEMBER_REFERENCES_LIST); + } + level++; + String key = idToKey(uuid, level); + Tree c = getOrAdd(t, key, UserConstants.NT_REP_MEMBER_REFERENCES); + + location.t = c; + location.level = level; + location.load = true; + + return addMember(uuid, location, threshold, maxLevel, onProperty); } - // if adding all new members to best-property would exceed the threshold - // the new ids need to be distributed to different member-ref-nodes - // for simplicity this is achieved by introducing new tree(s) - if ((propCnt + memberIds.size()) > membershipSizeThreshold) { - while (!memberIds.isEmpty()) { - Set s = new HashSet(); - Iterator it = memberIds.keySet().iterator(); - while (propCnt < membershipSizeThreshold && it.hasNext()) { - s.add(it.next()); - it.remove(); - propCnt++; + } + + private static Set getValues(Iterable refs, String uuid, Function onProperty, + TreeWriterLeaf location) { + if (location.load) { + // eager load, process callback and lookup separately + Set vals = Sets.newHashSet(refs); + location.load = false; + + for (String v : vals) { + if (!onProperty.apply(v)) { + break; } - propertyBuilder.addValues(s); - bestTree.setProperty(propertyBuilder.getPropertyState()); + } - if (it.hasNext()) { - // continue filling the next (new) node + propertyBuilder pair - propCnt = 0; - bestTree = createMemberRefTree(groupTree, membersList); - propertyBuilder = PropertyBuilder.array(Type.WEAKREFERENCE, UserConstants.REP_MEMBERS); + if (vals.contains(uuid)) { + return null; + } else { + return vals; + } + + } else { + // lazy load, process callback and lookup online + Set vals = new HashSet<>(); + boolean callback = true; + for (String v : refs) { + if (callback) { + callback = onProperty.apply(v); } + if (v.equals(uuid)) { + return null; + } + vals.add(v); } + return vals; + } + } + + private static Tree getOrAdd(Tree t, String name, String type) { + Tree c; + if (t.hasChild(name)) { + c = t.getChild(name); } else { - propertyBuilder.addValues(memberIds.keySet()); - bestTree.setProperty(propertyBuilder.getPropertyState()); + c = t.addChild(name); + c.setProperty(JcrConstants.JCR_PRIMARYTYPE, type, NAME); } + return c; } - return failed; - } - private static Tree createMemberRefTree(@Nonnull Tree groupTree, @Nonnull Tree membersList) { - if (!membersList.exists()) { - membersList = groupTree.addChild(UserConstants.REP_MEMBERS_LIST); - membersList.setProperty(JcrConstants.JCR_PRIMARYTYPE, UserConstants.NT_REP_MEMBER_REFERENCES_LIST, NAME); + private static void setRepMembers(Tree t, Iterable ids) { + PropertyBuilder propertyBuilder; + propertyBuilder = PropertyBuilder.array(Type.WEAKREFERENCE, UserConstants.REP_MEMBERS); + propertyBuilder.addValues(ids); + t.setProperty(propertyBuilder.getPropertyState()); } - Tree refTree = membersList.addChild(nextRefNodeName(membersList)); - refTree.setProperty(JcrConstants.JCR_PRIMARYTYPE, UserConstants.NT_REP_MEMBER_REFERENCES, NAME); - return refTree; - } - private static String nextRefNodeName(@Nonnull Tree membersList) { - // keep node names linear - int i = 0; - String name = String.valueOf(i); - while (membersList.hasChild(name)) { - name = String.valueOf(++i); + private static String idToKey(String id, int level) { + return id.charAt(level) + ""; } - return name; - } - /** - * Removes the member from the given group. - * - * @param groupTree group to remove the member from - * @param memberContentId member to remove - * @return {@code true} if the member was removed. - */ - boolean removeMember(@Nonnull Tree groupTree, @Nonnull String memberContentId) { - Map m = Maps.newHashMapWithExpectedSize(1); - m.put(memberContentId, "-"); - return removeMembers(groupTree, m).isEmpty(); - } + private static boolean isLeafType(Tree t, int level) { + if (level == -1) { + return !t.hasChild(UserConstants.REP_MEMBERS_LIST); + } else { + String pt = TreeUtil.getName(t, JcrConstants.JCR_PRIMARYTYPE); + return UserConstants.NT_REP_MEMBER_REFERENCES.equals(pt); + } + } - /** - * Removes the members from the given group. - * - * @param groupTree group to remove the member from - * @param memberIds Map of 'contentId':'memberId' of all members that need to be removed. - * @return the set of member IDs that was not successfully processed. - */ - Set removeMembers(@Nonnull Tree groupTree, @Nonnull Map memberIds) { - Tree membersList = groupTree.getChild(UserConstants.REP_MEMBERS_LIST); - Iterator trees = Iterators.concat( - Iterators.singletonIterator(groupTree), - membersList.getChildren().iterator() - ); - while (trees.hasNext() && !memberIds.isEmpty()) { - Tree t = trees.next(); + private static Map> groupByKey(Set uuids, int level) { + Map> ret = new HashMap<>(); + for (String uuid : uuids) { + String key = idToKey(uuid, level); + Set vals = ret.get(key); + if (vals == null) { + vals = new HashSet<>(); + } + vals.add(uuid); + ret.put(key, vals); + } + return ret; + } + + @Override + public Set removeMembers(@Nonnull Tree groupTree, @Nonnull Map memberIds) { + Set failed = new HashSet(memberIds.size()); + for (Entry e : memberIds.entrySet()) { + if (!removeMember(e.getKey(), groupTree, 0)) { + failed.add(e.getValue()); + } + } + return failed; + } + + private static boolean removeMember(String uuid, Tree t, int level) { + // if property exists, we're sure this is a leaf PropertyState refs = t.getProperty(UserConstants.REP_MEMBERS); if (refs != null) { - PropertyBuilder prop = PropertyBuilder.copy(Type.WEAKREFERENCE, refs); - Iterator> it = memberIds.entrySet().iterator(); - while (it.hasNext() && !prop.isEmpty()) { - String memberContentId = it.next().getKey(); - if (prop.hasValue(memberContentId)) { - prop.removeValue(memberContentId); - it.remove(); + Set vals = Sets.newHashSet(refs.getValue(Type.WEAKREFERENCES)); + if (vals.remove(uuid)) { + if (vals.isEmpty()) { + if (level > 0) { + // TODO more aggressive pruning in case of deletes + t.remove(); + } else { + t.removeProperty(UserConstants.REP_MEMBERS); + } + } else { + PropertyBuilder propertyBuilder = PropertyBuilder.array(Type.WEAKREFERENCE, + UserConstants.REP_MEMBERS); + propertyBuilder.addValues(vals); + t.setProperty(propertyBuilder.getPropertyState()); + } + return true; + } else { + return false; + } + } else if (isLeafType(t, level)) { + return false; + } else { + // continue going down the tree + if (level == 0) { + t = t.getChild(UserConstants.REP_MEMBERS_LIST); + if (!t.exists()) { + return false; + } + } + String key = idToKey(uuid, level); + if (t.hasChild(key)) { + return removeMember(uuid, t.getChild(key), level + 1); + } else { + return false; + } + } + } + } + + private static class ListWriter implements WriterStrategy { + + private int membershipSizeThreshold = DEFAULT_MEMBERSHIP_THRESHOLD; + + public void setMembershipSizeThreshold(int membershipSizeThreshold) { + this.membershipSizeThreshold = membershipSizeThreshold; + } + + @Override + public Set addMembers(@Nonnull Tree groupTree, @Nonnull Map memberIds) + throws RepositoryException { + + // check all possible rep:members properties for the new member and also find the one with the least values + Tree membersList = groupTree.getChild(UserConstants.REP_MEMBERS_LIST); + Iterator trees = Iterators.concat( + Iterators.singletonIterator(groupTree), + membersList.getChildren().iterator() + ); + + Set failed = new HashSet(memberIds.size()); + int bestCount = membershipSizeThreshold; + PropertyState bestProperty = null; + Tree bestTree = null; + + // remove existing memberIds from the map and find best-matching tree + // for the insertion of the new members. + while (trees.hasNext() && !memberIds.isEmpty()) { + Tree t = trees.next(); + PropertyState refs = t.getProperty(UserConstants.REP_MEMBERS); + if (refs != null) { + int numRefs = 0; + for (String ref : refs.getValue(Type.WEAKREFERENCES)) { + String id = memberIds.remove(ref); + if (id != null) { + failed.add(id); + if (memberIds.isEmpty()) { + break; + } + } + numRefs++; + } + if (numRefs < bestCount) { + bestCount = numRefs; + bestProperty = refs; + bestTree = t; } } - if (prop.isEmpty()) { - if (t == groupTree) { - t.removeProperty(UserConstants.REP_MEMBERS); + } + + // update member content structure by starting inserting new member IDs + // with the best-matching property and create new member-ref-nodes as needed. + if (!memberIds.isEmpty()) { + PropertyBuilder propertyBuilder; + int propCnt; + if (bestProperty == null) { + // we don't have a good candidate to store the new members. + // so there are no members at all or all are full + if (!groupTree.hasProperty(UserConstants.REP_MEMBERS)) { + bestTree = groupTree; } else { - t.remove(); + bestTree = createMemberRefTree(groupTree, membersList); } + propertyBuilder = PropertyBuilder.array(Type.WEAKREFERENCE, UserConstants.REP_MEMBERS); + propCnt = 0; } else { - t.setProperty(prop.getPropertyState()); + propertyBuilder = PropertyBuilder.copy(Type.WEAKREFERENCE, bestProperty); + propCnt = bestCount; } + // if adding all new members to best-property would exceed the threshold + // the new ids need to be distributed to different member-ref-nodes + // for simplicity this is achieved by introducing new tree(s) + if ((propCnt + memberIds.size()) > membershipSizeThreshold) { + while (!memberIds.isEmpty()) { + Set s = new HashSet(); + Iterator it = memberIds.keySet().iterator(); + while (propCnt < membershipSizeThreshold && it.hasNext()) { + s.add(it.next()); + it.remove(); + propCnt++; + } + propertyBuilder.addValues(s); + bestTree.setProperty(propertyBuilder.getPropertyState()); + + if (it.hasNext()) { + // continue filling the next (new) node + propertyBuilder pair + propCnt = 0; + bestTree = createMemberRefTree(groupTree, membersList); + propertyBuilder = PropertyBuilder.array(Type.WEAKREFERENCE, UserConstants.REP_MEMBERS); + } + } + } else { + propertyBuilder.addValues(memberIds.keySet()); + bestTree.setProperty(propertyBuilder.getPropertyState()); + } + } + return failed; + } + + private static Tree createMemberRefTree(@Nonnull Tree groupTree, @Nonnull Tree membersList) { + if (!membersList.exists()) { + membersList = groupTree.addChild(UserConstants.REP_MEMBERS_LIST); + membersList.setProperty(JcrConstants.JCR_PRIMARYTYPE, UserConstants.NT_REP_MEMBER_REFERENCES_LIST, NAME); } + Tree refTree = membersList.addChild(nextRefNodeName(membersList)); + refTree.setProperty(JcrConstants.JCR_PRIMARYTYPE, UserConstants.NT_REP_MEMBER_REFERENCES, NAME); + return refTree; } - return Sets.newHashSet(memberIds.values()); + + private static String nextRefNodeName(@Nonnull Tree membersList) { + // keep node names linear + int i = 0; + String name = String.valueOf(i); + while (membersList.hasChild(name)) { + name = String.valueOf(++i); + } + return name; + } + + @Override + public Set removeMembers(@Nonnull Tree groupTree, @Nonnull Map memberIds) { + Tree membersList = groupTree.getChild(UserConstants.REP_MEMBERS_LIST); + Iterator trees = Iterators.concat( + Iterators.singletonIterator(groupTree), + membersList.getChildren().iterator() + ); + while (trees.hasNext() && !memberIds.isEmpty()) { + Tree t = trees.next(); + PropertyState refs = t.getProperty(UserConstants.REP_MEMBERS); + if (refs != null) { + PropertyBuilder prop = PropertyBuilder.copy(Type.WEAKREFERENCE, refs); + Iterator> it = memberIds.entrySet().iterator(); + while (it.hasNext() && !prop.isEmpty()) { + String memberContentId = it.next().getKey(); + if (prop.hasValue(memberContentId)) { + prop.removeValue(memberContentId); + it.remove(); + } + } + if (prop.isEmpty()) { + if (t == groupTree) { + t.removeProperty(UserConstants.REP_MEMBERS); + } else { + t.remove(); + } + } else { + t.setProperty(prop.getPropertyState()); + } + } + } + return Sets.newHashSet(memberIds.values()); + } + } } \ No newline at end of file diff --git a/oak-core/src/main/resources/org/apache/jackrabbit/oak/builtin_nodetypes.cnd b/oak-core/src/main/resources/org/apache/jackrabbit/oak/builtin_nodetypes.cnd index 0214cbc788..6771a502de 100644 --- a/oak-core/src/main/resources/org/apache/jackrabbit/oak/builtin_nodetypes.cnd +++ b/oak-core/src/main/resources/org/apache/jackrabbit/oak/builtin_nodetypes.cnd @@ -781,6 +781,7 @@ */ [rep:MemberReferencesList] + * (rep:MemberReferences) = rep:MemberReferences protected COPY + + * (rep:MemberReferencesList) = rep:MemberReferences protected COPY /* @since oak 1.7.0 */ /** * @since oak 1.4 diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/GroupImplTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/GroupImplTest.java index 0dead27cab..2062984cb3 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/GroupImplTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/GroupImplTest.java @@ -16,11 +16,19 @@ */ package org.apache.jackrabbit.oak.security.user; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + import java.security.Principal; +import java.util.Arrays; import java.util.Iterator; +import java.util.List; +import java.util.Set; import java.util.UUID; -import com.google.common.collect.Iterators; import org.apache.jackrabbit.api.security.user.Authorizable; import org.apache.jackrabbit.api.security.user.Group; import org.apache.jackrabbit.oak.AbstractSecurityTest; @@ -28,10 +36,8 @@ import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; import org.junit.Test; import org.mockito.Mockito; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; +import com.google.common.collect.Iterators; +import com.google.common.collect.Lists; public class GroupImplTest extends AbstractSecurityTest { @@ -121,4 +127,37 @@ public class GroupImplTest extends AbstractSecurityTest { Iterator members = groupPrincipal.getMembers(); assertTrue(Iterators.elementsEqual(group.getMembers(), members)); } + + @Test + public void testMembersSetGetRm() throws Exception { + int size = 150; + + String[] tests = new String[size]; + for (int i = 0; i < size; i++) { + String id = "user" + System.currentTimeMillis() + "-" + i; + tests[i] = id; + assertTrue(uMgr.createGroup(id) != null); + } + Arrays.sort(tests); + + Set res1 = group.addMembers(tests); + assertTrue("unable to add ["+res1.size()+"] " + res1, res1.isEmpty()); + + List out = Lists.newArrayList(group.getMembers()); + assertEquals(size, out.size()); + + String[] got = new String[size]; + int i = 0; + for (Authorizable a : out) { + got[i++] = a.getID(); + } + Arrays.sort(got); + assertArrayEquals(tests, got); + + Set res2 = group.removeMembers(tests); + assertTrue("unable to remove " + res2, res2.isEmpty()); + + List out2 = Lists.newArrayList(group.getMembers()); + assertEquals(0, out2.size()); + } } \ No newline at end of file diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/MembershipWriterTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/MembershipWriterTest.java index bc50ecc64d..cf50b6f78f 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/MembershipWriterTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/MembershipWriterTest.java @@ -56,7 +56,7 @@ public class MembershipWriterTest extends MembershipBaseTest { @Before public void before() throws Exception { super.before(); - writer = new MembershipWriter(); + writer = new MembershipWriter(false); // set the threshold low for testing writer.setMembershipSizeThreshold(SIZE_TH); } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/MembershipWriterTestIT.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/MembershipWriterTestIT.java new file mode 100644 index 0000000000..0325145e25 --- /dev/null +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/MembershipWriterTestIT.java @@ -0,0 +1,142 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.oak.security.user; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; + +import java.util.Map; +import java.util.Random; +import java.util.Set; + +import org.apache.jackrabbit.api.security.user.Group; +import org.apache.jackrabbit.oak.api.Tree; +import org.junit.Test; + +import com.google.common.collect.Maps; + +public class MembershipWriterTestIT extends MembershipBaseTest { + + @Test + public void testBenchAddMembersUnique() throws Exception { + assumeTrue(Boolean.getBoolean("MembershipWriterTestIT.testBenchAddMembersUnique")); + + Group grp = createGroup(); + + // [TREE] + // [ADD] 1000 times x 5 items. duration 35 ms. + // [ADD] 1000 times x 50 items. duration 532 ms. // TODO + // [ADD] 1000 times x 500 items. duration 4982 ms. // TODO + + // [LIST] + // [ADD] 1000 times x 5 items. duration 156 ms. // TODO + // [ADD] 1000 times x 50 items. duration 777 ms. // TODO + // [ADD] 1000 times x 500 items. duration 11557 ms. // TODO + + int times = 1000; + int size = 5; + boolean useTreeWriter = false; + + MembershipWriter writer = new MembershipWriter(useTreeWriter); + Tree t = getTree(grp); + + while (true) { + long total = 0; + for (int c = 0; c < times; c++) { + long start = System.currentTimeMillis(); + + Map idMap = Maps.newHashMap(); + for (int i = 0; i < size; i++) { + String memberId = "user" + System.currentTimeMillis() + "-" + c + "-" + i; + idMap.put(getContentID(memberId), memberId); + } + Set res = writer.addMembers(t, idMap); + + long dur = System.currentTimeMillis() - start; + total += dur; + assertTrue("unable to add " + res, res.isEmpty()); + } + System.err.println("[ADD] " + times + " times x " + size + " items. duration " + total + " ms."); + } + } + + @Test + public void testBenchAddMembersExisting() throws Exception { + assumeTrue(Boolean.getBoolean("MembershipWriterTestIT.testBenchAddMembersExisting")); + + Group grp = createGroup(); + + // [TREE] + // [ADD] 10000 times x 5 samples | 50 items. duration 18ms. (inlined as + // a mvp) + // [ADD] 10000 times x 5 samples | 150 items. duration 40ms. + // [ADD] 10000 times x 50 samples | 150 items. duration 230ms. + // [ADD] 10000 times x 150 samples | 150 items. duration 400ms. + + // [LIST] + // [ADD] 10000 times x 5 samples | 50 items. duration 10 ms. (inlined as + // a mvp) + // [ADD] 10000 times x 5 samples | 150 items. duration 25 ms. + // [ADD] 10000 times x 50 samples | 150 items. duration 70ms. + // [ADD] 10000 times x 150 samples | 150 items. duration 140ms. + + int batch = 5; + int size = 150; + boolean useTreeWriter = true; + int times = 10000; + + MembershipWriter writer = new MembershipWriter(useTreeWriter); + + // Setup adds all members beforehand + Map ids = Maps.newHashMap(); + String[] keys = new String[size]; + for (int i = 0; i < size; i++) { + String memberId = "user" + System.currentTimeMillis() + "-" + i; + String key = getContentID(memberId); + ids.put(key, memberId); + keys[i] = key; + } + assertTrue("unable to setup ", writer.addMembers(getTree(grp), ids).isEmpty()); + root.commit(); + Tree t = getTree(grp); + + while (true) { + long total = 0; + Random r = new Random(); + for (int c = 0; c < times; c++) { + long start = System.currentTimeMillis(); + + Map sample = Maps.newHashMap(); + for (int i = 0; i < batch; i++) { + String key = keys[r.nextInt(size)]; + sample.put(key, "" + i); + } + int samples = sample.size(); + + Set res = writer.addMembers(t, sample); + + long dur = System.currentTimeMillis() - start; + total += dur; + assertTrue("should not add any (" + res.size() + " vs " + samples + ")! ", res.size() == samples); + } + + System.err.println("[AddExisting] " + times + " times x " + batch + " samples | " + size + + " items. duration " + total + " ms."); + } + } + +} \ No newline at end of file