Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jetty-users] JDBCSessionManager dirty operations

Stefan, Cemo, David,

I agree that it would be a good idea to rewrite the session management
afresh. There's code in there that has been there since jetty-6, maybe
even longer! This isn't going to happen overnight - as with everything
to do with the servlet spec its a complex beastie that requires some
careful thought, and needs to take account of the many different
flavours of session management: persistent/non-persistent,
clustered/non-clustered, and the various technologies: jdbc, mongo etc
etc etc.

The session management code should mostly hold sync locks on a
specific session, rarely over the entire session manager (which will
affect all session ops for a context) or over the entire session id
manager (which can affect all session ops for a server).  If you would
like to pinpoint areas where we can look at optimising the locking
strategy (patches are always nice! - see how to contribute here:
http://www.eclipse.org/jetty/documentation/current/advanced-contributing.html),
then that could probably be done fairly quickly.

regards
Jan


On 27 February 2014 21:50, Stefan Magnus Landrø <stefan.landro@xxxxxxxxx> wrote:
> Hi there,
>
> the problem here isn't really the synchronized block itself. The problem is
> that the different synchronization blocks cover lots of remote sql calls (@
> let's say 10ms each). Do the math - you'll notice how few session related
> operations you can do per second per jvm ...
> In my opinion, the jdbc-/(abstract-)session(id)manager code needs a complete
> rewrite.
>
> Stefan
>
>
> 2014-02-27 8:48 GMT+01:00 "" <lord.buddha@xxxxxxxxx>:
>
>> Indeed, tomcat uses a concurrent hash map and no explicit synchronisation
>> when accessing/updating session attributes.
>>
>> And in fact tomcat only uses synchronized in two places, when expiring the
>> session and when fire events to session listeners.
>>
>> Yeah, I hear that synchronisation "is not such an overhead", but then put
>> large load on big multi socket multicore box and watch it completely fail to
>> scale to the available hardware.
>>
>>
>> On 27 February 2014 20:35, "" <lord.buddha@xxxxxxxxx> wrote:
>>>
>>>
>>> Ouch.  Just looked at the   AbstractSessionManager...   Such
>>> synchronisation is the tip of the iceberg.
>>>
>>> I wonder if there really needs to be this amount of synchronisation when
>>> the order of the requests coming in that might affect a session can be quote
>>> arbitrary in themselves.   On AbstractSessionManager synchronisation
>>> protection on _attributes is perhaps overkill when a ConcurrentHashMap could
>>> be used instead.
>>>
>>>
>>>
>>>
>>>
>>> On 27 February 2014 20:02, "" <lord.buddha@xxxxxxxxx> wrote:
>>>>
>>>> I don't believe the synchronised block is useful here.   Synchronised
>>>> should be used to stop things breaking, not to protect against something
>>>> that is occasionally inefficient.
>>>>
>>>> So this is sufficient and will avoid inconsistent synchronization
>>>> warnings...
>>>>
>>>>    @Override
>>>>
>>>>
>>>>
>>>>    public void removeAttribute(String name) {
>>>>
>>>>
>>>>
>>>>
>>>>       if(getAttribute(name)){
>>>>          super.removeAttribute(name);
>>>>          _dirty = true;
>>>>       }
>>>>
>>>>
>>>>
>>>>
>>>>    }
>>>>
>>>>
>>>>
>>>>
>>>> On 27 February 2014 10:40, Jan Bartel <janb@xxxxxxxxxxx> wrote:
>>>>>
>>>>> Cemo,
>>>>>
>>>>> This will need some refactoring of the implementation of the session
>>>>> interface, but I'm not opposed to looking into it.
>>>>>
>>>>> Can you open a bug for this please and assign to me?
>>>>>
>>>>> thanks
>>>>> Jan
>>>>>
>>>>> On 27 February 2014 06:25, Cemo <cemalettin.koc@xxxxxxxxx> wrote:
>>>>> > Hi,
>>>>> >
>>>>> > Current org.eclipse.jetty.server.session.JDBCSessionManager has
>>>>> > removeAttribute as this:
>>>>> >
>>>>> >   public void removeAttribute (String name)
>>>>> >
>>>>> >  {
>>>>> >       super.removeAttribute(name);
>>>>> >
>>>>> >
>>>>> >
>>>>> >      _dirty=true;
>>>>> >  }
>>>>> >
>>>>> >
>>>>> > This has a side effect because of making dirty for every removing
>>>>> > operation.
>>>>> > This is causing a lot of trouble because each jstl set ( c:set ) call
>>>>> > in JSP
>>>>> > pages is also calling removeAttribute internally. What I am
>>>>> > suggesting is
>>>>> > that something like this:
>>>>> >
>>>>> >
>>>>> >    @Override
>>>>> >    public void removeAttribute(String name) {
>>>>> >
>>>>> >
>>>>> >
>>>>> >       synchronized (this){
>>>>> >          Object attribute = getAttribute(name);
>>>>> >
>>>>> >
>>>>> >
>>>>> >          if(attribute != null){
>>>>> >
>>>>> >
>>>>> >
>>>>> >             super.removeAttribute(name);
>>>>> >
>>>>> >
>>>>> >
>>>>> >             _dirty = true;
>>>>> >          }
>>>>> >       }
>>>>> >    }
>>>>> >
>>>>> > public
>>>>> >
>>>>> >
>>>>> > https://gist.github.com/cemo/9236abe34d2b126242ad
>>>>> >
>>>>> > What do you think?
>>>>> >
>>>>> > _______________________________________________
>>>>> > jetty-users mailing list
>>>>> > jetty-users@xxxxxxxxxxx
>>>>> > https://dev.eclipse.org/mailman/listinfo/jetty-users
>>>>> >
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Jan Bartel <janb@xxxxxxxxxxx>
>>>>> www.webtide.com
>>>>> 'Expert Jetty/CometD developer,production,operations advice'
>>>>> _______________________________________________
>>>>> jetty-users mailing list
>>>>> jetty-users@xxxxxxxxxxx
>>>>> https://dev.eclipse.org/mailman/listinfo/jetty-users
>>>>
>>>>
>>>
>>
>>
>> _______________________________________________
>> jetty-users mailing list
>> jetty-users@xxxxxxxxxxx
>> https://dev.eclipse.org/mailman/listinfo/jetty-users
>>
>
>
>
> --
> BEKK Open
> http://open.bekk.no
>
> TesTcl - a unit test framework for iRules
> http://testcl.com
>
> _______________________________________________
> jetty-users mailing list
> jetty-users@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/jetty-users
>



-- 
Jan Bartel <janb@xxxxxxxxxxx>
www.webtide.com
'Expert Jetty/CometD developer,production,operations advice'


Back to the top