Bug 1982996

From my understanding init_host calls _init_instance on each instances. It doesn't look like this is multi-threaded unless I'm missing something.

The conflict is kinda expected because _init_instance [1] tries to attach all VIF, there's no pre-validation, we just trap the conflict exception [2].

I don't see anything in place to prevent, throttle, cache or multithread these useless calls but I suspect the ironic calls are the ones slowing this down.

We will discuss this on Wednesday triage call but adding Mel to the needinfo, for preparation since she helped on the previous issue.


In the meantime Jaison, if you want to comment the lines in [2] from your environment and see if nova-compute is a lot faster, that would help us narrow down the source of slowness.

Thanks,

DVD

[1] https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1136-L1150
[2] https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L1520-L1522

Commenting those lines end with service startup failures.

Although if I skip plug_vif then it reduced the time taken for _get_power_state & _plug_vifs from 45min to 2mins.

()[root@e16-h12-b01-fc640 /]# diff /usr/lib/python3.6/site-packages/nova/virt/ironic/driver.py-bak  /usr/lib/python3.6/site-packages/nova/virt/ironic/driver.py
1683c1683
<         self._plug_vifs(node, instance, network_info)
---
> #        self._plug_vifs(node, instance, network_info)

Can we include some feature in nova for a configuration that will disable this for ironic nodes for only during service starts? So during nova instance build on ironic nodes, this configuration wont change the behavior of _plug_vif/attach_interface.
I am not sure if this will impact any functionality in case of ironic. 
If we could get this, then we can have tripleo-common to enable this configuration in nova for director based deployments.

Entirely unrelated but also related oddly, I was sick and tired of not being able to schedule breakfast on the weekends with one of my best friends who operates a multi-thousand node baremetal cluster fronted by Nova, because they have to restart their nova-compute services almost every weekend. So I put up a patch upstream to disable the plug_vifs action as it originates from startup. I'm unsure if we could get it backported, and that is likely best answered by the compute team. Adding link for the upstream change set.

Apologies, I somehow completely missed this needinfo for me :(

(In reply to Julia Kreger from comment #5)
> Entirely unrelated but also related oddly, I was sick and tired of not being
> able to schedule breakfast on the weekends with one of my best friends who
> operates a multi-thousand node baremetal cluster fronted by Nova, because
> they have to restart their nova-compute services almost every weekend. So I
> put up a patch upstream to disable the plug_vifs action as it originates
> from startup. I'm unsure if we could get it backported, and that is likely
> best answered by the compute team. Adding link for the upstream change set.

I just had a look at the linked patch [1] and it looks backportable to me.

If that patch will solve the issue in this rhbz, we can remove FutureFeature from this and address it as a bug fix backport.

[1] https://review.opendev.org/c/openstack/nova/+/813263

(In reply to melanie witt from comment #6)

> 
> If that patch will solve the issue in this rhbz, we can remove FutureFeature
> from this and address it as a bug fix backport.
> 

I do think so, given the timing in the notes indicated the vast majority of the time spent initializing was vif plugging, and trying to do parallel threaded cache creation just seems like a nightmare risk wise.

I also know of a couple operators who have expressed intent to, or already have pulled that patch down into their internal forks/backport branches, so I *suspect* it does the needful and will make folks happier. I also know an operator who already intentionally short circuited it on their own, it only came up *after* I put the patch up.

Jason, any opinion here?

(In reply to Julia Kreger from comment #7)
> (In reply to melanie witt from comment #6)
> 
> > 
> > If that patch will solve the issue in this rhbz, we can remove FutureFeature
> > from this and address it as a bug fix backport.
> > 
> 
> I do think so, given the timing in the notes indicated the vast majority of
> the time spent initializing was vif plugging, and trying to do parallel
> threaded cache creation just seems like a nightmare risk wise.
> 
> I also know of a couple operators who have expressed intent to, or already
> have pulled that patch down into their internal forks/backport branches, so
> I *suspect* it does the needful and will make folks happier. I also know an
> operator who already intentionally short circuited it on their own, it only
> came up *after* I put the patch up.
> 
> Jason, any opinion here?

To be clear, I have proposed backports for the patch upstream as I think the linked patch is an important fix for general operability.

What I mean to ask here is whether that patch will solve this rhbz (if so, we don't need it to be an RFE and it is a bug). If it won't solve the issue in this rhbz, I will open a new rhbz to use for the backport activity and leave this one as an RFE for an enhancement that is required beyond the bug fix.

(In reply to Julia Kreger from comment #7)
> (In reply to melanie witt from comment #6)
> 
> > 
> > If that patch will solve the issue in this rhbz, we can remove FutureFeature
> > from this and address it as a bug fix backport.
> > 
> 
> I do think so, given the timing in the notes indicated the vast majority of
> the time spent initializing was vif plugging, and trying to do parallel
> threaded cache creation just seems like a nightmare risk wise.
> 
> I also know of a couple operators who have expressed intent to, or already
> have pulled that patch down into their internal forks/backport branches, so
> I *suspect* it does the needful and will make folks happier. I also know an
> operator who already intentionally short circuited it on their own, it only
> came up *after* I put the patch up.
> 
> Jason, any opinion here?

Yes. I agree. Thanks.
parallelization may help in heavy computes with a large number of VMS, although we have not tested it.
I will try to check this on 100 vm computes and raise a separate bug if parallelization is really required in general.

Going with my usual "better to ask for forgiveness than for permission" approach, I'm going to remove the FutureFeature keyword, as it looks like the bugfix approach proposed by Melanie in comment #8 was accepted, and a separate BZ will be raised if we still need the original parallelization that was requested here.

This bug was originally opened against OSP16.1, so I'm changing the flags and target milestone to 16.1 to make it easier for the bug reporter to track the version they're interested in.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Red Hat OpenStack Platform 16.1.9 bug fix and enhancement advisory), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2022:8795