On 09/09/13 14:17, Boris Pavlovic wrote: > Nikola, > > The main problem in this case that we use DB in wrong way. > If you need in online request to return 100k rows then you are doing > something wrong IMHO.
Agreed! > And this fix is better then nothing (and it is the > best thing that we could do). > I am sensitive to that - this is tricky timing for such an issue, and as I stated on the review - will remove the -1 if there are more people who come up and say they want to keep it. I am just speaking up against this way of introducing optimisations which I think is wrong and should happen only in special cases like this (maybe, let's see :)), and not be something we do as a rule. If we want to be serious about performance - we can't keep on doing this - IMHO. N. > > Best regards, > Boris Pavlovic > > > > On Mon, Sep 9, 2013 at 4:10 PM, Nikola Đipanov <[email protected] > <mailto:[email protected]>> wrote: > > On 09/09/13 13:12, Roman Podolyaka wrote: > > Hi, > > > > I'm ok with both accepting this patch and reverting the commit, which > > introduced the regression, but it would be really nice to have > these DB > > optimizations in Nova. > > > > As for your concern of accepting such optimizations. I don't > think, it's > > a problem of such patches themselves, but rather with the lack of > > comprehensive tests of complex OpenStack installations in our CI > (at the > > same time I personally believe our CI is the best thing ever > happened to > > OpenStack :), CI team you really rock!). > > > > More CI is always good - and OpenStack CI is amazing without a doubt. > However - I think this is not something that ties in very much with > these kind of optimisations. > > The problem with (most) optimisations is that they optimise for a use > case. No matter how at scale you tested this - it is still one data > point - and this patch does not come for free (regression risk, > technical debt to name only a few). You can mitigate some of the issues > with CI - but you can't say for sure that this is not in fact slower for > people with different deployments/usage patterns. > > Now this patch may very well be a net win - but I personally have no way > of knowing that - and to me - this is the problem with this whole > approach - I see the downsides, but don't see the conclusive evidence of > the pros of it (we tried it on a really big (tm) cluster - it must be > better is not real data IMHO). > > I realize that we as a project are approaching the point where > performance is a thing we need to care about - but then we need to own > it as a project (maybe have a sub-team that works on this exclusively). > One shot DB query optimisations in Python code seem like a horrible > anti-pattern TBH. > > I may propose a session on this for the summit. > > Cheers, > > N. > > > Anyway, TripleO-CI found this regression. Maybe we should consider > > adding its job to Nova check/gate pipelines? > > > > Thanks, > > Roman > > > > > > On Mon, Sep 9, 2013 at 1:59 PM, Nikola Đipanov > <[email protected] <mailto:[email protected]> > > <mailto:[email protected] <mailto:[email protected]>>> wrote: > > > > On 09/09/13 11:25, Roman Podolyaka wrote: > > > Hi, > > > > > > There is a patch on review > (https://review.openstack.org/#/c/45422/) > > > fixing https://bugs.launchpad.net/tripleo/+bug/1221620 which has > > > importance 'Critical' in Nova and TripleO (long story short: > currently > > > Nova Baremetal deployments with more than one baremetal node > won't > > work). > > > > > > It would be really nice to have this patch reviewed by core > > developers, > > > so we can fix the bug ASAP. > > > > > > > Hey - thanks for responding quickly - I commented on the patch > and tbh I > > am starting to be -1 on this due to issues mentioned on the > review. > > > > I will accept that my take on this is too conservative :) and > remove a > > -1 if needed to get this in, but at this point, I have some doubts > > weather this is the right approach. > > > > Cheers, > > > > N. > > > > > > > Thanks, > > > Roman > > > > > > > > > _______________________________________________ > > > OpenStack-dev mailing list > > > [email protected] > <mailto:[email protected]> > > <mailto:[email protected] > <mailto:[email protected]>> > > > > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > > > > > > > > _______________________________________________ > > OpenStack-dev mailing list > > [email protected] > <mailto:[email protected]> > > <mailto:[email protected] > <mailto:[email protected]>> > > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > > > > > > > > > _______________________________________________ > > OpenStack-dev mailing list > > [email protected] > <mailto:[email protected]> > > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > > > > _______________________________________________ > OpenStack-dev mailing list > [email protected] > <mailto:[email protected]> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > > > > _______________________________________________ > OpenStack-dev mailing list > [email protected] > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > _______________________________________________ OpenStack-dev mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
