[
https://issues.apache.org/jira/browse/OAK-3003?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14638413#comment-14638413
]
angela edited comment on OAK-3003 at 7/23/15 8:38 AM:
------------------------------------------------------
[~rr...@adobe.com], [~lbyrum], as you can see in the attached diff the user
management API calls are not target of this patch and will not be affected in
any way. in other words: {{Group.get*Members}} and {{Authorizable.*memberOf()}}
and other related calls will just work exactly as before and will never make
use of this cache. as you can see doesn't even store the group IDs but the
principal names which cannot be used in the user management API anyway. So, the
Sling content distribution for user and group accounts will IMO not be affect.
Also the cache is system maintained content and cannot be created/modified
using regular JCR/Oak API calls and thus must be ignored as any other protected
content that doesn't have public API for modification (note: deletion should
however be possible; at least on the Oak API).
According to the subject the only reason for this this cache is to improve
login performance (where the principals and not the users/groups are relevant)
and therefore the patch is both in terms of code and in terms of effect limited
to the {{PrincipalProvider}}.
As far as the configuration option is concerned: This performance improvement
is only relevant in scenarios where a given user is member of many groups and
having dynamic group principals (dynamic in the same sense as we have it for
the "everyone" principal compared to explicitly stored membership) are not
available (which IMHO would be the preferred way to address this in any case).
Having user management being responsible for principal resolution is just a
nice default but by no means the only way to do this as user management is from
a repository point of view completely optional (and actually the least
important part of the whole security setup). See oak-exercise for examples and
training material to make this clear (and also to understand the difference
between principal and user management).
As far as explicit deletion is concerned as stated in {quote}Would you be OK
with clients using explicit deletion of the rep:cache node on updates? or would
you consider that exploiting an implementation detail?{quote}:
What updates are you referring to? Change to group membership on the group
node? That would only scale for users being removed from a group, where the
effect is limited to a single node to be touched; not for group-group
membership where you had to touch all declared and inherited members of the
group. Since the cache is only used for principal resolution and intended to
have a short expiration time (a couple of requests only), I wouldn't expect it
to be required or even desired anyway. In any case this is for sure an
implementation detail and you need to be aware that it might break in a
different setup; in particular since as of Oak 1.0 all the security components
can be plugged and replaced at runtime and you have no guarantee whatsoever
that a given setup will stick with this implementation of the user management
API (again: it's really a totally optionally component which you can get rid of
with both a custom principal provider and a custom login module... about 2 days
of work ;-)).
Regarding {quote}For my team at least the goal wasn't to address cases of
extremely dynamic group memberships, but rather large memberships with changes
only on the scale of once a day.{quote}:
The patch is not suited for extremely dynamic group membership but exactly for
the use case, where a user is member of a lot of groups which doesn't change
too often and thus can be cached for the time of one to some requests to
improve login performance. However you have to understand that this is an open
source project and we cannot introduce a cache that will only work for your
particular setup but be extremely poor in terms of scalability or performance
in a different setup (e.g. when group membership is changed more frequently and
thus updating the cache on every change would just not perform well, which is a
valid and existing use case either).
was (Author: anchela):
[~rr...@adobe.com], [~lbyrum], as you can see in the attached diff the user
management API calls are not target of this patch and will not be affected in
any way. in other words: {{Group.get*Members}} and {{Authorizable.*memberOf()}}
and other related calls will just work exactly as before and will never make
use of this cache. as you can see doesn't even store the group IDs but the
principal names which cannot be used in the user management API anyway. So, the
Sling content distribution for user and group accounts will IMO not be affect.
Also the cache is system maintained content and cannot be created/modified
using regular JCR/Oak API calls and thus must be ignored as any other protected
content that doesn't have public API for modification (note: deletion should
however be possible; at least on the Oak API).
According to the subject the only reason for this this cache is to improve
login performance (where the principals and not the users/groups are relevant)
and therefore the patch is both in terms of code and in terms of effect limited
to the {{PrincipalProvider}}.
As far as the configuration option is concerned: This performance improvement
is only relevant in scenarios where a given user is member of many groups and
having dynamic group principals are not available (which IMHO would be the
preferred way to address this in any case). Having user management being
responsible for principal resolution is just a nice default but by no means the
only way to do this as user management is from a repository point of view
completely optional (and actually the least important part of the whole
security setup). See oak-exercise for examples and training material to make
this clear (and also to understand the difference between principal and user
management).
As far as explicit deletion is concerned as stated in {quote}Would you be OK
with clients using explicit deletion of the rep:cache node on updates? or would
you consider that exploiting an implementation detail?{quote}:
What updates are you referring to? Change to group membership on the group
node? That would only scale for users being removed from a group, where the
effect is limited to a single node to be touched; not for group-group
membership where you had to touch all declared and inherited members of the
group. Since the cache is only used for principal resolution and intended to
have a short expiration time (a couple of requests only), I wouldn't expect it
to be required or even desired anyway. In any case this is for sure an
implementation detail and you need to be aware that it might break in a
different setup; in particular since as of Oak 1.0 all the security components
can be plugged and replaced at runtime and you have no guarantee whatsoever
that a given setup will stick with this implementation of the user management
API (again: it's really a totally optionally component which you can get rid of
with both a custom principal provider and a custom login module... about 2 days
of work ;-)).
Regarding {quote}For my team at least the goal wasn't to address cases of
extremely dynamic group memberships, but rather large memberships with changes
only on the scale of once a day.{quote}:
The patch is not suited for extremely dynamic group membership but exactly for
the use case, where a user is member of a lot of groups which doesn't change
too often and thus can be cached for the time of one to some requests to
improve login performance. However you have to understand that this is an open
source project and we cannot introduce a cache that will only work for your
particular setup but be extremely poor in terms of scalability or performance
in a different setup (e.g. when group membership is changed more frequently and
thus updating the cache on every change would just not perform well, which is a
valid and existing use case either).
> Improve login performance with huge group membership
> ----------------------------------------------------
>
> Key: OAK-3003
> URL: https://issues.apache.org/jira/browse/OAK-3003
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: angela
> Assignee: angela
> Attachments: OAK-3003.patch, group_cache_in_userprincipalprovider.txt
>
>
> As visible when running {{LoginWithMembershipTest}} default login performance
> (which uses the {{o.a.j.oak.security.principal.PrincipalProviderImpl}} to
> lookup the complete set of principals) suffers when a given user is member of
> a huge number of groups (see also OAK-2690 for benchmark data).
> While using dynamic group membership (and thus a custom {{PrincipalProvider}}
> would be the preferable way to deal with this, we still want to optimize
> {{PrincipalProvider.getPrincipals(String userId}} for the default
> implementation.
> With the introduction of a less generic implementation in OAK-2690, we might
> be able to come up with an optimization that makes use of the very
> implementation details of the user management while at the same time being
> able to properly secure it as we won't need to extend the public API.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
