[oslo][nova][stable][requirements] Fixing a high CPU usage from oslo.service into stable/rocky branch

Sean Mooney smooney at redhat.com
Wed Nov 21 22:35:30 UTC 2018


On Wed, 2018-11-21 at 17:03 -0500, Zane Bitter wrote:
> On 21/11/18 2:50 PM, Sean Mooney wrote:
> > On Wed, 2018-11-21 at 14:38 -0500, Zane Bitter wrote:
> > > On 21/11/18 9:47 AM, Herve Beraud wrote:
> > > > # Questions and proposed solutions
> > > > 
> > > > This thread try to summarize the current situation.
> > > > 
> > > > We need to find how to be able to proceed, so this thread aim to allow
> > > > to discuss between team to find the best way to fix.
> > > > 
> > > > 1. Do we need to continue to try to backport fixture on oslo.service to
> > > > fix the CI problem (https://review.openstack.org/#/c/617989/)?
> 
> This is the worst option, because you won't be able to use either an 
> older nova with a newer oslo.service, nor an older oslo.service with a 
> newer nova.
> 
> In fact, if I'm interpreting Matt's comment on 
> https://review.openstack.org/619246 correctly, then this may be a 
> non-starter because increasing lower-constraints is not allowed on 
> stable branches.
you also cannot use a newer versions then are in upper-constaints
this thread arose from the fact that performacne feature/bug fix form 1.32.x is being
backported to 1.31.6. upper-constainst set a limit on oslo.service of 1.31.x so you are
not expect to be able to use newer relases of oslo.service with the rocky release of nova.
> 
> > > > 2. Do we need to find an another approach like mocking
> > > > oslo.service.loopingcall._Event.wait in nova instead of mocking
> > > > oslo_service.loopingcall._ThreadingEvent.wait (example:
> > > > https://review.openstack.org/#/c/616697/2/nova/tests/unit/compute/test_compute_mgr.py)?
> 
> This is marginally better, provided that it's done in a 
> backwards-compatible way (i.e. that doesn't require bumping the 
> lower-constraints like https://review.openstack.org/619246 and 
> https://review.openstack.org/619022 do). Here's an example that should 
> do the trick:
> 
> https://review.openstack.org/619360
> 
> However, this does still mean that you can't use an older Nova with a 
> newer oslo.service. Which is bound to cause trouble for somebody.
> 
> It also relies on a different private interface, although we're less 
> likely to need to change this one in stable/rocky.
> 
> > > 3. Doesn't this get solved if we add a line like:
> > > 
> > >     _ThreadingEvent = _Event
> > > 
> > > in oslo.service on stable/rocky? That seems harmless and the easiest way
> > > to maintain the same sort-of-public interface so nothing else ought to
> > > break either. And with no change in Nova people won't need to worry
> > > about needing to update oslo.service at the same time they update Nova
> > > to avoid breakage.
> > > 
> > > Here's a patch: https://review.openstack.org/619342
> > 
> > a stable only patch is not really any better in my view then 2
> 
> Surely being able to update oslo.service and nova independently is 
> objectively better than having to upgrade them in lock-step.
> 
> Would it make you feel better if this patch were also on master? Why?
> 
> > you are also chaning the loopingcall modules semantics
> > as it is a different type even if you are allowing previously syntaxly
> > vaild code to execute it does not maintain backwards compatiblity.
> 
> This is a fair point; only the clear()/wait()/stop() methods are the 
> same. is_running() changes to is_set() and done() changes to set(). So 
> it is a bit hacky. But it still solves the instance of the problem we 
> know about (and FWIW any instance of the problem that *could*, in 
> principle, be solved by the fixture).
if we go with this appoch and are doing a stable only change then
instead of just doing _ThreadingEvent =_Event
could we either use the debtcoltor moduel to make it as deprecated
or stub out the signigure of the old interface and have it log a warning if invoked

nova is only using this in test but if we are consusred about people upgrading then
when we remove this workaourd at some point e.g. we shoudl at least warn other that might
have done the same and depended on the ThredingEvents.

> 
> > we are not using a staticaly compiled language so we dont need to consider
> > abi breakage this would result in for c or c++ but it i would stonglely prefer 1 or 2.
> > 
> > > 
> > > cheers,
> > > Zane.
> > > 
> > > > This is only a fix on the nova side and itallowsus to update
> > > > oslo.service requirements and allowsus to fix the high CPU usage issue.
> > > > I've submit this patch (https://review.openstack.org/619246)who
> > > > implement the description above.
> > > > 
> > > > Personaly I think we need to find an another approach like the mocking
> > > > remplacement (c.f 2).
> > > > 
> > > > We need to decide which way we use and to discuss about other solutions.
> > > > 
> > > 
> > > 
> 
> 



More information about the openstack-discuss mailing list