[ovirt-devel] hibernate's internal PersistentBag sent to FE

Tue Jun 2 07:05:48 UTC 2015

----- Original Message -----
> From: "Tomas Jelinek" <tjelinek at redhat.com>
> To: "Oved Ourfali" <ovedo at redhat.com>
> Cc: "Liran Zelkha" <lzelkha at redhat.com>, devel at ovirt.org
> Sent: Tuesday, June 2, 2015 9:59:17 AM
> Subject: Re: [ovirt-devel] hibernate's internal PersistentBag sent to FE
> 
> 
> 
> ----- Original Message -----
> > From: "Oved Ourfali" <ovedo at redhat.com>
> > To: awels at redhat.com
> > Cc: "Liran Zelkha" <lzelkha at redhat.com>, devel at ovirt.org
> > Sent: Tuesday, June 2, 2015 8:35:56 AM
> > Subject: Re: [ovirt-devel] hibernate's internal PersistentBag sent to FE
> > 
> > top-posting:
> > Due to the fact that we'll see that all over the place, I really think that
> > it would be best to support that at the frontend level, and not the backend
> > level.
> 
> well, from philosophical perspective I think that sending
> org.hibernate.collection.**internal**.PersistentBag from backend to FE and
> than faking it's implementation on FE
> is a way to hell. It will bring lots of problems. For example the FE will be
> directly dependent on the internal structure of an internal hibernate class
> and if we update hibernate, FE can fail miserably.
> Also the FE patch (https://gerrit.ovirt.org/#/c/41682/1) seems to be more
> tricky than expected - it seems to be working but Omer had issues with it
> and in some cases (when the delegating also retainAll()) it caused my JRE to
> fail on SIGSEGV.
> It can be fixed on FE but it is a hack with all risks this kinds of hacks
> brings.
> 
> > Doing it in the backend level will cause a lot of overhead.
> 
> Is it really lots of overhead? Comparing to all other layers the data has to
> pass anyway? When abandoning GWT FE and start to use REST we will anyway
> remove the GenericApiGWTServiceImpl which contains this cleaning
> (https://gerrit.ovirt.org/#/c/41810/).

I'm okay with doing this cleaning. Just not to propagate the fix to the business entities themselves (that's what I meant by saying in the backend level).

> 
> > I'll leave the technical details to the UX experts here to see what's the
> > right approach to do it in the frontend side.
> > 
> > Thanks,
> > Oved
> > 
> > ----- Original Message -----
> > > From: "Alexander Wels" <awels at redhat.com>
> > > To: "Martin Perina" <mperina at redhat.com>
> > > Cc: "Liran Zelkha" <lzelkha at redhat.com>, devel at ovirt.org
> > > Sent: Tuesday, June 2, 2015 4:16:31 AM
> > > Subject: Re: [ovirt-devel] hibernate's internal PersistentBag sent to FE
> > > 
> > > On Monday, June 01, 2015 10:51:28 AM Martin Perina wrote:
> > > > ----- Original Message -----
> > > > 
> > > > > From: "Alexander Wels" <awels at redhat.com>
> > > > > To: "Tomas Jelinek" <tjelinek at redhat.com>
> > > > > Cc: "Liran Zelkha" <lzelkha at redhat.com>, devel at ovirt.org
> > > > > Sent: Monday, June 1, 2015 4:19:47 PM
> > > > > Subject: Re: [ovirt-devel] hibernate's internal PersistentBag sent to
> > > > > FE
> > > > > 
> > > > > On Monday, June 01, 2015 09:33:07 AM Tomas Jelinek wrote:
> > > > > > ----- Original Message -----
> > > > > > 
> > > > > > > From: "Alexander Wels" <awels at redhat.com>
> > > > > > > To: devel at ovirt.org
> > > > > > > Cc: "Tomas Jelinek" <tjelinek at redhat.com>, "Liran Zelkha"
> > > > > > > <lzelkha at redhat.com> Sent: Monday, June 1, 2015 3:16:34 PM
> > > > > > > Subject: Re: [ovirt-devel] hibernate's internal PersistentBag
> > > > > > > sent
> > > > > > > to
> > > > > > > FE
> > > > > > > 
> > > > > > > On Monday, June 01, 2015 09:08:50 AM Tomas Jelinek wrote:
> > > > > > > > Hey all,
> > > > > > > > 
> > > > > > > > since the org.ovirt.engine.core.common.job.Job/Step... has been
> > > > > > > > moved
> > > > > > > > to
> > > > > > > > use
> > > > > > > > the JPA we have a problem on frontend. The problem is that the
> > > > > > > > @OneToMany
> > > > > > > > annotations results in a List which is of type PersistentBag.
> > > > > > > > When
> > > > > > > > we
> > > > > > > > send
> > > > > > > > this to Frontend it fails during deserialization. It actually
> > > > > > > > fails
> > > > > > > > quite
> > > > > > > > bad because the FE already has an ui-override of it which is
> > > > > > > > not
> > > > > > > > correct
> > > > > > > > resulting in a ton of NPEs in development mode.
> > > > > > > > 
> > > > > > > > So, there are 2 nasty fixes I have made where none of them
> > > > > > > > should
> > > > > > > > be
> > > > > > > > merged
> > > > > > > > but demonstrate the possibilities: 1: extend the FE to be able
> > > > > > > > to
> > > > > > > > work
> > > > > > > > with
> > > > > > > > the PersistentBag (https://gerrit.ovirt.org/#/c/41682/) not
> > > > > > > > really
> > > > > > > > good
> > > > > > > > solution since the PersistenBag is an internal Hibernate class
> > > > > > > > which
> > > > > > > > is
> > > > > > > > really not meant to be passed around
> > > > > > > > 
> > > > > > > > 2: fix on the backend to not send the PersistentBag but an
> > > > > > > > ArrayList.
> > > > > > > > This
> > > > > > > > is only a PoC fixed on a command we face the problem
> > > > > > > > (https://gerrit.ovirt.org/#/c/41797/) Obviously this is not
> > > > > > > > going
> > > > > > > > to
> > > > > > > > work
> > > > > > > > for other commands accessing the same Job nor for other
> > > > > > > > entities.
> > > > > > > > 
> > > > > > > > So, the first option is generic but very very bad. The second
> > > > > > > > option
> > > > > > > > should
> > > > > > > > be used but not sure how to do this in a cheep way (e.g.
> > > > > > > > without
> > > > > > > > using
> > > > > > > > reflection to deep traverse everything sent back to frontend
> > > > > > > > checking
> > > > > > > > if
> > > > > > > > it
> > > > > > > > does not have a PersistentBag in it.
> > > > > > > 
> > > > > > > Tomas,
> > > > > > > 
> > > > > > > Thanks, I was investigating the same issue, I noticed it last
> > > > > > > Friday
> > > > > > > just
> > > > > > > before leaving, so I was investigating the problem to see what
> > > > > > > was
> > > > > > > going
> > > > > > > on. You are right we should not be sending PersistentBag to the
> > > > > > > frontend
> > > > > > > at all. So how about we do a combination of [1] and [2], but
> > > > > > > instead
> > > > > > > of
> > > > > > > delegating in [1] we actually simple throw an exception stating
> > > > > > > don't
> > > > > > > sent PersistentBag to the front end. that way anyone
> > > > > > > inadvertently
> > > > > > > using
> > > > > > > it will be notified immediately (since their code won't work).
> > > > > > 
> > > > > > Throwing the exception would help us in debugging but the main
> > > > > > question
> > > > > > is
> > > > > > how will we make it work? Since we are planning to move more and
> > > > > > more
> > > > > > to
> > > > > > JPA so we will face this issue more and more often. Solving it one
> > > > > > by
> > > > > > one
> > > > > > on backend in each command is not going to work.
> > > > > 
> > > > > How about something like this [1]? It appears to use some aspects to
> > > > > translate
> > > > > the hibernate internal classes into normal java util classes.
> > > > > 
> > > > > [1] https://code.google.com/p/dehibernator/
> > > > 
> > > > Well, not sure about that from performance point of view.
> > > > 
> > > > If we really move away from GWT completely in 4.0, then implementing
> > > > all
> > > > hibernate inner collections in "uioverride" in a similar way as Tomas
> > > > did
> > > > seems to me as the best solution from performance and "easiest to
> > > > remove
> > > > when not needed" point of view.
> > > > 
> > > 
> > > I took some inspiration from that code and wrote my own fairly simple
> > > hibernate persistent collection replacer, and I put the patch up here
> > > [3].
> > > It
> > > still uses reflection so it is probably not the fastest thing ever
> > > written.
> > > 
> > > [3] https://gerrit.ovirt.org/#/c/41810/
> > > 
> > > > > > > Alexander
> > > > > > > 
> > > > > > > > Any better ideas?
> > > > > > > > Thanx,
> > > > > > > > Tomas
> > > > > > > > _______________________________________________
> > > > > > > > Devel mailing list
> > > > > > > > Devel at ovirt.org
> > > > > > > > http://lists.ovirt.org/mailman/listinfo/devel
> > > > > 
> > > > > _______________________________________________
> > > > > Devel mailing list
> > > > > Devel at ovirt.org
> > > > > http://lists.ovirt.org/mailman/listinfo/devel
> > > 
> > > _______________________________________________
> > > Devel mailing list
> > > Devel at ovirt.org
> > > http://lists.ovirt.org/mailman/listinfo/devel
> > > 
> > > 
> > > 
> > _______________________________________________
> > Devel mailing list
> > Devel at ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/devel
> > 
> _______________________________________________
> Devel mailing list
> Devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
> 
> 
> 