[weld-dev] Propagation of org.jboss.weld.context.ConversationContext.conversations through session failover

Benjamin Confino BENJAMIC at uk.ibm.com
Thu May 7 06:41:01 EDT 2020


Hello

Based on Allan Zhang's comments I have closed my pull requests and opened 
this one https://github.com/weld/core/pull/1990 which is a better match 
for the recommendations in the sessionmanagement spec. I still have not 
heard back from my customer so I will compile an updated test fix and 
chase them up. If they approve, or if they do not reply soon I will create 
pull requests for the other branches. 

Regards
Benjamin 



From:   Allan Zhang/Toronto/IBM
To:     Matej Novotny <manovotn at redhat.com>
Cc:     Benjamin Confino <BENJAMIC at uk.ibm.com>, Shinji Ohtsuka 
<EB92769 at jp.ibm.com>, Emily Jiang <EMIJIANG at uk.ibm.com>, 
weld-dev at lists.jboss.org
Date:   06/05/2020 17:42
Subject:        Re: [EXTERNAL] Re: [weld-dev] Propagation of 
org.jboss.weld.context.ConversationContext.conversations through session 
failover



>Shouldn't Liberty consider (at least after restart) any access to a 
collection-based session property as an action marking that property 
"dirty"?

Liberty already has write option of ALL_SESSION_ATTRIBUTES to mark the 
property "dirty" but customer wont use it for performance reasons.

The PR fix is following the JEE spec where property "dirty" require 
setAttribute() call. 
https://docs.oracle.com/cd/E13924_01/coh.340/e13819/sessionmanagement.htm

As a general rule, all session attributes should be treated as immutable 
objects if possible. This ensures that developers are consciously aware 
when they change attributes. With mutable objects, modifying attributes 
often requires two steps: modifying the state of the attribute object, and 
then manually updating the session with the modified attribute object by 
calling javax.servlet.http.HttpSession.setAttribute(). This means that 
your application should always call setAttribute() if the attribute value 
has been changed, otherwise, the modified attribute value will not 
replicate to the backup server.

Thanks
Allan Zhang




From:   Matej Novotny <manovotn at redhat.com>
To:     Benjamin Confino <BENJAMIC at uk.ibm.com>
Cc:     Shinji Ohtsuka <EB92769 at jp.ibm.com>, Emily Jiang 
<EMIJIANG at uk.ibm.com>, weld-dev at lists.jboss.org, Allan Zhang 
<zhang at ca.ibm.com>
Date:   2020-05-06 07:52 AM
Subject:        [EXTERNAL] Re: [weld-dev] Propagation of 
org.jboss.weld.context.ConversationContext.conversations through session 
failover



Hi,

thanks for explanation, I understand the flow now.

However, your suggested PR still looks like a workaround more than a fix 
to me.

I doubt Weld is the only case where this will emerge. You can easily store 
any collection in session and expect it to behave like we do.
So in the very least, anything of type java.util.Collection that is stored 
in session is susceptible to this behaviour.
Shouldn't Liberty consider (at least after restart) any access to a 
collection-based session property as an action marking that property 
"dirty"?

Regards
Matej

