<font size=2 face="sans-serif">Hello<br>
</font>
<br><font size=2 face="sans-serif">I spoke to Allan and he agrees with
you that it is a grey area in the spec. I'm investigating what can be done
on our end, but since it is a grey area shouldn't weld err on the side
of following Oracle's advice? Or at least include a setting to switch to
following the advice? Especially since it would be technically simple to
do so; as the map is only modified in one class and Allan says it would
be a good performance improvement.<br>
<br>
Regards<br>
Benjamin</font>
<br>
<br>
<br>
<br><font size=1 color=#5f5f5f face="sans-serif">From:
</font><font size=1 face="sans-serif">Matej Novotny <manovotn@redhat.com></font>
<br><font size=1 color=#5f5f5f face="sans-serif">To:
</font><font size=1 face="sans-serif">Benjamin Confino <BENJAMIC@uk.ibm.com></font>
<br><font size=1 color=#5f5f5f face="sans-serif">Cc:
</font><font size=1 face="sans-serif">Allan Zhang <zhang@ca.ibm.com>,
Shinji Ohtsuka <EB92769@jp.ibm.com>, Emily Jiang <EMIJIANG@uk.ibm.com>,
weld-dev@lists.jboss.org</font>
<br><font size=1 color=#5f5f5f face="sans-serif">Date:
</font><font size=1 face="sans-serif">11/05/2020 09:33</font>
<br><font size=1 color=#5f5f5f face="sans-serif">Subject:
</font><font size=1 face="sans-serif">[EXTERNAL] Re:
[weld-dev] Propagation of org.jboss.weld.context.ConversationContext.conversations
through session failover</font>
<br>
<hr noshade>
<br>
<br>
<br><tt><font size=2>The document you linked is not a specification itself
and as such is not binding. Do you have an actual specification bits that
prove this?<br>
<br>
I went looking into servlet specification[1] and asked someone from WFLY
who has more expertise in this field to also take a look.<br>
None of us found anything stating that you are required to call setAttribute()
every time you change mutable attribute.<br>
In fact there is very little about attributes (or "replication triggers"
for attributes) and you could say this is grey area.<br>
<br>
To me it still makes way more sense to distinguish between mutable/immutable
attributes and for mutable ones you should set "dirty" state
even on read, not just write.<br>
<br>
Matej<br>
_____________________________________________________________________________________<br>
[1] </font></tt><a href="https://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_FINAL.pdf"><tt><font size=2>https://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_FINAL.pdf</font></tt></a><tt><font size=2>
<br>
<br>
----- Original Message -----<br>
> From: "Benjamin Confino" <BENJAMIC@uk.ibm.com><br>
> To: "Allan Zhang" <zhang@ca.ibm.com><br>
> Cc: "Matej Novotny" <manovotn@redhat.com>, "Shinji
Ohtsuka" <EB92769@jp.ibm.com>, "Emily Jiang"<br>
> <EMIJIANG@uk.ibm.com>, weld-dev@lists.jboss.org<br>
> Sent: Thursday, May 7, 2020 12:41:01 PM<br>
> Subject: RE: [weld-dev] Propagation of org.jboss.weld.context.ConversationContext.conversations
through session<br>
> failover<br>
> <br>
> Hello<br>
> <br>
> Based on Allan Zhang's comments I have closed my pull requests and
opened<br>
> this one </font></tt><a href="https://github.com/weld/core/pull/1990"><tt><font size=2>https://github.com/weld/core/pull/1990</font></tt></a><tt><font size=2>
which is a better match<br>
> for the recommendations in the sessionmanagement spec. I still have
not<br>
> heard back from my customer so I will compile an updated test fix
and<br>
> chase them up. If they approve, or if they do not reply soon I will
create<br>
> pull requests for the other branches.<br>
> <br>
> Regards<br>
> Benjamin<br>
> <br>
> <br>
> <br>
> From: Allan Zhang/Toronto/IBM<br>
> To: Matej Novotny <manovotn@redhat.com><br>
> Cc: Benjamin Confino <BENJAMIC@uk.ibm.com>, Shinji
Ohtsuka<br>
> <EB92769@jp.ibm.com>, Emily Jiang <EMIJIANG@uk.ibm.com>,<br>
> weld-dev@lists.jboss.org<br>
> Date: 06/05/2020 17:42<br>
> Subject: Re: [EXTERNAL] Re: [weld-dev]
Propagation of<br>
> org.jboss.weld.context.ConversationContext.conversations through session<br>
> failover<br>
> <br>
> <br>
> <br>
> >Shouldn't Liberty consider (at least after restart) any access
to a<br>
> collection-based session property as an action marking that property<br>
> "dirty"?<br>
> <br>
> Liberty already has write option of ALL_SESSION_ATTRIBUTES to mark
the<br>
> property "dirty" but customer wont use it for performance
reasons.<br>
> <br>
> The PR fix is following the JEE spec where property "dirty"
require<br>
> setAttribute() call.<br>
> </font></tt><a href="https://docs.oracle.com/cd/E13924_01/coh.340/e13819/sessionmanagement.htm"><tt><font size=2>https://docs.oracle.com/cd/E13924_01/coh.340/e13819/sessionmanagement.htm</font></tt></a><tt><font size=2>
<br>
> <br>
> As a general rule, all session attributes should be treated as immutable<br>
> objects if possible. This ensures that developers are consciously
aware<br>
> when they change attributes. With mutable objects, modifying attributes<br>
> often requires two steps: modifying the state of the attribute object,
and<br>
> then manually updating the session with the modified attribute object
by<br>
> calling javax.servlet.http.HttpSession.setAttribute(). This means
that<br>
> your application should always call setAttribute() if the attribute
value<br>
> has been changed, otherwise, the modified attribute value will not<br>
> replicate to the backup server.<br>
> <br>
> Thanks<br>
> Allan Zhang<br>
> <br>
> <br>
> <br>
> <br>
> From: Matej Novotny <manovotn@redhat.com><br>
> To: Benjamin Confino <BENJAMIC@uk.ibm.com><br>
> Cc: Shinji Ohtsuka <EB92769@jp.ibm.com>, Emily
Jiang<br>
> <EMIJIANG@uk.ibm.com>, weld-dev@lists.jboss.org, Allan Zhang<br>
> <zhang@ca.ibm.com><br>
> Date: 2020-05-06 07:52 AM<br>
> Subject: [EXTERNAL] Re: [weld-dev] Propagation
of<br>
> org.jboss.weld.context.ConversationContext.conversations through session<br>
> failover<br>
> <br>
> <br>
> <br>
> Hi,<br>
> <br>
> thanks for explanation, I understand the flow now.<br>
> <br>
> However, your suggested PR still looks like a workaround more than
a fix<br>
> to me.<br>
> <br>
> I doubt Weld is the only case where this will emerge. You can easily
store<br>
> any collection in session and expect it to behave like we do.<br>
> So in the very least, anything of type java.util.Collection that is
stored<br>
> in session is susceptible to this behaviour.<br>
> Shouldn't Liberty consider (at least after restart) any access to
a<br>
> collection-based session property as an action marking that property<br>
> "dirty"?<br>
> <br>
> Regards<br>
> Matej<br>
> <br>
> ----- Original Message -----<br>
> > From: "Benjamin Confino" <BENJAMIC@uk.ibm.com><br>
> > To: "Matej Novotny" <manovotn@redhat.com><br>
> > Cc: "Shinji Ohtsuka" <EB92769@jp.ibm.com>, "Emily
Jiang"<br>
> <EMIJIANG@uk.ibm.com>, weld-dev@lists.jboss.org, "Allan<br>
> > Zhang" <zhang@ca.ibm.com><br>
> > Sent: Tuesday, May 5, 2020 3:59:30 PM<br>
> > Subject: RE: [weld-dev] Propagation of<br>
> org.jboss.weld.context.ConversationContext.conversations through session<br>
> > failover<br>
> > <br>
> > Hello Matej<br>
> > <br>
> > No worries about the delay, my customer has not responded so
you are<br>
> ahead<br>
> > of schedule. You are correct, the purpose of this PR is to trigger
the<br>
> > "dirty" state in liberty's session storage. Let me
try and describe the<br>
> > flow again and hopefully it will be clearer:<br>
> > <br>
> > <br>
> > A1) The application is started for the first time. A conversation
map is<br>
> > created and stored into the session context. This marks it as
dirty.<br>
> > A2) The application puts stuff into the map. Lets say this leaves
the<br>
> map<br>
> > in State A<br>
> > A3) The server is told to shut down.<br>
> > A4) Liberty sees the map is dirty and saves map State A to persistent<br>
> > storage.<br>
> > <br>
> > B1) The server starts again.<br>
> > B2) Weld retrieves the conversation map (State A) from persistent<br>
> storage.<br>
> > Since it already has a map it does not put anything into session<br>
> storage.<br>
> > Thus the map is clean.<br>
> > B3) The application does more stuff, moving the map to (State
B).<br>
> > B3) The server is told to shut down.<br>
> > B4) Liberty sees the map is clean, it doesn't transfer anything
to<br>
> > persistent storage.<br>
> > <br>
> > C1) The server starts again.<br>
> > C2) Weld retrieves the conversation map (State A). This is the
wrong<br>
> > state.<br>
> > <br>
> > <br>
> > My PR changes B2 so that Weld will set the same map back into
session<br>
> > storage. Then in B4 liberty will see the map is dirty and save
State B<br>
> to<br>
> > storage, ensuring the correct state is retrieved at C2.<br>
> > <br>
> > We do have alternative solutions, for example you can configure
Liberty<br>
> to<br>
> > store everything regardless of whether it is dirty or not. However
this<br>
> > has performance implications and so my customer does not want
to use<br>
> that<br>
> > option.<br>
> > <br>
> > Regards<br>
> > Benjamin<br>
> > <br>
> > <br>
> > <br>
> > <br>
> > From: Matej Novotny <manovotn@redhat.com><br>
> > To: Benjamin Confino <BENJAMIC@uk.ibm.com><br>
> > Cc: weld-dev@lists.jboss.org, Allan Zhang <zhang@ca.ibm.com>,
Shinji<br>
> > Ohtsuka <EB92769@jp.ibm.com>, Emily Jiang <EMIJIANG@uk.ibm.com><br>
> > Date: 05/05/2020 12:17<br>
> > Subject: [EXTERNAL] Re: [weld-dev]
Propagation of<br>
> > org.jboss.weld.context.ConversationContext.conversations through
session<br>
> > failover<br>
> > <br>
> > <br>
> > <br>
> > I finally found some time to look into this, it took longer than
I<br>
> > anticipated, sorry for that.<br>
> > <br>
> > Firstly, I am not quite following the flow of the reproducer
you<br>
> > described.<br>
> > Weld creates the conversation map and stores in into the session
on<br>
> > creation. We then retrieve this map from session whenever needed
and<br>
> store<br>
> > additional things into it.<br>
> > All the while we use the same map (the same reference, or object
if you<br>
> > will)....so Liberty should see the same state in it, right? Or
am I<br>
> > missing something obvious?<br>
> > <br>
> > I am not familiar with how Liberty deals with server shutdown
and how it<br>
> > stores session attributes, but i would expect that if you just
grab the<br>
> > existing map from there, it will be up to date.<br>
> > The PRs you sent seem superfluous to me - you are overriding
the<br>
> attribute<br>
> > with exactly the same thing. probably just to trigger "dirty"
state in<br>
> > your session storage?<br>
> > <br>
> > Regards<br>
> > Matej<br>
> > <br>
> > ----- Original Message -----<br>
> > > From: "Benjamin Confino" <BENJAMIC@uk.ibm.com><br>
> > > To: "Matej Novotny" <manovotn@redhat.com><br>
> > > Cc: weld-dev@lists.jboss.org, "Allan Zhang" <zhang@ca.ibm.com>,<br>
> "Shinji<br>
> > Ohtsuka" <EB92769@jp.ibm.com>, "Emily Jiang"<br>
> > > <EMIJIANG@uk.ibm.com><br>
> > > Sent: Wednesday, April 29, 2020 11:56:43 AM<br>
> > > Subject: RE: [weld-dev] Propagation of<br>
> > org.jboss.weld.context.ConversationContext.conversations through
session<br>
> > > failover<br>
> > > <br>
> > > Thank you for the heads up. When it's time to think about
delivery<br>
> I'll<br>
> > be<br>
> > > sure to create a PR against 3.1 and I presume you'd like
me to leave<br>
> > > master alone for now?<br>
> > > <br>
> > > Regards<br>
> > > Benjamin<br>
> > > <br>
> > > <br>
> > > <br>
> > > From: Matej Novotny <manovotn@redhat.com><br>
> > > To: Benjamin Confino <BENJAMIC@uk.ibm.com><br>
> > > Cc: weld-dev@lists.jboss.org, Allan Zhang
<zhang@ca.ibm.com>,<br>
> Shinji<br>
> > > Ohtsuka <EB92769@jp.ibm.com>, Emily Jiang <EMIJIANG@uk.ibm.com><br>
> > > Date: 29/04/2020 09:51<br>
> > > Subject: [EXTERNAL] Re: [weld-dev]
Propagation of<br>
> > > org.jboss.weld.context.ConversationContext.conversations
through<br>
> session<br>
> > > failover<br>
> > > <br>
> > > <br>
> > > <br>
> > > Hi,<br>
> > > <br>
> > > I'll take a look later today.<br>
> > > Note that master branch is no longer Weld 3.x, it is 4.x
(Jakarta EE<br>
> 9)<br>
> > > and the CI there is going bonkers yet as I am in the middle
of<br>
> changing<br>
> > > it.<br>
> > > If you want to file a PR against Weld 3, you can use 3.1
branch for<br>
> > that.<br>
> > > <br>
> > > Regards<br>
> > > Matej<br>
> > > <br>
> > > ----- Original Message -----<br>
> > > > From: "Benjamin Confino" <BENJAMIC@uk.ibm.com><br>
> > > > To: weld-dev@lists.jboss.org<br>
> > > > Cc: "Allan Zhang" <zhang@ca.ibm.com>,
"Shinji Ohtsuka"<br>
> > > <EB92769@jp.ibm.com>, "Emily Jiang" <EMIJIANG@uk.ibm.com><br>
> > > > Sent: Tuesday, April 28, 2020 2:31:44 PM<br>
> > > > Subject: [weld-dev] Propagation of<br>
> > > org.jboss.weld.context.ConversationContext.conversations
through<br>
> session<br>
> > > failover<br>
> > > > <br>
> > > > Hello weld<br>
> > > > <br>
> > > > I had a customer report that they were getting conversation
not<br>
> found<br>
> > > > exceptions when restarting their server and visiting
a url with a<br>
> > ?cid=1<br>
> > > > suffix.<br>
> > > > <br>
> > > > After investigation I believe the issue is that weld
was acquiring<br>
> > it's<br>
> > > > ConversationContext.conversations from the session
database via<br>
> > > > com.ibm.ws.session.store.db.DatabaseSession.getMultiRowAppData().<br>
> Once<br>
> > > weld<br>
> > > > had retrieved the conversations map it would then decide
that since<br>
> > the<br>
> > > map<br>
> > > > was already in the session attributes there was no
need to put it<br>
> back<br>
> > > into<br>
> > > > the attributes.<br>
> > > > <br>
> > > > This means that Liberty did not realise the conversations
map had<br>
> been<br>
> > > > updated, and did not store it's updated state into
the database when<br>
> > the<br>
> > > > server shut down again.<br>
> > > > <br>
> > > > I have submitted a pair of pull requests that asks
weld to mark the<br>
> > > > conversation map as dirty upon access - this behaviour
is gated<br>
> behind<br>
> > > > ConfigurationKey.RESET_HTTP_SESSION_ATTR_ON_BEAN_ACCESS
- I have<br>
> > tested<br>
> > > it<br>
> > > > locally and it works. The next step is to prepare a
test fix for the<br>
> > > > customer to verify. However I wanted to send you this
quick note to<br>
> > keep<br>
> > > you<br>
> > > > in the loop.<br>
> > > > <br>
> > > > Regards<br>
> > > > Benjamin<br>
> > > > Unless stated otherwise above:<br>
> > > > IBM United Kingdom Limited - Registered in England
and Wales with<br>
> > number<br>
> > > > 741598.<br>
> > > > Registered office: PO Box 41, North Harbour, Portsmouth,
Hampshire<br>
> PO6<br>
> > > 3AU<br>
> > > > <br>
> > > > _______________________________________________<br>
> > > > weld-dev mailing list<br>
> > > > weld-dev@lists.jboss.org<br>
> > > > <br>
> > > <br>
> > <br>
> </font></tt><a href="https://lists.jboss.org/mailman/listinfo/weld-dev"><tt><font size=2>https://lists.jboss.org/mailman/listinfo/weld-dev</font></tt></a><tt><font size=2><br>
> <br>
> > <br>
> > > <br>
> > > <br>
> > > <br>
> > > <br>
> > > <br>
> > > Unless stated otherwise above:<br>
> > > IBM United Kingdom Limited - Registered in England and Wales
with<br>
> number<br>
> > > 741598.<br>
> > > Registered office: PO Box 41, North Harbour, Portsmouth,
Hampshire PO6<br>
> > 3AU<br>
> > > <br>
> > <br>
> > <br>
> > <br>
> > <br>
> > Unless stated otherwise above:<br>
> > IBM United Kingdom Limited - Registered in England and Wales
with number<br>
> > 741598.<br>
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
PO6<br>
> 3AU<br>
> > <br>
> <br>
> <br>
> <br>
> <br>
> <br>
> Unless stated otherwise above:<br>
> IBM United Kingdom Limited - Registered in England and Wales with
number<br>
> 741598.<br>
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
PO6 3AU<br>
> <br>
<br>
</font></tt>
<br>
<br><font size=2 face="sans-serif"><br>
Unless stated otherwise above:<br>
IBM United Kingdom Limited - Registered in England and Wales with number
741598. <br>
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
3AU<br>
</font>