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