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
>> 
>> 
> 

Reply via email to