2:58 a.m.
----- Original Message -----
From: "Alexander Wels" <awels(a)redhat.com>
To: "Oved Ourfali" <ovedo(a)redhat.com>
Cc: "Tomas Jelinek" <tjelinek(a)redhat.com>, "Liran Zelkha"
<lzelkha(a)redhat.com>, devel(a)ovirt.org
Sent: Tuesday, June 2, 2015 3:28:35 PM
Subject: Re: [ovirt-devel] hibernate's internal PersistentBag sent to FE
On Tuesday, June 02, 2015 09:10:38 AM Oved Ourfali wrote:
> ----- 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 ?
>
I will be working on 41810 to make sure it is solid and is working as
expected. I am expecting it to take 1-2 days to fully complete, but I will
try
to do it asap.
OK, Alexander have posted a new version of https://gerrit.ovirt.org/#/c/41810 and I think
it is now good enough to be merged. I have tested it on my setup and everything seems to
be working again.
I think there is some room for enhancements but that enhancements can be discussed
separately in a separate patch.
Long story short - everyone who was stuck can now debug the FE again :) Thanx Alexander
for the patch!
> > @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