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

----- Original Message ----- > From: "Tomas Jelinek" <tjelinek(a)redhat.com&gt; > To: awels(a)redhat.com > Cc: "Liran Zelkha" <lzelkha(a)redhat.com&gt;, "Oved Ourfali" > <ovedo(a)redhat.com&gt;, 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&gt; > > To: "Liran Zelkha" <lzelkha(a)redhat.com&gt; > > Cc: "Tomas Jelinek" <tjelinek(a)redhat.com&gt;, "Oved Ourfali" > > <ovedo(a)redhat.com&gt;, 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&gt; > > > > To: "Oved Ourfali" <ovedo(a)redhat.com&gt; > > > > Cc: awels(a)redhat.com, "Liran Zelkha" <lzelkha(a)redhat.com&gt;, > > > > 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&gt; > > > > > To: awels(a)redhat.com > > > > > Cc: "Liran Zelkha" <lzelkha(a)redhat.com&gt;, 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&gt; > > > > > > To: "Martin Perina" <mperina(a)redhat.com&gt; > > > > > > Cc: "Liran Zelkha" <lzelkha(a)redhat.com&gt;, 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&gt; > > > > > > > > To: "Tomas Jelinek" <tjelinek(a)redhat.com&gt; > > > > > > > > Cc: "Liran Zelkha" <lzelkha(a)redhat.com&gt;, 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&gt; > > > > > > > > > > To: devel(a)ovirt.org > > > > > > > > > > Cc: "Tomas Jelinek" <tjelinek(a)redhat.com&gt;, "Liran > > > > > > > > > > Zelkha" > > > > > > > > > > <lzelkha(a)redhat.com&gt; 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

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