From: "Tomas Jelinek" <tjelinek(a)redhat.com>
To: "Oved Ourfali" <ovedo(a)redhat.com>
Cc: "Liran Zelkha" <lzelkha(a)redhat.com>, devel(a)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(a)redhat.com>
> To: awels(a)redhat.com
> Cc: "Liran Zelkha" <lzelkha(a)redhat.com>, devel(a)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(a)redhat.com>
> > To: "Martin Perina" <mperina(a)redhat.com>
> > Cc: "Liran Zelkha" <lzelkha(a)redhat.com>, devel(a)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(a)redhat.com>
> > > > To: "Tomas Jelinek" <tjelinek(a)redhat.com>
> > > > Cc: "Liran Zelkha" <lzelkha(a)redhat.com>,
devel(a)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(a)redhat.com>
> > > > > > To: devel(a)ovirt.org
> > > > > > Cc: "Tomas Jelinek" <tjelinek(a)redhat.com>,
"Liran Zelkha"
> > > > > > <lzelkha(a)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(a)ovirt.org
> > > > > > >
http://lists.ovirt.org/mailman/listinfo/devel
> > > >
> > > > _______________________________________________
> > > > Devel mailing list
> > > > Devel(a)ovirt.org
> > > >
http://lists.ovirt.org/mailman/listinfo/devel
> >
> > _______________________________________________
> > Devel mailing list
> > Devel(a)ovirt.org
> >
http://lists.ovirt.org/mailman/listinfo/devel
> >
> >
> >
> _______________________________________________
> Devel mailing list
> Devel(a)ovirt.org
>
http://lists.ovirt.org/mailman/listinfo/devel
>
_______________________________________________
Devel mailing list
Devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel