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 tr...@apache.org on 2013/11/05 06:44:53 UTC
svn commit: r1538870 -
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/
Author: tripod
Date: Tue Nov 5 05:44:52 2013
New Revision: 1538870
URL: http://svn.apache.org/r1538870
Log:
OAK-1138 Implement global per principal permission entry cache (wip)
- make EntryIterator even more lazy (avoid seekNext() in next())
- only cache 'everyone's entries globally
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractEntryIterator.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProvider.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractEntryIterator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractEntryIterator.java?rev=1538870&r1=1538869&r2=1538870&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractEntryIterator.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractEntryIterator.java Tue Nov 5 05:44:52 2013
@@ -27,38 +27,39 @@ import com.google.common.collect.Iterato
*/
abstract class AbstractEntryIterator implements Iterator<PermissionEntry> {
- // the ordered permission entries at a given path in the hierarchy
- protected Iterator<PermissionEntry> nextEntries;
-
// the next permission entry
protected PermissionEntry next;
+ // indicate that seekNext was already invoked
+ private boolean sought;
+
@Override
public boolean hasNext() {
- if (next == null && nextEntries == null) {
- // lazy initialization
- nextEntries = Iterators.emptyIterator();
+ if (!sought) {
seekNext();
+ sought = true;
}
return next != null;
}
@Override
public PermissionEntry next() {
+ if (!sought) {
+ seekNext();
+ sought = true;
+ }
if (next == null) {
throw new NoSuchElementException();
}
-
- PermissionEntry pe = next;
- seekNext();
- return pe;
+ sought = false;
+ return next;
}
+
@Override
public void remove() {
throw new UnsupportedOperationException();
}
- @CheckForNull
protected abstract void seekNext();
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java?rev=1538870&r1=1538869&r2=1538870&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java Tue Nov 5 05:44:52 2013
@@ -520,7 +520,11 @@ final class CompiledPermissionImpl imple
private static final class LazyIterator extends AbstractEntryIterator {
- private boolean isUser;
+ private final boolean isUser;
+
+ // the ordered permission entries at a given path in the hierarchy
+ private Iterator<PermissionEntry> nextEntries = Iterators.emptyIterator();
+
private TreePermissionImpl tp;
private LazyIterator(@Nonnull TreePermissionImpl tp, boolean isUser) {
@@ -528,9 +532,9 @@ final class CompiledPermissionImpl imple
this.isUser = isUser;
}
- @CheckForNull
+ @Override
protected void seekNext() {
- for (next = null; next == null; ) {
+ for (next = null; next == null;) {
if (nextEntries.hasNext()) {
next = nextEntries.next();
} else {
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java?rev=1538870&r1=1538869&r2=1538870&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java Tue Nov 5 05:44:52 2013
@@ -16,18 +16,31 @@
*/
package org.apache.jackrabbit.oak.security.authorization.permission;
+import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
+import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+
/**
- * <code>PermissionEntryCache</code> caches the permission entries of principals. The cache is held globally and contains
+ * {@code PermissionEntryCache} caches the permission entries of principals. The cache is held globally and contains
* a version of the principal permission entries of the session that read them last. each session gets a lazy copy of
* the cache and needs to verify if each cached principal permission set still reflects the state that the session sees.
* every newly loaded principal permission set can be pushed down to the base cache if it does not exist there yet, or
* if it's newer.
+ *
+ * Todo:
+ * - currently only the entries of 'everyone' are globally cached. this should be improved to dynamically cache those
+ * principals that are used often
+ * - report cache usage metrics
+ * - limit size of local caches based on ppe sizes. the current implementation loads all ppes. this can get a memory
+ * problem, as well as a performance problem for principals with many entries. principals with many entries must
+ * fallback to the direct store.load() methods when providing the entries. if those principals with many entries
+ * are used often, they might get elected to live in the global cache; memory permitting.
*/
public class PermissionEntryCache {
@@ -66,19 +79,45 @@ public class PermissionEntryCache {
}
}
- // check if base cache has the entries
- PrincipalPermissionEntries baseppe = base.get(principalName);
- if (baseppe == null || ppe.getTimestamp() > baseppe.getTimestamp()) {
- base.put(principalName, ppe);
+ // currently we only cache 'everyones' entries. but the cache should dynamically cache the principals
+ // that are used often.
+ if (EveryonePrincipal.NAME.equals(principalName)) {
+ // check if base cache has the entries
+ PrincipalPermissionEntries baseppe = base.get(principalName);
+ if (baseppe == null || ppe.getTimestamp() > baseppe.getTimestamp()) {
+ base.put(principalName, ppe);
+ }
}
return ppe;
}
+ public void load(PermissionStore store, Map<String, Collection<PermissionEntry>> pathEntryMap, String principalName) {
+ // todo: conditionally load entries if too many
+ PrincipalPermissionEntries ppe = getEntries(store, principalName);
+ for (Map.Entry<String, Collection<PermissionEntry>> e: ppe.getEntries().entrySet()) {
+ Collection<PermissionEntry> pathEntries = pathEntryMap.get(e.getKey());
+ if (pathEntries == null) {
+ pathEntries = new TreeSet<PermissionEntry>(e.getValue());
+ pathEntryMap.put(e.getKey(), pathEntries);
+ } else {
+ pathEntries.addAll(e.getValue());
+ }
+ }
+ }
+
+ public void load(PermissionStore store, Collection<PermissionEntry> ret, String principalName, String path) {
+ // todo: conditionally load entries if too many
+ PrincipalPermissionEntries ppe = getEntries(store, principalName);
+ ret.addAll(ppe.getEntries(path));
+ }
+
public boolean hasEntries(PermissionStore store, String principalName) {
+ // todo: conditionally load entries if too many
return getNumEntries(store, principalName) > 0;
}
public long getNumEntries(PermissionStore store, String principalName) {
+ // todo: conditionally load entries if too many
return getEntries(store, principalName).getEntries().size();
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProvider.java?rev=1538870&r1=1538869&r2=1538870&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProvider.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProvider.java Tue Nov 5 05:44:52 2013
@@ -24,7 +24,7 @@ import javax.annotation.Nonnull;
import org.apache.jackrabbit.oak.api.Tree;
/**
- * <code>PermissionEntryProvider</code> provides permission entries for a given set of principals.
+ * {@code PermissionEntryProvider} provides permission entries for a given set of principals.
* It may internally hold a cache to improve performance and usually operates on the permission store.
*/
public interface PermissionEntryProvider {
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java?rev=1538870&r1=1538869&r2=1538870&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java Tue Nov 5 05:44:52 2013
@@ -35,7 +35,7 @@ import com.google.common.base.Strings;
import com.google.common.collect.Iterators;
/**
- * <code>PermissionEntryProviderImpl</code>...
+ * {@code PermissionEntryProviderImpl} ...
*/
public class PermissionEntryProviderImpl implements PermissionEntryProvider {
@@ -75,9 +75,15 @@ public class PermissionEntryProviderImpl
}
}
}
- this.pathEntryMap = cnt < MAX_SIZE
- ? createMap(cache, store, existingNames)
- : null;
+ if (cnt < MAX_SIZE) {
+ // cache all entries of all principals
+ pathEntryMap = new HashMap<String, Collection<PermissionEntry>>();
+ for (String name: principalNames) {
+ cache.load(store, pathEntryMap, name);
+ }
+ } else {
+ pathEntryMap = null;
+ }
}
public void flush() {
@@ -122,35 +128,18 @@ public class PermissionEntryProviderImpl
private Collection<PermissionEntry> loadEntries(@Nonnull String path) {
Collection<PermissionEntry> ret = new TreeSet<PermissionEntry>();
for (String name: existingNames) {
- PrincipalPermissionEntries ppe = cache.getEntries(store, name);
- ret.addAll(ppe.getEntries(path));
+ cache.load(store, ret, name, path);
}
return ret;
}
- private static Map<String, Collection<PermissionEntry>> createMap(@Nonnull PermissionEntryCache.Local cache,
- @Nonnull PermissionStore store,
- @Nonnull Set<String> principalNames) {
- Map<String, Collection<PermissionEntry>> pathEntryMap = new HashMap<String, Collection<PermissionEntry>>();
- for (String name: principalNames) {
- PrincipalPermissionEntries ppe = cache.getEntries(store, name);
- for (Map.Entry<String, Collection<PermissionEntry>> e: ppe.getEntries().entrySet()) {
- Collection<PermissionEntry> pathEntries = pathEntryMap.get(e.getKey());
- if (pathEntries == null) {
- pathEntries = new TreeSet<PermissionEntry>(e.getValue());
- pathEntryMap.put(e.getKey(), pathEntries);
- } else {
- pathEntries.addAll(e.getValue());
- }
- }
- }
- return pathEntryMap;
- }
-
private final class EntryIterator extends AbstractEntryIterator {
private final EntryPredicate predicate;
+ // the ordered permission entries at a given path in the hierarchy
+ private Iterator<PermissionEntry> nextEntries = Iterators.emptyIterator();
+
// the next oak path for which to retrieve permission entries
private String path;
@@ -159,9 +148,9 @@ public class PermissionEntryProviderImpl
this.path = Strings.nullToEmpty(predicate.getPath());
}
- @CheckForNull
+ @Override
protected void seekNext() {
- for (next = null; next == null; ) {
+ for (next = null; next == null;) {
if (nextEntries.hasNext()) {
PermissionEntry pe = nextEntries.next();
if (predicate.apply(pe)) {