You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Felix Meschberger <fm...@gmail.com> on 2010/01/06 23:11:26 UTC

Remove Session Pooling

Hi all,

Today I stumbled upon a potential problem with the JCR Session Pooling
we have in the JCR Base bundle.

Some time ago, we disabled session pooling by default. Only today I
actually set this default for the Embedded Jackrabbit bundle (see
SLING-1272).

The problems with session pooling are manyfold, some of the issues are:

  * Only works with SimpleCredentials authentication
  * Wrong level of abstraction: such optimizations are the task of the
       repository implementation and not of the user
  * Cleanup of the session for reuse is brittle and timeconsuming
       (due to a JCR search to ensure unlocking transient locks)
  * Little to no gain in performance (in fact performance is even
       lower than using plain Jackrabbit Sessions.

The only real use of the current session pooling, we might discuss, is
the optional limitation of concurrent requests per user. But even this
feature is disabled by default.

For these reasons, I think we should remove the Session Pooling support
from the JCR base bundle.

WDYT ?

Regards
Felix

Re: Remove Session Pooling

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Wed, Jan 6, 2010 at 11:11 PM, Felix Meschberger <fm...@gmail.com> wrote:
>...I think we should remove the Session Pooling support
> from the JCR base bundle...

+1

-Bertrand

Re: Remove Session Pooling

Posted by Carsten Ziegeler <cz...@apache.org>.
+1 for removal

Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Remove Session Pooling

Posted by Ian Boston <ie...@tfd.co.uk>.
On 7 Jan 2010, at 09:22, Felix Meschberger wrote:

> Hi,
> 
> You are correctly noting these potential issues.
> 
> But for a long time now, Jackrabbit has dramatically grown in this area:
> 
>  * The compiled ACLs are not cached within the session but in an
>    ACL cache (where they IMHO belong)


Perhaps I am looking in the wrong place but in acl.AbstractCompiledPermissions line 33 there is a class property "cache" which is a LRUMap

abstract compiled permissions is an class property of the acl.DefaultAccessManager  line 124 "compiledPermissions" which is initialsed at login to the session, line 156.

So AFAICT the cache is bound to the session in JR 1.6 ? 

This is the cache I am concerned about.

Is there another cache that I am missing, I couldnt see one, everything appears to go via AbstractCompilePermissions.getResult() line 42 which uses the above cache ?


>  * Session setup once was a very heavy-weight operation (due to
>    Principal lookup etc.). This has also been highly optimized by
>    now. In fact Repository.login is even as fast as (if not faster
>    than) retrieving and checking a Session from the session pool !


I agree, establishing the session will be faster than retrieving from the pool, but recreating the state of the cache I mentioned above will take longer.


> 
> In fact, for our Communiqué 5 product we have switched off session
> pooling for a long time now -- interestingly for performance and
> stability reasons.

AFAICT, in JR1.6 by default  Group deny is not allowed, hence the repo only goes read denied on a per user basis as an ACL that explicitly denies the user read is found, which is quite rare, so ACLs only get created for write operations which are typically < 10% of operations.

If however you have use cases that demand that groups can be denied (private sub tree), and you have modified the DefaultAccessManager to allow group deny, then the cache is heavilly used and most of the ACL's do get compiled and resolved even for read.

IIRC JR2 does allow group deny, or at least the compilation order is now correct to support it.

This is what I am concerned about.

------------------

I agree the stability problems may well trump any performance issues.


> 
> What you might want to check with respect to performance, is temporarily
> switching off session pooling by setting the "Max Idle Session"
> configuration value to zero (0).

Rather than do that, I will check the average number of unique ACLs accessed per call and work out the additional cost of compilation on every request. I will get back with some figures today.

Ian


> 
> Regards
> Felix
> 
> On 07.01.2010 10:12, Ian Boston wrote:
>> 
>> On 6 Jan 2010, at 22:11, Felix Meschberger wrote:
>> 
>>> Hi all,
>>> 
>>> Today I stumbled upon a potential problem with the JCR Session Pooling
>>> we have in the JCR Base bundle.
>>> 
>>> Some time ago, we disabled session pooling by default. Only today I
>>> actually set this default for the Embedded Jackrabbit bundle (see
>>> SLING-1272).
>>> 
>>> The problems with session pooling are manyfold, some of the issues are:
>>> 
>>> * Only works with SimpleCredentials authentication
>>> * Wrong level of abstraction: such optimizations are the task of the
>>>      repository implementation and not of the user
>>> * Cleanup of the session for reuse is brittle and timeconsuming
>>>      (due to a JCR search to ensure unlocking transient locks)
>>> * Little to no gain in performance (in fact performance is even
>>>      lower than using plain Jackrabbit Sessions.
>>> 
>>> The only real use of the current session pooling, we might discuss, is
>>> the optional limitation of concurrent requests per user. But even this
>>> feature is disabled by default.
>>> 
>>> For these reasons, I think we should remove the Session Pooling support
>>> from the JCR base bundle.
>>> 
>>> WDYT ?
>> 
>> 
>> What happens to compiled ACL's if there is no session pooling. IIRC where the JCR is not in "everyone can read everything" mode, the Session is the location where compiled ACL's are stored. If the session is not pooled every request has to recompile the ACLs.
>> 
>> This wont be noticed for situations where most reads dont need an ACL, but where they do and there are a high number of ACL (or the cost of resolving and compiling the ACLs is higher due to complex rules) then removing session pooling is going to have an impact.
>> 
>> The ACL resolution mechanism in DefaultAccessControlManager is highly optimised and very fast once the ACL has been compiled, which is good since its an extremely high traffic area of the Jackrabbit code base, but compilation of the ACL is not fast particularly where there are many ACLs effecting a single node.
>> 
>> I suspect that if you are comparing performance in "everyone can read everything" you wont see any impact, have you tried to see what happens when there is a more complex ACL structure that is compiled ?
>> 
>> Also, I was told once that JCR XASessions and the associated SecurtiyManager, and all JCR core thing with an init() attached to the session was a heavy and expensive object (relative term) that should be re-used, has this changed ?
>> 
>> I am not going to vote on this, but I do want to discuss it since when I first looked at Sling I was relieved to see session pooling in place.
>> 
>> I could also be that I am miss-understanding session pooling, but I thought the key feature was that if a user came back, and there was a session in the pool that they had used before, they got the same session back and were able to re-use all the work of previous requests in the ACL area.
>> 
>> Ian
>> 
>>> 
>>> Regards
>>> Felix
>> 
>> 
> 


Re: Remove Session Pooling

Posted by Felix Meschberger <fm...@gmail.com>.
Hi,

On 11.01.2010 09:38, Ian Boston wrote:
> 
> On 11 Jan 2010, at 07:52, Felix Meschberger wrote:
> 
>> Hi,
>>
>> On 09.01.2010 11:48, Ian Boston wrote:
>>> Ok, so I was slow on the uptake, as usual, I now see the problems and I agree, the pooling should be removed.
>>
>> Ok, shall I move on then ? Or should I wait for the full consequences
>> with respect to ACL caching (see below) are known ?
> 
> 
> I am happy with that, you have tested in performance with no pooling, and I think it will only become an issue with group deny, which is not a problem for Sling or Jackrabbit at the moment.

Ok, thanks.

Regards
Felix

> 
>>
>>>
>>> Just for the record, here is want I observed, only worth reading if you like me haven't looked at the pool code. (Felix I think you said all of this in shorthand form, sorry)
>>
>> Thanks for the flowser. Yet your description hits the nail right at the
>> center and is also very concise!
>>
>>>
>>> 1. The only sessions that can go into the pool are sessions authenticated with a password, as the password is stored with the pool and used to check if the login request can get the session out of the pool which btw is a pool for the user in question. If you have any form of LoginModule associated with an AuthenticationHandler (eg the OpenID or a container auth, CAS, webAuth, ie anything where Sling does not see the password), then the pool wont work.
>>>
>>> 2. There is one pool per user, and the "user pools" are never cleaned up. Since sessions are only cleaned when taken out of the pool, if 1M users hit your app server and then exited their browser there would be 1M pools and 1-4M open JCR sessions (browsers have 1-4 http connections per window). The current code does not clean user pools or defunct sessions.
>>>
>>> 3. The inactive session list is a linked list that needs to be tightly synchronised. I think I am seeing the same session being taken out of the pool and shared incorrectly, resulting in a release happening more than once. Some of the time this results in a logout which shows up. Since sessions are not thread safe, I think it might have been the cause of other random problems.
>>>
>>> 4. slingRepository.loginAdministrative() uses SimpleCredentials and so is pooled. Limiting the number of concurrent request that require an administrative session to < 10 (the default per user pool size).
> 
> actually this is wrong, there is no limit by default, but only the first 10 get pooled.
> 
> 
>>>
>>> --------------------------------------------------------------------
>>>
>>> Can you point me to where the compiled ACLs are cached, I cant find the code, I need to check that my customisations haven't broken anything ?
>>
>> Oops, now you got me...
>>
>> According to my interpretation of the code, your are right in saying the
>> compiled ACLs are cached per-Session and not globally...
>>
>> Unfortunately, I have to admit that this is an area of Jackrabbit code,
>> I do not know in full detail. So it might be worth asking on the
>> Jackrabbit list about this....
> 
> Ok, will do.
> 
>>
>> Regards
>> Felix
>>
>>>
>>> Thanks
>>> Ian
>>>
>>>
>>>
>>> On 7 Jan 2010, at 09:22, Felix Meschberger wrote:
>>>
>>>> Hi,
>>>>
>>>> You are correctly noting these potential issues.
>>>>
>>>> But for a long time now, Jackrabbit has dramatically grown in this area:
>>>>
>>>> * The compiled ACLs are not cached within the session but in an
>>>>   ACL cache (where they IMHO belong)
>>>> * Session setup once was a very heavy-weight operation (due to
>>>>   Principal lookup etc.). This has also been highly optimized by
>>>>   now. In fact Repository.login is even as fast as (if not faster
>>>>   than) retrieving and checking a Session from the session pool !
>>>>
>>>> In fact, for our Communiqué 5 product we have switched off session
>>>> pooling for a long time now -- interestingly for performance and
>>>> stability reasons.
>>>>
>>>> What you might want to check with respect to performance, is temporarily
>>>> switching off session pooling by setting the "Max Idle Session"
>>>> configuration value to zero (0).
>>>>
>>>> Regards
>>>> Felix
>>>>
>>>> On 07.01.2010 10:12, Ian Boston wrote:
>>>>>
>>>>> On 6 Jan 2010, at 22:11, Felix Meschberger wrote:
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> Today I stumbled upon a potential problem with the JCR Session Pooling
>>>>>> we have in the JCR Base bundle.
>>>>>>
>>>>>> Some time ago, we disabled session pooling by default. Only today I
>>>>>> actually set this default for the Embedded Jackrabbit bundle (see
>>>>>> SLING-1272).
>>>>>>
>>>>>> The problems with session pooling are manyfold, some of the issues are:
>>>>>>
>>>>>> * Only works with SimpleCredentials authentication
>>>>>> * Wrong level of abstraction: such optimizations are the task of the
>>>>>>     repository implementation and not of the user
>>>>>> * Cleanup of the session for reuse is brittle and timeconsuming
>>>>>>     (due to a JCR search to ensure unlocking transient locks)
>>>>>> * Little to no gain in performance (in fact performance is even
>>>>>>     lower than using plain Jackrabbit Sessions.
>>>>>>
>>>>>> The only real use of the current session pooling, we might discuss, is
>>>>>> the optional limitation of concurrent requests per user. But even this
>>>>>> feature is disabled by default.
>>>>>>
>>>>>> For these reasons, I think we should remove the Session Pooling support
>>>>>> from the JCR base bundle.
>>>>>>
>>>>>> WDYT ?
>>>>>
>>>>>
>>>>> What happens to compiled ACL's if there is no session pooling. IIRC where the JCR is not in "everyone can read everything" mode, the Session is the location where compiled ACL's are stored. If the session is not pooled every request has to recompile the ACLs.
>>>>>
>>>>> This wont be noticed for situations where most reads dont need an ACL, but where they do and there are a high number of ACL (or the cost of resolving and compiling the ACLs is higher due to complex rules) then removing session pooling is going to have an impact.
>>>>>
>>>>> The ACL resolution mechanism in DefaultAccessControlManager is highly optimised and very fast once the ACL has been compiled, which is good since its an extremely high traffic area of the Jackrabbit code base, but compilation of the ACL is not fast particularly where there are many ACLs effecting a single node.
>>>>>
>>>>> I suspect that if you are comparing performance in "everyone can read everything" you wont see any impact, have you tried to see what happens when there is a more complex ACL structure that is compiled ?
>>>>>
>>>>> Also, I was told once that JCR XASessions and the associated SecurtiyManager, and all JCR core thing with an init() attached to the session was a heavy and expensive object (relative term) that should be re-used, has this changed ?
>>>>>
>>>>> I am not going to vote on this, but I do want to discuss it since when I first looked at Sling I was relieved to see session pooling in place.
>>>>>
>>>>> I could also be that I am miss-understanding session pooling, but I thought the key feature was that if a user came back, and there was a session in the pool that they had used before, they got the same session back and were able to re-use all the work of previous requests in the ACL area.
>>>>>
>>>>> Ian
>>>>>
>>>>>>
>>>>>> Regards
>>>>>> Felix
>>>>>
>>>>>
>>>>
>>>
>>>
>>
> 
> 


Re: Remove Session Pooling

Posted by Ian Boston <ie...@tfd.co.uk>.
On 11 Jan 2010, at 07:52, Felix Meschberger wrote:

> Hi,
> 
> On 09.01.2010 11:48, Ian Boston wrote:
>> Ok, so I was slow on the uptake, as usual, I now see the problems and I agree, the pooling should be removed.
> 
> Ok, shall I move on then ? Or should I wait for the full consequences
> with respect to ACL caching (see below) are known ?


I am happy with that, you have tested in performance with no pooling, and I think it will only become an issue with group deny, which is not a problem for Sling or Jackrabbit at the moment.

> 
>> 
>> Just for the record, here is want I observed, only worth reading if you like me haven't looked at the pool code. (Felix I think you said all of this in shorthand form, sorry)
> 
> Thanks for the flowser. Yet your description hits the nail right at the
> center and is also very concise!
> 
>> 
>> 1. The only sessions that can go into the pool are sessions authenticated with a password, as the password is stored with the pool and used to check if the login request can get the session out of the pool which btw is a pool for the user in question. If you have any form of LoginModule associated with an AuthenticationHandler (eg the OpenID or a container auth, CAS, webAuth, ie anything where Sling does not see the password), then the pool wont work.
>> 
>> 2. There is one pool per user, and the "user pools" are never cleaned up. Since sessions are only cleaned when taken out of the pool, if 1M users hit your app server and then exited their browser there would be 1M pools and 1-4M open JCR sessions (browsers have 1-4 http connections per window). The current code does not clean user pools or defunct sessions.
>> 
>> 3. The inactive session list is a linked list that needs to be tightly synchronised. I think I am seeing the same session being taken out of the pool and shared incorrectly, resulting in a release happening more than once. Some of the time this results in a logout which shows up. Since sessions are not thread safe, I think it might have been the cause of other random problems.
>> 
>> 4. slingRepository.loginAdministrative() uses SimpleCredentials and so is pooled. Limiting the number of concurrent request that require an administrative session to < 10 (the default per user pool size).

actually this is wrong, there is no limit by default, but only the first 10 get pooled.


>> 
>> --------------------------------------------------------------------
>> 
>> Can you point me to where the compiled ACLs are cached, I cant find the code, I need to check that my customisations haven't broken anything ?
> 
> Oops, now you got me...
> 
> According to my interpretation of the code, your are right in saying the
> compiled ACLs are cached per-Session and not globally...
> 
> Unfortunately, I have to admit that this is an area of Jackrabbit code,
> I do not know in full detail. So it might be worth asking on the
> Jackrabbit list about this....

Ok, will do.

> 
> Regards
> Felix
> 
>> 
>> Thanks
>> Ian
>> 
>> 
>> 
>> On 7 Jan 2010, at 09:22, Felix Meschberger wrote:
>> 
>>> Hi,
>>> 
>>> You are correctly noting these potential issues.
>>> 
>>> But for a long time now, Jackrabbit has dramatically grown in this area:
>>> 
>>> * The compiled ACLs are not cached within the session but in an
>>>   ACL cache (where they IMHO belong)
>>> * Session setup once was a very heavy-weight operation (due to
>>>   Principal lookup etc.). This has also been highly optimized by
>>>   now. In fact Repository.login is even as fast as (if not faster
>>>   than) retrieving and checking a Session from the session pool !
>>> 
>>> In fact, for our Communiqué 5 product we have switched off session
>>> pooling for a long time now -- interestingly for performance and
>>> stability reasons.
>>> 
>>> What you might want to check with respect to performance, is temporarily
>>> switching off session pooling by setting the "Max Idle Session"
>>> configuration value to zero (0).
>>> 
>>> Regards
>>> Felix
>>> 
>>> On 07.01.2010 10:12, Ian Boston wrote:
>>>> 
>>>> On 6 Jan 2010, at 22:11, Felix Meschberger wrote:
>>>> 
>>>>> Hi all,
>>>>> 
>>>>> Today I stumbled upon a potential problem with the JCR Session Pooling
>>>>> we have in the JCR Base bundle.
>>>>> 
>>>>> Some time ago, we disabled session pooling by default. Only today I
>>>>> actually set this default for the Embedded Jackrabbit bundle (see
>>>>> SLING-1272).
>>>>> 
>>>>> The problems with session pooling are manyfold, some of the issues are:
>>>>> 
>>>>> * Only works with SimpleCredentials authentication
>>>>> * Wrong level of abstraction: such optimizations are the task of the
>>>>>     repository implementation and not of the user
>>>>> * Cleanup of the session for reuse is brittle and timeconsuming
>>>>>     (due to a JCR search to ensure unlocking transient locks)
>>>>> * Little to no gain in performance (in fact performance is even
>>>>>     lower than using plain Jackrabbit Sessions.
>>>>> 
>>>>> The only real use of the current session pooling, we might discuss, is
>>>>> the optional limitation of concurrent requests per user. But even this
>>>>> feature is disabled by default.
>>>>> 
>>>>> For these reasons, I think we should remove the Session Pooling support
>>>>> from the JCR base bundle.
>>>>> 
>>>>> WDYT ?
>>>> 
>>>> 
>>>> What happens to compiled ACL's if there is no session pooling. IIRC where the JCR is not in "everyone can read everything" mode, the Session is the location where compiled ACL's are stored. If the session is not pooled every request has to recompile the ACLs.
>>>> 
>>>> This wont be noticed for situations where most reads dont need an ACL, but where they do and there are a high number of ACL (or the cost of resolving and compiling the ACLs is higher due to complex rules) then removing session pooling is going to have an impact.
>>>> 
>>>> The ACL resolution mechanism in DefaultAccessControlManager is highly optimised and very fast once the ACL has been compiled, which is good since its an extremely high traffic area of the Jackrabbit code base, but compilation of the ACL is not fast particularly where there are many ACLs effecting a single node.
>>>> 
>>>> I suspect that if you are comparing performance in "everyone can read everything" you wont see any impact, have you tried to see what happens when there is a more complex ACL structure that is compiled ?
>>>> 
>>>> Also, I was told once that JCR XASessions and the associated SecurtiyManager, and all JCR core thing with an init() attached to the session was a heavy and expensive object (relative term) that should be re-used, has this changed ?
>>>> 
>>>> I am not going to vote on this, but I do want to discuss it since when I first looked at Sling I was relieved to see session pooling in place.
>>>> 
>>>> I could also be that I am miss-understanding session pooling, but I thought the key feature was that if a user came back, and there was a session in the pool that they had used before, they got the same session back and were able to re-use all the work of previous requests in the ACL area.
>>>> 
>>>> Ian
>>>> 
>>>>> 
>>>>> Regards
>>>>> Felix
>>>> 
>>>> 
>>> 
>> 
>> 
> 


Re: Remove Session Pooling

Posted by Felix Meschberger <fm...@gmail.com>.
Hi,

On 09.01.2010 11:48, Ian Boston wrote:
> Ok, so I was slow on the uptake, as usual, I now see the problems and I agree, the pooling should be removed.

Ok, shall I move on then ? Or should I wait for the full consequences
with respect to ACL caching (see below) are known ?

> 
> Just for the record, here is want I observed, only worth reading if you like me haven't looked at the pool code. (Felix I think you said all of this in shorthand form, sorry)

Thanks for the flowser. Yet your description hits the nail right at the
center and is also very concise!

> 
> 1. The only sessions that can go into the pool are sessions authenticated with a password, as the password is stored with the pool and used to check if the login request can get the session out of the pool which btw is a pool for the user in question. If you have any form of LoginModule associated with an AuthenticationHandler (eg the OpenID or a container auth, CAS, webAuth, ie anything where Sling does not see the password), then the pool wont work.
> 
> 2. There is one pool per user, and the "user pools" are never cleaned up. Since sessions are only cleaned when taken out of the pool, if 1M users hit your app server and then exited their browser there would be 1M pools and 1-4M open JCR sessions (browsers have 1-4 http connections per window). The current code does not clean user pools or defunct sessions.
> 
> 3. The inactive session list is a linked list that needs to be tightly synchronised. I think I am seeing the same session being taken out of the pool and shared incorrectly, resulting in a release happening more than once. Some of the time this results in a logout which shows up. Since sessions are not thread safe, I think it might have been the cause of other random problems.
> 
> 4. slingRepository.loginAdministrative() uses SimpleCredentials and so is pooled. Limiting the number of concurrent request that require an administrative session to < 10 (the default per user pool size).
> 
> --------------------------------------------------------------------
> 
> Can you point me to where the compiled ACLs are cached, I cant find the code, I need to check that my customisations haven't broken anything ?

Oops, now you got me...

According to my interpretation of the code, your are right in saying the
compiled ACLs are cached per-Session and not globally...

Unfortunately, I have to admit that this is an area of Jackrabbit code,
I do not know in full detail. So it might be worth asking on the
Jackrabbit list about this....

Regards
Felix

> 
> Thanks
> Ian
> 
> 
> 
> On 7 Jan 2010, at 09:22, Felix Meschberger wrote:
> 
>> Hi,
>>
>> You are correctly noting these potential issues.
>>
>> But for a long time now, Jackrabbit has dramatically grown in this area:
>>
>>  * The compiled ACLs are not cached within the session but in an
>>    ACL cache (where they IMHO belong)
>>  * Session setup once was a very heavy-weight operation (due to
>>    Principal lookup etc.). This has also been highly optimized by
>>    now. In fact Repository.login is even as fast as (if not faster
>>    than) retrieving and checking a Session from the session pool !
>>
>> In fact, for our Communiqué 5 product we have switched off session
>> pooling for a long time now -- interestingly for performance and
>> stability reasons.
>>
>> What you might want to check with respect to performance, is temporarily
>> switching off session pooling by setting the "Max Idle Session"
>> configuration value to zero (0).
>>
>> Regards
>> Felix
>>
>> On 07.01.2010 10:12, Ian Boston wrote:
>>>
>>> On 6 Jan 2010, at 22:11, Felix Meschberger wrote:
>>>
>>>> Hi all,
>>>>
>>>> Today I stumbled upon a potential problem with the JCR Session Pooling
>>>> we have in the JCR Base bundle.
>>>>
>>>> Some time ago, we disabled session pooling by default. Only today I
>>>> actually set this default for the Embedded Jackrabbit bundle (see
>>>> SLING-1272).
>>>>
>>>> The problems with session pooling are manyfold, some of the issues are:
>>>>
>>>> * Only works with SimpleCredentials authentication
>>>> * Wrong level of abstraction: such optimizations are the task of the
>>>>      repository implementation and not of the user
>>>> * Cleanup of the session for reuse is brittle and timeconsuming
>>>>      (due to a JCR search to ensure unlocking transient locks)
>>>> * Little to no gain in performance (in fact performance is even
>>>>      lower than using plain Jackrabbit Sessions.
>>>>
>>>> The only real use of the current session pooling, we might discuss, is
>>>> the optional limitation of concurrent requests per user. But even this
>>>> feature is disabled by default.
>>>>
>>>> For these reasons, I think we should remove the Session Pooling support
>>>> from the JCR base bundle.
>>>>
>>>> WDYT ?
>>>
>>>
>>> What happens to compiled ACL's if there is no session pooling. IIRC where the JCR is not in "everyone can read everything" mode, the Session is the location where compiled ACL's are stored. If the session is not pooled every request has to recompile the ACLs.
>>>
>>> This wont be noticed for situations where most reads dont need an ACL, but where they do and there are a high number of ACL (or the cost of resolving and compiling the ACLs is higher due to complex rules) then removing session pooling is going to have an impact.
>>>
>>> The ACL resolution mechanism in DefaultAccessControlManager is highly optimised and very fast once the ACL has been compiled, which is good since its an extremely high traffic area of the Jackrabbit code base, but compilation of the ACL is not fast particularly where there are many ACLs effecting a single node.
>>>
>>> I suspect that if you are comparing performance in "everyone can read everything" you wont see any impact, have you tried to see what happens when there is a more complex ACL structure that is compiled ?
>>>
>>> Also, I was told once that JCR XASessions and the associated SecurtiyManager, and all JCR core thing with an init() attached to the session was a heavy and expensive object (relative term) that should be re-used, has this changed ?
>>>
>>> I am not going to vote on this, but I do want to discuss it since when I first looked at Sling I was relieved to see session pooling in place.
>>>
>>> I could also be that I am miss-understanding session pooling, but I thought the key feature was that if a user came back, and there was a session in the pool that they had used before, they got the same session back and were able to re-use all the work of previous requests in the ACL area.
>>>
>>> Ian
>>>
>>>>
>>>> Regards
>>>> Felix
>>>
>>>
>>
> 
> 


Re: Remove Session Pooling

Posted by Ian Boston <ie...@tfd.co.uk>.
Ok, so I was slow on the uptake, as usual, I now see the problems and I agree, the pooling should be removed.

Just for the record, here is want I observed, only worth reading if you like me haven't looked at the pool code. (Felix I think you said all of this in shorthand form, sorry)

1. The only sessions that can go into the pool are sessions authenticated with a password, as the password is stored with the pool and used to check if the login request can get the session out of the pool which btw is a pool for the user in question. If you have any form of LoginModule associated with an AuthenticationHandler (eg the OpenID or a container auth, CAS, webAuth, ie anything where Sling does not see the password), then the pool wont work.

2. There is one pool per user, and the "user pools" are never cleaned up. Since sessions are only cleaned when taken out of the pool, if 1M users hit your app server and then exited their browser there would be 1M pools and 1-4M open JCR sessions (browsers have 1-4 http connections per window). The current code does not clean user pools or defunct sessions.

3. The inactive session list is a linked list that needs to be tightly synchronised. I think I am seeing the same session being taken out of the pool and shared incorrectly, resulting in a release happening more than once. Some of the time this results in a logout which shows up. Since sessions are not thread safe, I think it might have been the cause of other random problems.

4. slingRepository.loginAdministrative() uses SimpleCredentials and so is pooled. Limiting the number of concurrent request that require an administrative session to < 10 (the default per user pool size).

--------------------------------------------------------------------

Can you point me to where the compiled ACLs are cached, I cant find the code, I need to check that my customisations haven't broken anything ?

Thanks
Ian



On 7 Jan 2010, at 09:22, Felix Meschberger wrote:

> Hi,
> 
> You are correctly noting these potential issues.
> 
> But for a long time now, Jackrabbit has dramatically grown in this area:
> 
>  * The compiled ACLs are not cached within the session but in an
>    ACL cache (where they IMHO belong)
>  * Session setup once was a very heavy-weight operation (due to
>    Principal lookup etc.). This has also been highly optimized by
>    now. In fact Repository.login is even as fast as (if not faster
>    than) retrieving and checking a Session from the session pool !
> 
> In fact, for our Communiqué 5 product we have switched off session
> pooling for a long time now -- interestingly for performance and
> stability reasons.
> 
> What you might want to check with respect to performance, is temporarily
> switching off session pooling by setting the "Max Idle Session"
> configuration value to zero (0).
> 
> Regards
> Felix
> 
> On 07.01.2010 10:12, Ian Boston wrote:
>> 
>> On 6 Jan 2010, at 22:11, Felix Meschberger wrote:
>> 
>>> Hi all,
>>> 
>>> Today I stumbled upon a potential problem with the JCR Session Pooling
>>> we have in the JCR Base bundle.
>>> 
>>> Some time ago, we disabled session pooling by default. Only today I
>>> actually set this default for the Embedded Jackrabbit bundle (see
>>> SLING-1272).
>>> 
>>> The problems with session pooling are manyfold, some of the issues are:
>>> 
>>> * Only works with SimpleCredentials authentication
>>> * Wrong level of abstraction: such optimizations are the task of the
>>>      repository implementation and not of the user
>>> * Cleanup of the session for reuse is brittle and timeconsuming
>>>      (due to a JCR search to ensure unlocking transient locks)
>>> * Little to no gain in performance (in fact performance is even
>>>      lower than using plain Jackrabbit Sessions.
>>> 
>>> The only real use of the current session pooling, we might discuss, is
>>> the optional limitation of concurrent requests per user. But even this
>>> feature is disabled by default.
>>> 
>>> For these reasons, I think we should remove the Session Pooling support
>>> from the JCR base bundle.
>>> 
>>> WDYT ?
>> 
>> 
>> What happens to compiled ACL's if there is no session pooling. IIRC where the JCR is not in "everyone can read everything" mode, the Session is the location where compiled ACL's are stored. If the session is not pooled every request has to recompile the ACLs.
>> 
>> This wont be noticed for situations where most reads dont need an ACL, but where they do and there are a high number of ACL (or the cost of resolving and compiling the ACLs is higher due to complex rules) then removing session pooling is going to have an impact.
>> 
>> The ACL resolution mechanism in DefaultAccessControlManager is highly optimised and very fast once the ACL has been compiled, which is good since its an extremely high traffic area of the Jackrabbit code base, but compilation of the ACL is not fast particularly where there are many ACLs effecting a single node.
>> 
>> I suspect that if you are comparing performance in "everyone can read everything" you wont see any impact, have you tried to see what happens when there is a more complex ACL structure that is compiled ?
>> 
>> Also, I was told once that JCR XASessions and the associated SecurtiyManager, and all JCR core thing with an init() attached to the session was a heavy and expensive object (relative term) that should be re-used, has this changed ?
>> 
>> I am not going to vote on this, but I do want to discuss it since when I first looked at Sling I was relieved to see session pooling in place.
>> 
>> I could also be that I am miss-understanding session pooling, but I thought the key feature was that if a user came back, and there was a session in the pool that they had used before, they got the same session back and were able to re-use all the work of previous requests in the ACL area.
>> 
>> Ian
>> 
>>> 
>>> Regards
>>> Felix
>> 
>> 
> 


Re: Remove Session Pooling

Posted by Felix Meschberger <fm...@gmail.com>.
Hi,

You are correctly noting these potential issues.

But for a long time now, Jackrabbit has dramatically grown in this area:

  * The compiled ACLs are not cached within the session but in an
    ACL cache (where they IMHO belong)
  * Session setup once was a very heavy-weight operation (due to
    Principal lookup etc.). This has also been highly optimized by
    now. In fact Repository.login is even as fast as (if not faster
    than) retrieving and checking a Session from the session pool !

In fact, for our Communiqué 5 product we have switched off session
pooling for a long time now -- interestingly for performance and
stability reasons.

What you might want to check with respect to performance, is temporarily
switching off session pooling by setting the "Max Idle Session"
configuration value to zero (0).

Regards
Felix

On 07.01.2010 10:12, Ian Boston wrote:
> 
> On 6 Jan 2010, at 22:11, Felix Meschberger wrote:
> 
>> Hi all,
>>
>> Today I stumbled upon a potential problem with the JCR Session Pooling
>> we have in the JCR Base bundle.
>>
>> Some time ago, we disabled session pooling by default. Only today I
>> actually set this default for the Embedded Jackrabbit bundle (see
>> SLING-1272).
>>
>> The problems with session pooling are manyfold, some of the issues are:
>>
>>  * Only works with SimpleCredentials authentication
>>  * Wrong level of abstraction: such optimizations are the task of the
>>       repository implementation and not of the user
>>  * Cleanup of the session for reuse is brittle and timeconsuming
>>       (due to a JCR search to ensure unlocking transient locks)
>>  * Little to no gain in performance (in fact performance is even
>>       lower than using plain Jackrabbit Sessions.
>>
>> The only real use of the current session pooling, we might discuss, is
>> the optional limitation of concurrent requests per user. But even this
>> feature is disabled by default.
>>
>> For these reasons, I think we should remove the Session Pooling support
>> from the JCR base bundle.
>>
>> WDYT ?
> 
> 
> What happens to compiled ACL's if there is no session pooling. IIRC where the JCR is not in "everyone can read everything" mode, the Session is the location where compiled ACL's are stored. If the session is not pooled every request has to recompile the ACLs.
> 
> This wont be noticed for situations where most reads dont need an ACL, but where they do and there are a high number of ACL (or the cost of resolving and compiling the ACLs is higher due to complex rules) then removing session pooling is going to have an impact.
> 
> The ACL resolution mechanism in DefaultAccessControlManager is highly optimised and very fast once the ACL has been compiled, which is good since its an extremely high traffic area of the Jackrabbit code base, but compilation of the ACL is not fast particularly where there are many ACLs effecting a single node.
> 
> I suspect that if you are comparing performance in "everyone can read everything" you wont see any impact, have you tried to see what happens when there is a more complex ACL structure that is compiled ?
> 
> Also, I was told once that JCR XASessions and the associated SecurtiyManager, and all JCR core thing with an init() attached to the session was a heavy and expensive object (relative term) that should be re-used, has this changed ?
> 
> I am not going to vote on this, but I do want to discuss it since when I first looked at Sling I was relieved to see session pooling in place.
> 
> I could also be that I am miss-understanding session pooling, but I thought the key feature was that if a user came back, and there was a session in the pool that they had used before, they got the same session back and were able to re-use all the work of previous requests in the ACL area.
> 
> Ian
> 
>>
>> Regards
>> Felix
> 
> 


Re: Remove Session Pooling

Posted by Ian Boston <ie...@tfd.co.uk>.
On 6 Jan 2010, at 22:11, Felix Meschberger wrote:

> Hi all,
> 
> Today I stumbled upon a potential problem with the JCR Session Pooling
> we have in the JCR Base bundle.
> 
> Some time ago, we disabled session pooling by default. Only today I
> actually set this default for the Embedded Jackrabbit bundle (see
> SLING-1272).
> 
> The problems with session pooling are manyfold, some of the issues are:
> 
>  * Only works with SimpleCredentials authentication
>  * Wrong level of abstraction: such optimizations are the task of the
>       repository implementation and not of the user
>  * Cleanup of the session for reuse is brittle and timeconsuming
>       (due to a JCR search to ensure unlocking transient locks)
>  * Little to no gain in performance (in fact performance is even
>       lower than using plain Jackrabbit Sessions.
> 
> The only real use of the current session pooling, we might discuss, is
> the optional limitation of concurrent requests per user. But even this
> feature is disabled by default.
> 
> For these reasons, I think we should remove the Session Pooling support
> from the JCR base bundle.
> 
> WDYT ?


What happens to compiled ACL's if there is no session pooling. IIRC where the JCR is not in "everyone can read everything" mode, the Session is the location where compiled ACL's are stored. If the session is not pooled every request has to recompile the ACLs.

This wont be noticed for situations where most reads dont need an ACL, but where they do and there are a high number of ACL (or the cost of resolving and compiling the ACLs is higher due to complex rules) then removing session pooling is going to have an impact.

The ACL resolution mechanism in DefaultAccessControlManager is highly optimised and very fast once the ACL has been compiled, which is good since its an extremely high traffic area of the Jackrabbit code base, but compilation of the ACL is not fast particularly where there are many ACLs effecting a single node.

I suspect that if you are comparing performance in "everyone can read everything" you wont see any impact, have you tried to see what happens when there is a more complex ACL structure that is compiled ?

Also, I was told once that JCR XASessions and the associated SecurtiyManager, and all JCR core thing with an init() attached to the session was a heavy and expensive object (relative term) that should be re-used, has this changed ?

I am not going to vote on this, but I do want to discuss it since when I first looked at Sling I was relieved to see session pooling in place.

I could also be that I am miss-understanding session pooling, but I thought the key feature was that if a user came back, and there was a session in the pool that they had used before, they got the same session back and were able to re-use all the work of previous requests in the ACL area.

Ian

> 
> Regards
> Felix


Re: Remove Session Pooling

Posted by Felix Meschberger <fm...@gmail.com>.
I have created SLING-1283 to track this removal.

Regards
Felix

[1] https://issues.apache.org/jira/browse/SLING-1283

On 06.01.2010 23:11, Felix Meschberger wrote:
> Hi all,
> 
> Today I stumbled upon a potential problem with the JCR Session Pooling
> we have in the JCR Base bundle.
> 
> Some time ago, we disabled session pooling by default. Only today I
> actually set this default for the Embedded Jackrabbit bundle (see
> SLING-1272).
> 
> The problems with session pooling are manyfold, some of the issues are:
> 
>   * Only works with SimpleCredentials authentication
>   * Wrong level of abstraction: such optimizations are the task of the
>        repository implementation and not of the user
>   * Cleanup of the session for reuse is brittle and timeconsuming
>        (due to a JCR search to ensure unlocking transient locks)
>   * Little to no gain in performance (in fact performance is even
>        lower than using plain Jackrabbit Sessions.
> 
> The only real use of the current session pooling, we might discuss, is
> the optional limitation of concurrent requests per user. But even this
> feature is disabled by default.
> 
> For these reasons, I think we should remove the Session Pooling support
> from the JCR base bundle.
> 
> WDYT ?
> 
> Regards
> Felix

Re: Remove Session Pooling

Posted by Justin Edelson <ju...@gmail.com>.

On Jan 6, 2010, at 5:26 PM, Felix Meschberger <fm...@gmail.com>  
wrote:

> Hi,
>
> On 06.01.2010 23:20, Justin Edelson wrote:
>> +1
>>
>> Session pooling is part of Jackrabbit 2 (which is where it belongs)
>> anyway. IIRC, you removed it in your JR 2 branch.
>
> I actually removed it in that branch exactly for the reasons outlined
> below ;-)

And I now realize that it was db connection pooling which was added  
in  JR 2.

Justin

> Regards
> Felix
>
>>
>> On Jan 6, 2010, at 5:11 PM, Felix Meschberger <fm...@gmail.com>  
>> wrote:
>>
>>> Hi all,
>>>
>>> Today I stumbled upon a potential problem with the JCR Session  
>>> Pooling
>>> we have in the JCR Base bundle.
>>>
>>> Some time ago, we disabled session pooling by default. Only today I
>>> actually set this default for the Embedded Jackrabbit bundle (see
>>> SLING-1272).
>>>
>>> The problems with session pooling are manyfold, some of the issues  
>>> are:
>>>
>>> * Only works with SimpleCredentials authentication
>>> * Wrong level of abstraction: such optimizations are the task of the
>>>      repository implementation and not of the user
>>> * Cleanup of the session for reuse is brittle and timeconsuming
>>>      (due to a JCR search to ensure unlocking transient locks)
>>> * Little to no gain in performance (in fact performance is even
>>>      lower than using plain Jackrabbit Sessions.
>>>
>>> The only real use of the current session pooling, we might  
>>> discuss, is
>>> the optional limitation of concurrent requests per user. But even  
>>> this
>>> feature is disabled by default.
>>>
>>> For these reasons, I think we should remove the Session Pooling  
>>> support
>>> from the JCR base bundle.
>>>
>>> WDYT ?
>>>
>>> Regards
>>> Felix
>>

Re: Remove Session Pooling

Posted by Felix Meschberger <fm...@gmail.com>.
Hi,

On 06.01.2010 23:20, Justin Edelson wrote:
> +1
> 
> Session pooling is part of Jackrabbit 2 (which is where it belongs)
> anyway. IIRC, you removed it in your JR 2 branch.

I actually removed it in that branch exactly for the reasons outlined
below ;-)

Regards
Felix

> 
> On Jan 6, 2010, at 5:11 PM, Felix Meschberger <fm...@gmail.com> wrote:
> 
>> Hi all,
>>
>> Today I stumbled upon a potential problem with the JCR Session Pooling
>> we have in the JCR Base bundle.
>>
>> Some time ago, we disabled session pooling by default. Only today I
>> actually set this default for the Embedded Jackrabbit bundle (see
>> SLING-1272).
>>
>> The problems with session pooling are manyfold, some of the issues are:
>>
>>  * Only works with SimpleCredentials authentication
>>  * Wrong level of abstraction: such optimizations are the task of the
>>       repository implementation and not of the user
>>  * Cleanup of the session for reuse is brittle and timeconsuming
>>       (due to a JCR search to ensure unlocking transient locks)
>>  * Little to no gain in performance (in fact performance is even
>>       lower than using plain Jackrabbit Sessions.
>>
>> The only real use of the current session pooling, we might discuss, is
>> the optional limitation of concurrent requests per user. But even this
>> feature is disabled by default.
>>
>> For these reasons, I think we should remove the Session Pooling support
>> from the JCR base bundle.
>>
>> WDYT ?
>>
>> Regards
>> Felix
> 

Re: Remove Session Pooling

Posted by Justin Edelson <ju...@gmail.com>.
+1

Session pooling is part of Jackrabbit 2 (which is where it belongs)  
anyway. IIRC, you removed it in your JR 2 branch.

On Jan 6, 2010, at 5:11 PM, Felix Meschberger <fm...@gmail.com>  
wrote:

> Hi all,
>
> Today I stumbled upon a potential problem with the JCR Session Pooling
> we have in the JCR Base bundle.
>
> Some time ago, we disabled session pooling by default. Only today I
> actually set this default for the Embedded Jackrabbit bundle (see
> SLING-1272).
>
> The problems with session pooling are manyfold, some of the issues  
> are:
>
>  * Only works with SimpleCredentials authentication
>  * Wrong level of abstraction: such optimizations are the task of the
>       repository implementation and not of the user
>  * Cleanup of the session for reuse is brittle and timeconsuming
>       (due to a JCR search to ensure unlocking transient locks)
>  * Little to no gain in performance (in fact performance is even
>       lower than using plain Jackrabbit Sessions.
>
> The only real use of the current session pooling, we might discuss, is
> the optional limitation of concurrent requests per user. But even this
> feature is disabled by default.
>
> For these reasons, I think we should remove the Session Pooling  
> support
> from the JCR base bundle.
>
> WDYT ?
>
> Regards
> Felix