9:10 a.m.
----- Original Message -----
From: "Tomas Jelinek" <tjelinek(a)redhat.com>
To: awels(a)redhat.com
Cc: "Liran Zelkha" <lzelkha(a)redhat.com>, "Oved Ourfali"
<ovedo(a)redhat.com>, devel(a)ovirt.org
Sent: Tuesday, June 2, 2015 4:08:05 PM
Subject: Re: [ovirt-devel] hibernate's internal PersistentBag sent to FE
----- Original Message -----
> From: "Alexander Wels" <awels(a)redhat.com>
> To: "Liran Zelkha" <lzelkha(a)redhat.com>
> Cc: "Tomas Jelinek" <tjelinek(a)redhat.com>, "Oved Ourfali"
> <ovedo(a)redhat.com>, devel(a)ovirt.org
> Sent: Tuesday, June 2, 2015 2:51:31 PM
> Subject: Re: [ovirt-devel] hibernate's internal PersistentBag sent to FE
>
> On Tuesday, June 02, 2015 03:05:11 AM Liran Zelkha wrote:
> > ----- Original Message -----
> >
> > > From: "Tomas Jelinek" <tjelinek(a)redhat.com>
> > > To: "Oved Ourfali" <ovedo(a)redhat.com>
> > > Cc: awels(a)redhat.com, "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.
> >
> > I don't think it really matters where we do it, as long as server code
> > will
> > still use the PersistentBag (otherwise we won't be able to use attached
> > entities, and performance will suffer). I believe that serializing these
> > objects to JSON/XML - will work, we won't need to do 2 level of
> > conversions
> > (PersistentBag-->List-->JSON/XML). So if the issue is only on the GWT
> > side,
> > and we won't to write this code once, and not per entity - where is the
> > best way to put it?
>
> The original problem comes from the fact that we are serializing an object
> from the backend to the frontend that the frontend cannot handle (The
> persistent bag). This is a backend implementation detail that the frontend
> really doesn't care about. In essence GWT is Javascript that is made to
> look
> like Java, but it has a bunch of limitations due to the fact it still ends
> up
> as javascript in the end.
>
> > > > 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/).
> > >
>
> The overhead comes from having to scan through every single object that we
> are
> transferring to the front end, even though right now only one or two object
> types have this particular problem. The scanning only happens if we
> actually
> transfer the object to the frontend. If there was a way to determine if we
> need to scan the objects being returned to the front end that would cut
> down
> on the overhead. Another question to ask, is the overhead BAD enough to
> care?
> I honestly don't know without doing some profiling. But my gut is telling
> me
> no
> it is not.
I agree this is the correct fix on the correct place. On the other hand, it
needs some optimizations and tests if it actually works and if it is not a
big bottleneck.
Also we need to consider if we want to have some annotation on classes we
want to filter before sending back to frontend or we want this to be done
all the time.
Long story short, I think the patch which broke the frontend debugging should
be reverted and merged once the https://gerrit.ovirt.org/#/c/41810/ is
finished and tested.
What's the ETA on finishing https://gerrit.ovirt.org/#/c/41810 ?
@Liran: could you please revert your patch? Because currently the FE
debugging is pretty much broken (and I don't trust the web mode either...)
and the solution is not completely clear now.
Let's see what the ETA is, as perhaps we can merge something soon.
Liran's patch is a big change in all DB layers, so won't be easy to revert.
>
> > > > 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
>
>