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@ca.ibm.com>

> To: "Matej Novotny" <manovotn@redhat.com>

> Cc: "Benjamin Confino" <BENJAMIC@uk.ibm.com>, "Shinji
Ohtsuka" <EB92769@jp.ibm.com>, "Emily Jiang"

> <EMIJIANG@uk.ibm.com>, weld-dev@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_application_platform/6/html/release_notes_6.0.1/ar01s06

> @Benjamin Can the customer use SET_AND_NON_PRIMITIVE_GET setting?

> 

> Thanks,

> Allan Zhang

> 

> 

> 

> From:                
Matej Novotny <manovotn@redhat.com>

> To:                
Benjamin Confino <BENJAMIC@uk.ibm.com>

> Cc:                
Shinji Ohtsuka <EB92769@jp.ibm.com>, Emily Jiang

>             <EMIJIANG@uk.ibm.com>,
weld-dev@lists.jboss.org, Allan Zhang

>             <zhang@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://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@uk.ibm.com>

> > To: "Matej Novotny" <manovotn@redhat.com>

> > Cc: "Shinji Ohtsuka" <EB92769@jp.ibm.com>, "Emily
Jiang"

> <EMIJIANG@uk.ibm.com>, weld-dev@lists.jboss.org, "Allan

> > Zhang" <zhang@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@redhat.com>

> > To:     Benjamin Confino <BENJAMIC@uk.ibm.com>

> > Cc:     Allan Zhang <zhang@ca.ibm.com>, Shinji
Ohtsuka

> > <EB92769@jp.ibm.com>, Emily Jiang <EMIJIANG@uk.ibm.com>,

> > weld-dev@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://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_FINAL.pdf

> 

> >

> >

> > ----- Original Message -----

> > > From: "Benjamin Confino" <BENJAMIC@uk.ibm.com>

> > > To: "Allan Zhang" <zhang@ca.ibm.com>

> > > Cc: "Matej Novotny" <manovotn@redhat.com>,
"Shinji Ohtsuka"

> > <EB92769@jp.ibm.com>, "Emily Jiang"

> > > <EMIJIANG@uk.ibm.com>, weld-dev@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@redhat.com>

> > > Cc:     Benjamin Confino <BENJAMIC@uk.ibm.com>,
Shinji Ohtsuka

> > > <EB92769@jp.ibm.com>, Emily Jiang <EMIJIANG@uk.ibm.com>,

> > > weld-dev@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@redhat.com>

> > > To:     Benjamin Confino <BENJAMIC@uk.ibm.com>

> > > Cc:     Shinji Ohtsuka <EB92769@jp.ibm.com>,
Emily Jiang

> > > <EMIJIANG@uk.ibm.com>, weld-dev@lists.jboss.org, Allan
Zhang

> > > <zhang@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@uk.ibm.com>

> > > > To: "Matej Novotny" <manovotn@redhat.com>

> > > > Cc: "Shinji Ohtsuka" <EB92769@jp.ibm.com>,
"Emily Jiang"

> > > <EMIJIANG@uk.ibm.com>, weld-dev@lists.jboss.org, "Allan

> > > > Zhang" <zhang@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@redhat.com>

> > > > To:     Benjamin Confino <BENJAMIC@uk.ibm.com>

> > > > Cc:     weld-dev@lists.jboss.org, Allan Zhang
<zhang@ca.ibm.com>,

> > Shinji

> > > > Ohtsuka <EB92769@jp.ibm.com>, Emily Jiang <EMIJIANG@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@uk.ibm.com>

> > > > > To: "Matej Novotny" <manovotn@redhat.com>

> > > > > Cc: weld-dev@lists.jboss.org, "Allan Zhang"
<zhang@ca.ibm.com>,

> > > "Shinji

> > > > Ohtsuka" <EB92769@jp.ibm.com>, "Emily
Jiang"

> > > > > <EMIJIANG@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@redhat.com>

> > > > > To:     Benjamin Confino <BENJAMIC@uk.ibm.com>

> > > > > Cc:     weld-dev@lists.jboss.org, Allan
Zhang <zhang@ca.ibm.com>,

> > > Shinji

> > > > > Ohtsuka <EB92769@jp.ibm.com>, Emily Jiang
<EMIJIANG@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@uk.ibm.com>

> > > > > > To: weld-dev@lists.jboss.org

> > > > > > Cc: "Allan Zhang" <zhang@ca.ibm.com>,
"Shinji Ohtsuka"

> > > > > <EB92769@jp.ibm.com>, "Emily Jiang"
<EMIJIANG@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@lists.jboss.org

> > > > > >

> > > > >

> > > >

> > >

> >

> 
https://lists.jboss.org/mailman/listinfo/weld-dev

> 

> >

> > >

> > > >

> > > > >

> > > > >

> > > > >

> > > > >

> > > > >

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

> >

> 

> 

> 

> 