Re: [weld-dev] Propagation of org.jboss.weld.context.ConversationContext.conversations through session failover
6:41 a.m.
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(a)redhat.com>
Cc: Benjamin Confino <BENJAMIC(a)uk.ibm.com>, Shinji Ohtsuka
<EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)uk.ibm.com>,
weld-dev(a)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
> Unless stated otherwise
above:
> IBM United Kingdom Limited - Registered in England and Wales with number
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
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(a)redhat.com>
To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
Cc: Shinji Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, Allan Zhang
<zhang(a)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(a)uk.ibm.com>
To: "Matej Novotny" <manovotn(a)redhat.com>
Cc: "Shinji Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, "Allan
Zhang" <zhang(a)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(a)redhat.com>
To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
Cc: weld-dev(a)lists.jboss.org, Allan Zhang <zhang(a)ca.ibm.com>, Shinji
Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)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(a)uk.ibm.com>
> To: "Matej Novotny" <manovotn(a)redhat.com>
> Cc: weld-dev(a)lists.jboss.org, "Allan Zhang" <zhang(a)ca.ibm.com>,
"Shinji
Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
> <EMIJIANG(a)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(a)redhat.com>
> To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> Cc: weld-dev(a)lists.jboss.org, Allan Zhang <zhang(a)ca.ibm.com>,
Shinji
> Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)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(a)uk.ibm.com>
> > To: weld-dev(a)lists.jboss.org
> > Cc: "Allan Zhang" <zhang(a)ca.ibm.com>, "Shinji
Ohtsuka"
> <EB92769(a)jp.ibm.com>, "Emily Jiang" <EMIJIANG(a)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(a)lists.jboss.org
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.jboss.org_mail...
> 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
4:33 a.m.
New subject: Propagation of org.jboss.weld.context.ConversationContext.conversations through session failover
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?
I went looking into servlet specification[1] and asked someone from WFLY who has more
expertise in this field to also take a look.
None of us found anything stating that you are required to call setAttribute() every time
you change mutable attribute.
In fact there is very little about attributes (or "replication triggers" for
attributes) and you could say this is grey area.
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.
Matej
_____________________________________________________________________________________
[1] https://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_F...
----- Original Message -----
From: "Benjamin Confino" <BENJAMIC(a)uk.ibm.com>
To: "Allan Zhang" <zhang(a)ca.ibm.com>
Cc: "Matej Novotny" <manovotn(a)redhat.com>, "Shinji Ohtsuka"
<EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org
Sent: Thursday, May 7, 2020 12:41:01 PM
Subject: RE: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
failover
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(a)redhat.com>
Cc: Benjamin Confino <BENJAMIC(a)uk.ibm.com>, Shinji Ohtsuka
<EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)uk.ibm.com>,
weld-dev(a)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(a)redhat.com>
To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
Cc: Shinji Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, Allan Zhang
<zhang(a)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(a)uk.ibm.com>
> To: "Matej Novotny" <manovotn(a)redhat.com>
> Cc: "Shinji Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, "Allan
> Zhang" <zhang(a)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(a)redhat.com>
> To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> Cc: weld-dev(a)lists.jboss.org, Allan Zhang <zhang(a)ca.ibm.com>, Shinji
> Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)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(a)uk.ibm.com>
> > To: "Matej Novotny" <manovotn(a)redhat.com>
> > Cc: weld-dev(a)lists.jboss.org, "Allan Zhang"
<zhang(a)ca.ibm.com>,
"Shinji
> Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
> > <EMIJIANG(a)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(a)redhat.com>
> > To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> > Cc: weld-dev(a)lists.jboss.org, Allan Zhang <zhang(a)ca.ibm.com>,
Shinji
> > Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)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(a)uk.ibm.com>
> > > To: weld-dev(a)lists.jboss.org
> > > Cc: "Allan Zhang" <zhang(a)ca.ibm.com>, "Shinji
Ohtsuka"
> > <EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)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(a)lists.jboss.org
> > >
> >
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.jboss.org_mail...
>
> >
> >
> >
> >
> >
> > 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
5:02 p.m.
New subject: Propagation of org.jboss.weld.context.ConversationContext.conversations through session failover
Hello
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.
Regards
Benjamin
From: Matej Novotny <manovotn(a)redhat.com>
To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
Cc: Allan Zhang <zhang(a)ca.ibm.com>, Shinji Ohtsuka
<EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)uk.ibm.com>,
weld-dev(a)lists.jboss.org
Date: 11/05/2020 09:33
Subject: [EXTERNAL] Re: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
failover
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?
I went looking into servlet specification[1] and asked someone from WFLY
who has more expertise in this field to also take a look.
None of us found anything stating that you are required to call
setAttribute() every time you change mutable attribute.
In fact there is very little about attributes (or "replication triggers"
for attributes) and you could say this is grey area.
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.
Matej
_____________________________________________________________________________________
[1]
https://urldefense.proofpoint.com/v2/url?u=https-3A__javaee.github.io_ser...
----- Original Message -----
> > >
> > >
> 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
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
From: "Benjamin Confino" <BENJAMIC(a)uk.ibm.com>
To: "Allan Zhang" <zhang(a)ca.ibm.com>
Cc: "Matej Novotny" <manovotn(a)redhat.com>, "Shinji Ohtsuka"
<EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org
Sent: Thursday, May 7, 2020 12:41:01 PM
Subject: RE: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
failover
Hello
Based on Allan Zhang's comments I have closed my pull requests and
opened
this one
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_weld_core...
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(a)redhat.com>
Cc: Benjamin Confino <BENJAMIC(a)uk.ibm.com>, Shinji Ohtsuka
<EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)uk.ibm.com>,
weld-dev(a)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://urldefense.proofpoint.com/v2/url?u=https-3A__docs.oracle.com_cd_E...
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(a)redhat.com>
To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
Cc: Shinji Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, Allan Zhang
<zhang(a)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(a)uk.ibm.com>
> To: "Matej Novotny" <manovotn(a)redhat.com>
> Cc: "Shinji Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, "Allan
> Zhang" <zhang(a)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(a)redhat.com>
> To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> Cc: weld-dev(a)lists.jboss.org, Allan Zhang <zhang(a)ca.ibm.com>,
Shinji
> Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)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(a)uk.ibm.com>
> > To: "Matej Novotny" <manovotn(a)redhat.com>
> > Cc: weld-dev(a)lists.jboss.org, "Allan Zhang"
<zhang(a)ca.ibm.com>,
"Shinji
> Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
> > <EMIJIANG(a)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(a)redhat.com>
> > To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> > Cc: weld-dev(a)lists.jboss.org, Allan Zhang <zhang(a)ca.ibm.com>,
Shinji
> > Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)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(a)uk.ibm.com>
> > > To: weld-dev(a)lists.jboss.org
> > > Cc: "Allan Zhang" <zhang(a)ca.ibm.com>, "Shinji
Ohtsuka"
> > <EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)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(a)lists.jboss.org
> > >
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.jboss.org_mail...
> 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
9:10 a.m.
New subject: Propagation of org.jboss.weld.context.ConversationContext.conversations through session failover
@Allan
I read part 7.7.2 of the spec and I see no indications of such requirement there.
FYI, you are probably not subscribed to weld-dev list and your emails are not getting
through (but I am getting them as I am in the CC) -
https://lists.jboss.org/pipermail/weld-dev/2020-May/date.html
@Benjamin
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?
As for Weld, the conversation map is not the only thing we store in session.
For instance, whole session context is stored there (every single bean). Even conversation
context might be stored there and maybe something else but from the top of my head, these
are quite prominent.
And we also do not call setAttribute() every single time a bean changes or is accessed.
That would be massive perf overhead.
What I mean is that the issue you are seeing is a tip of an iceberg - it looks easy to fix
for this one scenario/case, but it wouldn't be a complete change as multiple other
things function the same way and it would require bigger code changes that I don't
think are worth it.
What you are asking for is, simply said, an unsupported replication mode.
Unless you can change the way Liberty treats mutable attributes in session, your customer
should probably swap to a mode where all attributes are stored/replicated on shutdown.
Matej
----- Original Message -----
From: "Benjamin Confino" <BENJAMIC(a)uk.ibm.com>
To: "Matej Novotny" <manovotn(a)redhat.com>
Cc: "Shinji Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, "Allan
Zhang" <zhang(a)ca.ibm.com>
Sent: Monday, May 11, 2020 11:02:04 PM
Subject: RE: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
failover
Hello
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.
Regards
Benjamin
From: Matej Novotny <manovotn(a)redhat.com>
To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
Cc: Allan Zhang <zhang(a)ca.ibm.com>, Shinji Ohtsuka
<EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)uk.ibm.com>,
weld-dev(a)lists.jboss.org
Date: 11/05/2020 09:33
Subject: [EXTERNAL] Re: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
failover
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?
I went looking into servlet specification[1] and asked someone from WFLY
who has more expertise in this field to also take a look.
None of us found anything stating that you are required to call
setAttribute() every time you change mutable attribute.
In fact there is very little about attributes (or "replication triggers"
for attributes) and you could say this is grey area.
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.
Matej
_____________________________________________________________________________________
[1]
https://urldefense.proofpoint.com/v2/url?u=https-3A__javaee.github.io_ser...
----- Original Message -----
> From: "Benjamin Confino" <BENJAMIC(a)uk.ibm.com>
> To: "Allan Zhang" <zhang(a)ca.ibm.com>
> Cc: "Matej Novotny" <manovotn(a)redhat.com>, "Shinji
Ohtsuka"
<EB92769(a)jp.ibm.com>, "Emily Jiang"
> <EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org
> Sent: Thursday, May 7, 2020 12:41:01 PM
> Subject: RE: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
> failover
>
> Hello
>
> Based on Allan Zhang's comments I have closed my pull requests and
opened
> this one
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_weld_core...
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(a)redhat.com>
> Cc: Benjamin Confino <BENJAMIC(a)uk.ibm.com>, Shinji Ohtsuka
> <EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)uk.ibm.com>,
> weld-dev(a)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://urldefense.proofpoint.com/v2/url?u=https-3A__docs.oracle.com_cd_E...
>
> 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(a)redhat.com>
> To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> Cc: Shinji Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
> <EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, Allan Zhang
> <zhang(a)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(a)uk.ibm.com>
> > To: "Matej Novotny" <manovotn(a)redhat.com>
> > Cc: "Shinji Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily
Jiang"
> <EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, "Allan
> > Zhang" <zhang(a)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(a)redhat.com>
> > To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> > Cc: weld-dev(a)lists.jboss.org, Allan Zhang <zhang(a)ca.ibm.com>,
Shinji
> > Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)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(a)uk.ibm.com>
> > > To: "Matej Novotny" <manovotn(a)redhat.com>
> > > Cc: weld-dev(a)lists.jboss.org, "Allan Zhang"
<zhang(a)ca.ibm.com>,
> "Shinji
> > Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
> > > <EMIJIANG(a)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(a)redhat.com>
> > > To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> > > Cc: weld-dev(a)lists.jboss.org, Allan Zhang <zhang(a)ca.ibm.com>,
> Shinji
> > > Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)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(a)uk.ibm.com>
> > > > To: weld-dev(a)lists.jboss.org
> > > > Cc: "Allan Zhang" <zhang(a)ca.ibm.com>, "Shinji
Ohtsuka"
> > > <EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)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(a)lists.jboss.org
> > > >
> > >
> >
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.jboss.org_mail...
>
> >
> > >
> > >
> > >
> > >
> > >
> > > 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
>
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
12:26 p.m.
New subject: Propagation of org.jboss.weld.context.ConversationContext.conversations through session failover
I thought this was an old bug that resolved already
https://issues.redhat.com/browse/WELD-1130
Similarly It was documented under know issue of JBPAPP6-1326 - CDI beans
with SET replication trigger are not replicating
https://access.redhat.com/documentation/en-us/jboss_enterprise_applicatio...
@Benjamin Can the customer use SET_AND_NON_PRIMITIVE_GET setting?
Thanks,
Allan Zhang
From: Matej Novotny <manovotn(a)redhat.com
To: Benjamin Confino <BENJAMIC(a)uk.ibm.com
Cc: Shinji Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, Allan Zhang
<zhang(a)ca.ibm.com Date: 2020-05-12 09:10 AM
Subject: [EXTERNAL] Re: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations
through session failover
@Allan
I read part 7.7.2 of the spec and I see no indications of such requirement
there.
FYI, you are probably not subscribed to weld-dev list and your emails are
not getting through (but I am getting them as I am in the CC) -
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.jboss.org_pipe...
@Benjamin
> As a general rule, all session attributes should be
treated as immutable
> > > > > >
> > > >
> > > > 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
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?
As for Weld, the conversation map is not the only thing we store in
session.
For instance, whole session context is stored there (every single bean).
Even conversation context might be stored there and maybe something else
but from the top of my head, these are quite prominent.
And we also do not call setAttribute() every single time a bean changes or
is accessed. That would be massive perf overhead.
What I mean is that the issue you are seeing is a tip of an iceberg - it
looks easy to fix for this one scenario/case, but it wouldn't be a complete
change as multiple other things function the same way and it would require
bigger code changes that I don't think are worth it.
What you are asking for is, simply said, an unsupported replication mode.
Unless you can change the way Liberty treats mutable attributes in session,
your customer should probably swap to a mode where all attributes are
stored/replicated on shutdown.
Matej
----- Original Message -----
From: "Benjamin Confino" <BENJAMIC(a)uk.ibm.com To: "Matej Novotny" <manovotn(a)redhat.com Cc: "Shinji Ohtsuka" <EB92769(a)jp.ibm.com>,
"Emily Jiang"
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org,
"Allan
Zhang" <zhang(a)ca.ibm.com Sent:
Monday, May 11, 2020 11:02:04 PM
Subject: RE: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
failover
Hello
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.
Regards
Benjamin
From: Matej Novotny <manovotn(a)redhat.com To:
Benjamin Confino <BENJAMIC(a)uk.ibm.com Cc:
Allan Zhang <zhang(a)ca.ibm.com>, Shinji Ohtsuka
<EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)uk.ibm.com>,
weld-dev(a)lists.jboss.org
Date: 11/05/2020 09:33
Subject: [EXTERNAL] Re: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
failover
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?
I went looking into servlet specification[1] and asked someone from WFLY
who has more expertise in this field to also take a look.
None of us found anything stating that you are required to call
setAttribute() every time you change mutable attribute.
In fact there is very little about attributes (or "replication triggers"
for attributes) and you could say this is grey area.
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.
Matej
_____________________________________________________________________________________
[1]
https://urldefense.proofpoint.com/v2/url?u=https-3A__javaee.github.io_ser...
----- Original Message -----
> From: "Benjamin Confino" <BENJAMIC(a)uk.ibm.com > To: "Allan Zhang" <zhang(a)ca.ibm.com > Cc: "Matej Novotny"
<manovotn(a)redhat.com>, "Shinji Ohtsuka"
<EB92769(a)jp.ibm.com>, "Emily Jiang"
> <EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org
> Sent: Thursday, May 7, 2020 12:41:01 PM
> Subject: RE: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
> failover
> Hello
> Based on Allan Zhang's comments I have closed my pull
requests and
opened
> this one
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_weld_core...
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(a)redhat.com > Cc:
Benjamin Confino <BENJAMIC(a)uk.ibm.com>, Shinji Ohtsuka
> <EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)uk.ibm.com>,
> weld-dev(a)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://urldefense.proofpoint.com/v2/url?u=https-3A__docs.oracle.com_cd_E...
> 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(a)redhat.com > To: Benjamin Confino <BENJAMIC(a)uk.ibm.com > Cc: Shinji Ohtsuka <EB92769(a)jp.ibm.com>, Emily
Jiang
> <EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, Allan Zhang
> <zhang(a)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(a)uk.ibm.com > > To: "Matej Novotny"
<manovotn(a)redhat.com > > Cc: "Shinji
Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
> <EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, "Allan
> > Zhang" <zhang(a)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(a)redhat.com > > To: Benjamin Confino <BENJAMIC(a)uk.ibm.com > > Cc: weld-dev(a)lists.jboss.org, Allan Zhang
<zhang(a)ca.ibm.com>,
Shinji
> > Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)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(a)uk.ibm.com > > > To: "Matej Novotny"
<manovotn(a)redhat.com > > > Cc:
weld-dev(a)lists.jboss.org, "Allan Zhang" <zhang(a)ca.ibm.com>,
> "Shinji
> > Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
> > > <EMIJIANG(a)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(a)redhat.com >
> > To: Benjamin Confino <BENJAMIC(a)uk.ibm.com >
> > Cc: weld-dev(a)lists.jboss.org, Allan Zhang <zhang(a)ca.ibm.com>,
> Shinji
> > > Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)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(a)uk.ibm.com > > > > To: weld-dev(a)lists.jboss.org
> > > > Cc: "Allan Zhang" <zhang(a)ca.ibm.com>, "Shinji
Ohtsuka"
> > > <EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)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(a)lists.jboss.org
> > > > > >
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.jboss.org_mail...
> 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
10:38 a.m.
New subject: Propagation of org.jboss.weld.context.ConversationContext.conversations through session failover
That's a very good catch! You're right.
Tracking this back, I think I understand why and how it works the way it does now.
That JBoss issue was a known problem before a rework of session replication - after that
it was no longer an issue and that, I suppose,
was the reason why we still had WELD-1130 open and unsolved. I guess we didn't hear
any complaints from other EE servers until you came
and implemented WELD-1130 using the added config option
'RESET_HTTP_SESSION_ATTR_ON_BEAN_ACCESS'.
In the light of that I think the correct way to solve this would be a PR just like
Benjamin sent initially - https://github.com/weld/core/pull/1983/files
Implement aggressive re-storing of conversation map in the session but gate it behind the
aforementioned configuration property (so that default Weld behaviour stays intact).
I've already created a WELD issue for it - https://issues.redhat.com/browse/WELD-2626
Feel free to send PRs.
Regards
Matej
----- Original Message -----
From: "Allan Zhang" <zhang(a)ca.ibm.com>
To: "Matej Novotny" <manovotn(a)redhat.com>
Cc: "Benjamin Confino" <BENJAMIC(a)uk.ibm.com>, "Shinji Ohtsuka"
<EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org
Sent: Tuesday, May 12, 2020 6:26:34 PM
Subject: RE: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
failover
I thought this was an old bug that resolved already
https://issues.redhat.com/browse/WELD-1130
Similarly It was documented under know issue of JBPAPP6-1326 - CDI beans
with SET replication trigger are not replicating
https://access.redhat.com/documentation/en-us/jboss_enterprise_applicatio...
@Benjamin Can the customer use SET_AND_NON_PRIMITIVE_GET setting?
Thanks,
Allan Zhang
From: Matej Novotny <manovotn(a)redhat.com>
To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
Cc: Shinji Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, Allan Zhang
<zhang(a)ca.ibm.com>
Date: 2020-05-12 09:10 AM
Subject: [EXTERNAL] Re: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations
through session failover
@Allan
I read part 7.7.2 of the spec and I see no indications of such requirement
there.
FYI, you are probably not subscribed to weld-dev list and your emails are
not getting through (but I am getting them as I am in the CC) -
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.jboss.org_pipe...
@Benjamin
> 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?
As for Weld, the conversation map is not the only thing we store in
session.
For instance, whole session context is stored there (every single bean).
Even conversation context might be stored there and maybe something else
but from the top of my head, these are quite prominent.
And we also do not call setAttribute() every single time a bean changes or
is accessed. That would be massive perf overhead.
What I mean is that the issue you are seeing is a tip of an iceberg - it
looks easy to fix for this one scenario/case, but it wouldn't be a complete
change as multiple other things function the same way and it would require
bigger code changes that I don't think are worth it.
What you are asking for is, simply said, an unsupported replication mode.
Unless you can change the way Liberty treats mutable attributes in session,
your customer should probably swap to a mode where all attributes are
stored/replicated on shutdown.
Matej
----- Original Message -----
> From: "Benjamin Confino" <BENJAMIC(a)uk.ibm.com>
> To: "Matej Novotny" <manovotn(a)redhat.com>
> Cc: "Shinji Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, "Allan
> Zhang" <zhang(a)ca.ibm.com>
> Sent: Monday, May 11, 2020 11:02:04 PM
> Subject: RE: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
> failover
>
> Hello
>
> 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.
>
> Regards
> Benjamin
>
>
>
> From: Matej Novotny <manovotn(a)redhat.com>
> To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> Cc: Allan Zhang <zhang(a)ca.ibm.com>, Shinji Ohtsuka
> <EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)uk.ibm.com>,
> weld-dev(a)lists.jboss.org
> Date: 11/05/2020 09:33
> Subject: [EXTERNAL] Re: [weld-dev] Propagation of
> org.jboss.weld.context.ConversationContext.conversations through session
> failover
>
>
>
> 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?
>
> I went looking into servlet specification[1] and asked someone from WFLY
> who has more expertise in this field to also take a look.
> None of us found anything stating that you are required to call
> setAttribute() every time you change mutable attribute.
> In fact there is very little about attributes (or "replication triggers"
> for attributes) and you could say this is grey area.
>
> 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.
>
> Matej
>
_____________________________________________________________________________________
> [1]
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__javaee.github.io_ser...
>
>
> ----- Original Message -----
> > From: "Benjamin Confino" <BENJAMIC(a)uk.ibm.com>
> > To: "Allan Zhang" <zhang(a)ca.ibm.com>
> > Cc: "Matej Novotny" <manovotn(a)redhat.com>, "Shinji
Ohtsuka"
> <EB92769(a)jp.ibm.com>, "Emily Jiang"
> > <EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org
> > Sent: Thursday, May 7, 2020 12:41:01 PM
> > Subject: RE: [weld-dev] Propagation of
> org.jboss.weld.context.ConversationContext.conversations through session
> > failover
> >
> > Hello
> >
> > Based on Allan Zhang's comments I have closed my pull requests and
> opened
> > this one
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_weld_core...
> 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(a)redhat.com>
> > Cc: Benjamin Confino <BENJAMIC(a)uk.ibm.com>, Shinji Ohtsuka
> > <EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)uk.ibm.com>,
> > weld-dev(a)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://urldefense.proofpoint.com/v2/url?u=https-3A__docs.oracle.com_cd_E...
>
> >
> > 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(a)redhat.com>
> > To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> > Cc: Shinji Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
> > <EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, Allan Zhang
> > <zhang(a)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(a)uk.ibm.com>
> > > To: "Matej Novotny" <manovotn(a)redhat.com>
> > > Cc: "Shinji Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily
Jiang"
> > <EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, "Allan
> > > Zhang" <zhang(a)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(a)redhat.com>
> > > To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> > > Cc: weld-dev(a)lists.jboss.org, Allan Zhang <zhang(a)ca.ibm.com>,
> Shinji
> > > Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)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(a)uk.ibm.com>
> > > > To: "Matej Novotny" <manovotn(a)redhat.com>
> > > > Cc: weld-dev(a)lists.jboss.org, "Allan Zhang"
<zhang(a)ca.ibm.com>,
> > "Shinji
> > > Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
> > > > <EMIJIANG(a)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(a)redhat.com>
> > > > To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> > > > Cc: weld-dev(a)lists.jboss.org, Allan Zhang
<zhang(a)ca.ibm.com>,
> > Shinji
> > > > Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)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(a)uk.ibm.com>
> > > > > To: weld-dev(a)lists.jboss.org
> > > > > Cc: "Allan Zhang" <zhang(a)ca.ibm.com>,
"Shinji Ohtsuka"
> > > > <EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)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(a)lists.jboss.org
> > > > >
> > > >
> > >
> >
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.jboss.org_mail...
>
> >
> > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > 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
> >
>
>
>
>
> 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
>
10:15 a.m.
New subject: Propagation of org.jboss.weld.context.ConversationContext.conversations through session failover
It sounds like we have an agreement.
As you have requested I have reopened my PRs:
2.4: https://github.com/weld/core/pull/1983
3.1: https://github.com/weld/core/pull/1992
master: https://github.com/weld/core/pull/1982
Please let me know if you have any other changes you'd like made.
Best regards
Benjamin
From: Matej Novotny <manovotn(a)redhat.com>
To: Allan Zhang <zhang(a)ca.ibm.com>
Cc: Benjamin Confino <BENJAMIC(a)uk.ibm.com>, Shinji Ohtsuka
<EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)uk.ibm.com>,
weld-dev(a)lists.jboss.org
Date: 13/05/2020 15:38
Subject: [EXTERNAL] Re: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
failover
That's a very good catch! You're right.
Tracking this back, I think I understand why and how it works the way it
does now.
That JBoss issue was a known problem before a rework of session
replication - after that it was no longer an issue and that, I suppose,
was the reason why we still had WELD-1130 open and unsolved. I guess we
didn't hear any complaints from other EE servers until you came
and implemented WELD-1130 using the added config option
'RESET_HTTP_SESSION_ATTR_ON_BEAN_ACCESS'.
In the light of that I think the correct way to solve this would be a PR
just like Benjamin sent initially -
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_weld_core...
Implement aggressive re-storing of conversation map in the session but
gate it behind the aforementioned configuration property (so that default
Weld behaviour stays intact).
I've already created a WELD issue for it -
https://urldefense.proofpoint.com/v2/url?u=https-3A__issues.redhat.com_br...
Feel free to send PRs.
Regards
Matej
----- Original Message -----
From: "Allan Zhang" <zhang(a)ca.ibm.com>
To: "Matej Novotny" <manovotn(a)redhat.com>
Cc: "Benjamin Confino" <BENJAMIC(a)uk.ibm.com>, "Shinji Ohtsuka"
<EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org
Sent: Tuesday, May 12, 2020 6:26:34 PM
Subject: RE: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
failover
I thought this was an old bug that resolved already
https://urldefense.proofpoint.com/v2/url?u=https-3A__issues.redhat.com_br...
Similarly It was documented under know issue of JBPAPP6-1326 - CDI beans
with SET replication trigger are not replicating
https://urldefense.proofpoint.com/v2/url?u=https-3A__access.redhat.com_do...
@Benjamin Can the customer use SET_AND_NON_PRIMITIVE_GET setting?
Thanks,
Allan Zhang
From: Matej Novotny <manovotn(a)redhat.com>
To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
Cc: Shinji Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, Allan Zhang
<zhang(a)ca.ibm.com>
Date: 2020-05-12 09:10 AM
Subject: [EXTERNAL] Re: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations
through session failover
@Allan
I read part 7.7.2 of the spec and I see no indications of such
requirement
there.
FYI, you are probably not subscribed to weld-dev list and your emails
are
not getting through (but I am getting them as I am in the CC) -
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.jboss.org_pipe...
@Benjamin
> 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?
As for Weld, the conversation map is not the only thing we store in
session.
For instance, whole session context is stored there (every single bean).
Even conversation context might be stored there and maybe something else
but from the top of my head, these are quite prominent.
And we also do not call setAttribute() every single time a bean changes
or
is accessed. That would be massive perf overhead.
What I mean is that the issue you are seeing is a tip of an iceberg - it
looks easy to fix for this one scenario/case, but it wouldn't be a
complete
change as multiple other things function the same way and it would
require
bigger code changes that I don't think are worth it.
What you are asking for is, simply said, an unsupported replication
mode.
Unless you can change the way Liberty treats mutable attributes in
session,
your customer should probably swap to a mode where all attributes
are
stored/replicated on shutdown.
Matej
----- Original Message -----
> From: "Benjamin Confino" <BENJAMIC(a)uk.ibm.com>
> To: "Matej Novotny" <manovotn(a)redhat.com>
> Cc: "Shinji Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, "Allan
> Zhang" <zhang(a)ca.ibm.com>
> Sent: Monday, May 11, 2020 11:02:04 PM
> Subject: RE: [weld-dev] Propagation of
org.jboss.weld.context.ConversationContext.conversations through session
> failover
>
> Hello
>
> 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.
>
> Regards
> Benjamin
>
>
>
> From: Matej Novotny <manovotn(a)redhat.com>
> To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> Cc: Allan Zhang <zhang(a)ca.ibm.com>, Shinji Ohtsuka
> <EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)uk.ibm.com>,
> weld-dev(a)lists.jboss.org
> Date: 11/05/2020 09:33
> Subject: [EXTERNAL] Re: [weld-dev] Propagation of
> org.jboss.weld.context.ConversationContext.conversations through
session
> failover
>
>
>
> 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?
>
> I went looking into servlet specification[1] and asked someone from
WFLY
> who has more expertise in this field to also take a look.
> None of us found anything stating that you are required to call
> setAttribute() every time you change mutable attribute.
> In fact there is very little about attributes (or "replication
triggers"
> for attributes) and you could say this is grey area.
>
> 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.
>
> Matej
>
_____________________________________________________________________________________
> [1]
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__javaee.github.io_ser...
>
>
> ----- Original Message -----
> > From: "Benjamin Confino" <BENJAMIC(a)uk.ibm.com>
> > To: "Allan Zhang" <zhang(a)ca.ibm.com>
> > Cc: "Matej Novotny" <manovotn(a)redhat.com>, "Shinji
Ohtsuka"
> <EB92769(a)jp.ibm.com>, "Emily Jiang"
> > <EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org
> > Sent: Thursday, May 7, 2020 12:41:01 PM
> > Subject: RE: [weld-dev] Propagation of
> org.jboss.weld.context.ConversationContext.conversations through
session
> > failover
> >
> > Hello
> >
> > Based on Allan Zhang's comments I have closed my pull requests and
> opened
> > this one
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_weld_core...
> 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(a)redhat.com>
> > Cc: Benjamin Confino <BENJAMIC(a)uk.ibm.com>, Shinji Ohtsuka
> > <EB92769(a)jp.ibm.com>, Emily Jiang <EMIJIANG(a)uk.ibm.com>,
> > weld-dev(a)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://urldefense.proofpoint.com/v2/url?u=https-3A__docs.oracle.com_cd_E...
>
> >
> > 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(a)redhat.com>
> > To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> > Cc: Shinji Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
> > <EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, Allan Zhang
> > <zhang(a)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(a)uk.ibm.com>
> > > To: "Matej Novotny" <manovotn(a)redhat.com>
> > > Cc: "Shinji Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily
Jiang"
> > <EMIJIANG(a)uk.ibm.com>, weld-dev(a)lists.jboss.org, "Allan
> > > Zhang" <zhang(a)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(a)redhat.com>
> > > To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> > > Cc: weld-dev(a)lists.jboss.org, Allan Zhang <zhang(a)ca.ibm.com>,
> Shinji
> > > Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)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(a)uk.ibm.com>
> > > > To: "Matej Novotny" <manovotn(a)redhat.com>
> > > > Cc: weld-dev(a)lists.jboss.org, "Allan Zhang"
<zhang(a)ca.ibm.com>,
> > "Shinji
> > > Ohtsuka" <EB92769(a)jp.ibm.com>, "Emily Jiang"
> > > > <EMIJIANG(a)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(a)redhat.com>
> > > > To: Benjamin Confino <BENJAMIC(a)uk.ibm.com>
> > > > Cc: weld-dev(a)lists.jboss.org, Allan Zhang
<zhang(a)ca.ibm.com>,
> > Shinji
> > > > Ohtsuka <EB92769(a)jp.ibm.com>, Emily Jiang
<EMIJIANG(a)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(a)uk.ibm.com>
> > > > > To: weld-dev(a)lists.jboss.org
> > > > > Cc: "Allan Zhang" <zhang(a)ca.ibm.com>,
"Shinji Ohtsuka"
> > > > <EB92769(a)jp.ibm.com>, "Emily Jiang"
<EMIJIANG(a)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(a)lists.jboss.org
> > > > >
> > > >
> > >
> >
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.jboss.org_mail...
>
> >
> > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > 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
> >
>
>
>
>
> 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
1441
days inactive
1448
days old
6 comments
3 participants
participants (3)
-
Allan Zhang -
Benjamin Confino -
Matej Novotny