----- Original Message -----
> From: "Benjamin Confino" <BENJAMIC at uk.ibm.com>
> To: "Matej Novotny" <manovotn at redhat.com>
> Cc: "Shinji Ohtsuka" <EB92769 at jp.ibm.com>, "Emily Jiang" 
<EMIJIANG at uk.ibm.com>, weld-dev at lists.jboss.org, "Allan
> Zhang" <zhang at ca.ibm.com>
> Sent: Tuesday, May 5, 2020 3:59:30 PM
> Subject: RE: [weld-dev] Propagation of 
org.jboss.weld.context.ConversationContext.conversations through session
> failover
> 
> Hello Matej
> 
> No worries about the delay, my customer has not responded so you are 
ahead
> of schedule. You are correct, the purpose of this PR is to trigger the
> "dirty" state in liberty's session storage. Let me try and describe the
> flow again and hopefully it will be clearer:
> 
> 
> A1) The application is started for the first time. A conversation map is
> created and stored into the session context. This marks it as dirty.
> A2) The application puts stuff into the map. Lets say this leaves the 
map
> in State A
> A3) The server is told to shut down.
> A4) Liberty sees the map is dirty and saves map State A to persistent
> storage.
> 
> B1) The server starts again.
> B2) Weld retrieves the conversation map (State A) from persistent 
storage.
> Since it already has a map it does not put anything into session 
storage.
> Thus the map is clean.
> B3) The application does more stuff, moving the map to (State B).
> B3) The server is told to shut down.
> B4) Liberty sees the map is clean, it doesn't transfer anything to
> persistent storage.
> 
> C1) The server starts again.
> C2) Weld retrieves the conversation map (State A). This is the wrong
> state.
> 
> 
> My PR changes B2 so that Weld will set the same map back into session
> storage. Then in B4 liberty will see the map is dirty and save State B 
to
> storage, ensuring the correct state is retrieved at C2.
> 
> We do have alternative solutions, for example you can configure Liberty 
to
> store everything regardless of whether it is dirty or not. However this
> has performance implications and so my customer does not want to use 
that
> option.
> 
> Regards
> Benjamin
> 
> 
> 
> 
> From:   Matej Novotny <manovotn at redhat.com>
> To:     Benjamin Confino <BENJAMIC at uk.ibm.com>
> Cc:     weld-dev at lists.jboss.org, Allan Zhang <zhang at ca.ibm.com>, Shinji
> Ohtsuka <EB92769 at jp.ibm.com>, Emily Jiang <EMIJIANG at uk.ibm.com>
> Date:   05/05/2020 12:17
> Subject:        [EXTERNAL] Re: [weld-dev] Propagation of
> org.jboss.weld.context.ConversationContext.conversations through session
> failover
> 
> 
> 
> I finally found some time to look into this, it took longer than I
> anticipated, sorry for that.
> 
> Firstly, I am not quite following the flow of the reproducer you
> described.
> Weld creates the conversation map and stores in into the session on
> creation. We then retrieve this map from session whenever needed and 
store
> additional things into it.
> All the while we use the same map (the same reference, or object if you
> will)....so Liberty should see the same state in it, right? Or am I
> missing something obvious?
> 
> I am not familiar with how Liberty deals with server shutdown and how it
> stores session attributes, but i would expect that if you just grab the
> existing map from there, it will be up to date.
> The PRs you sent seem superfluous to me - you are overriding the 
attribute
> with exactly the same thing. probably just to trigger "dirty" state in
> your session storage?
> 
> Regards
> Matej
> 
> ----- Original Message -----
> > From: "Benjamin Confino" <BENJAMIC at uk.ibm.com>
> > To: "Matej Novotny" <manovotn at redhat.com>
> > Cc: weld-dev at lists.jboss.org, "Allan Zhang" <zhang at ca.ibm.com>, 
"Shinji
> Ohtsuka" <EB92769 at jp.ibm.com>, "Emily Jiang"
> > <EMIJIANG at uk.ibm.com>
> > Sent: Wednesday, April 29, 2020 11:56:43 AM
> > Subject: RE: [weld-dev] Propagation of
> org.jboss.weld.context.ConversationContext.conversations through session
> > failover
> > 
> > Thank you for the heads up. When it's time to think about delivery 
I'll
> be
> > sure to create a PR against 3.1 and I presume you'd like me to leave
> > master alone for now?
> > 
> > Regards
> > Benjamin
> > 
> > 
> > 
> > From:   Matej Novotny <manovotn at redhat.com>
> > To:     Benjamin Confino <BENJAMIC at uk.ibm.com>
> > Cc:     weld-dev at lists.jboss.org, Allan Zhang <zhang at ca.ibm.com>, 
Shinji
> > Ohtsuka <EB92769 at jp.ibm.com>, Emily Jiang <EMIJIANG at uk.ibm.com>
> > Date:   29/04/2020 09:51
> > Subject:        [EXTERNAL] Re: [weld-dev] Propagation of
> > org.jboss.weld.context.ConversationContext.conversations through 
session
> > failover
> > 
> > 
> > 
> > Hi,
> > 
> > I'll take a look later today.
> > Note that master branch is no longer Weld 3.x, it is 4.x (Jakarta EE 
9)
> > and the CI there is going bonkers yet as I am in the middle of 
changing
> > it.
> > If you want to file a PR against Weld 3, you can use 3.1 branch for
> that.
> > 
> > Regards
> > Matej
> > 
> > ----- Original Message -----
> > > From: "Benjamin Confino" <BENJAMIC at uk.ibm.com>
> > > To: weld-dev at lists.jboss.org
> > > Cc: "Allan Zhang" <zhang at ca.ibm.com>, "Shinji Ohtsuka"
> > <EB92769 at jp.ibm.com>, "Emily Jiang" <EMIJIANG at uk.ibm.com>
> > > Sent: Tuesday, April 28, 2020 2:31:44 PM
> > > Subject: [weld-dev] Propagation of
> > org.jboss.weld.context.ConversationContext.conversations through 
session
> > failover
> > > 
> > > Hello weld
> > > 
> > > I had a customer report that they were getting conversation not 
found
> > > exceptions when restarting their server and visiting a url with a
> ?cid=1
> > > suffix.
> > > 
> > > After investigation I believe the issue is that weld was acquiring
> it's
> > > ConversationContext.conversations from the session database via
> > > com.ibm.ws.session.store.db.DatabaseSession.getMultiRowAppData(). 
Once
> > weld
> > > had retrieved the conversations map it would then decide that since
> the
> > map
> > > was already in the session attributes there was no need to put it 
back
> > into
> > > the attributes.
> > > 
> > > This means that Liberty did not realise the conversations map had 
been
> > > updated, and did not store it's updated state into the database when
> the
> > > server shut down again.
> > > 
> > > I have submitted a pair of pull requests that asks weld to mark the
> > > conversation map as dirty upon access - this behaviour is gated 
behind
> > > ConfigurationKey.RESET_HTTP_SESSION_ATTR_ON_BEAN_ACCESS - I have
> tested
> > it
> > > locally and it works. The next step is to prepare a test fix for the
> > > customer to verify. However I wanted to send you this quick note to
> keep
> > you
> > > in the loop.
> > > 
> > > Regards
> > > Benjamin
> > > Unless stated otherwise above:
> > > IBM United Kingdom Limited - Registered in England and Wales with
> number
> > > 741598.
> > > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire 
PO6
> > 3AU
> > > 
> > > _______________________________________________
> > > weld-dev mailing list
> > > weld-dev at lists.jboss.org
> > > 
> > 
> 
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.jboss.org_mailman_listinfo_weld-2Ddev&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=hl8XiFr1UrTSgyXVc4KO_i51sM3Gyhlu1KZ4TkyseEQ&m=WVR6Rz8X_vITi7c26XiQc9JlPkPasM6Px9gWE5r-pTg&s=qyzWuJ4v2pIKB7CZ39-u649jw6ouFAc8zecTq1XY1-k&e=

> 
> > 
> > 
> > 
> > 
> > 
> > Unless stated otherwise above:
> > IBM United Kingdom Limited - Registered in England and Wales with 
number
> > 741598.
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
> 3AU
> > 
> 
> 
> 
> 
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU
> 





Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/weld-dev/attachments/20200507/8fba33d2/attachment.html 


More information about the weld-dev mailing list