Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-05-31
15:06:46 jaypipes sahid: "The whole NUMA thing is basically a cartography for the scheduler to help it making good decisions." <-- you're referring to the kernel process scheduler, right? not the nova-scheduler,...
15:07:08 mriedem belmoreira: what are you trying to solve with it?
15:07:14 mriedem missing allocations?
15:07:15 sahid jaypipes: yes yes :)
15:07:23 jaypipes sahid: ok, just making sure :)
15:07:36 belmoreira mriedem we had an issue with ironic that triggered the recreation of all compute_nodes; now the new compute_nodes can't be registered in placement because the previous resource_provider/allocations
15:08:12 mriedem so can't create the new resource provider because another with the same name/uuid already exists right?
15:08:15 jaypipes sahid, stephenfin: you'll note I tried as much as possible to remove any and all mention of NUMA topology (host or guest/virtual) in the CPU resources spec.
15:08:23 mriedem and can't delete the existing one because the existing one has allocations already?
15:08:48 belmoreira mriedem: yes
15:08:54 belmoreira the cleanest way to recover would be to delete everything related to ironic resources providers and recreate again
15:09:06 jaypipes sahid, stephenfin: at least in so much as the NUMA topology stuff wasn't really germane to the "request different providers supply some X amount of dedicated or shared CPU resources for the guest"
15:09:25 belmoreira unfortunately I have ~2000 nodes in this state
15:09:28 mriedem belmoreira: yeah i think what i'd probably do is pick one resource provider / allocation combo since they should be 1:1 with ironic,
15:09:40 mriedem clean it up using the osc-placement CLI
15:09:57 mriedem and then run heal_allocations but probably want to put a cap on the --max-count
15:09:59 stephenfin jaypipes: ack. In a meeting/working on NUMA-aware vswitches, but I'll take a look before EOD
15:10:12 mriedem belmoreira: this is why i had a note in the code about allowing passing in a single instance id to heal
15:10:17 mriedem that would be ideal in this case as a test
15:10:38 jaypipes stephenfin: no worries, you already reviewed again (and I will update to your comments)
15:10:46 belmoreira mriedem I will first test all on copies of the DBs
15:11:04 mriedem belmoreira: ok yeah definitely doing it in a test env would be good for both of us :)
15:12:26 belmoreira other use-case for this code is updating ironic allocations that were created before a resource_class was defined
15:13:38 mriedem so the existing allocations for the ironic instance are against vcpu/memory_mb/disk_gb and you want to change then to be against custom_baremetal_belmiro, yes?
15:13:46 mriedem *them
15:14:31 belmoreira mriedem: but the recreation of the compute_node in nova is something that needs to be reviewed; has been creating few issues. Don't know yet why this happened...
15:14:35 mriedem that's probably where the --force option mentioned in the comments would come in, since currently the command will skip any instances that already have allocations, not attempt to overwrite them
15:15:27 belmoreira mriedem:yes, that's way I was pointing this as another use-case
15:15:27 mriedem belmoreira: so i thought that sounded very much like https://review.openstack.org/#/c/508555/
15:16:53 belmoreira mriedem: this was not rebalance... we had an operator working in ironic/nova-compute and know is very hard to track why this happened
15:18:07 belmoreira mriedem: but anyway it looks a very dangerous operation to do when having placement. Before placement this was OK (recreating a new compute_node)
15:18:13 mriedem was the related nova-compute service deleted by any chance?
15:18:37 belmoreira mriedem: no, I don't yet
15:18:40 mriedem ok
15:18:53 mriedem for ironic the compute node record should have a predictable uuid
15:19:21 mriedem unless...
15:19:33 mriedem we don't use the ironic node uuid when creating the compute node
15:20:37 stephenfin mriedem, dansmith: We've been talking about extending the limits dict but I just noticed this https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1968-L1991
15:20:39 mriedem looks like we don't https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L580
15:21:10 stephenfin mriedem, dansmith: Given that we've essentially deprecated 'limits' and have 'requested_networks' available to us, couldn't I just use that as is?
15:21:14 jaypipes stephenfin: who's been talking about extending the limits dict?
15:21:16 belmoreira mriedem: that's the problem. When the compute_node is recreated we get a new uuid
15:21:18 mriedem belmoreira: so i think in that code, resources['hypervisor_hostname'] for ironic is always the node uuid,
15:21:26 stephenfin jaypipes: not me
15:21:27 mriedem we should use that when creating the compute node record if we think it's new
15:21:35 mriedem belmoreira: yup, seems like a trivial fix
15:21:49 mriedem belmoreira: not sure if you have enough words to open a bug?
15:22:39 mriedem stephenfin: my guess is requested_networks aren't passed back to the cell conductor during a reschedule
15:22:55 mriedem oh it looks like they are https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1890
15:22:56 belmoreira This is related with https://review.openstack.org/#/c/508555/
15:23:16 belmoreira sorry not this one...
15:23:34 stephenfin mriedem: Yeah, I checked - the info is there
15:24:16 stephenfin For migrate and unshelve, I would need to check 'instance.info_cache.network_info' but that's a-ok
15:25:03 kashyap stephenfin: Thanks for the review here, https://review.openstack.org/#/c/567258/. Yep, that could go...
15:25:32 kashyap I'll respin.
15:26:37 mriedem belmoreira: hmm, this should discover if there is already a compute node in the db with the same hypervisor_hostname, which is the ironic node uuid https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L518
15:26:44 belmoreira mriedem: https://bugs.launchpad.net/nova/+bug/1771806 (the main issue here is the new UUID that's generated. Don't think is required to have a different bug)
15:26:45 openstack Launchpad bug 1771806 in OpenStack Compute (nova) "Ironic nova-compute failover creates new resource provider removing the resource_provider_aggregates link " [Medium,Confirmed] - Assigned to Surya Seetharaman (tssurya)
15:26:50 openstackgerrit Merged openstack/nova stable/ocata: Fix shelving a paused instance https://review.openstack.org/569655
15:26:50 mriedem so it might not be a rebalance, but that code should have found it
15:27:29 mriedem unless you're hitting https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L531
15:27:50 mriedem belmoreira: yeah, and in https://bugs.launchpad.net/nova/+bug/1771806 i asked why https://review.openstack.org/#/c/508555/ doesn't fix the issue, and i'm still kind of wondering that
15:27:51 openstack Launchpad bug 1771806 in OpenStack Compute (nova) "Ironic nova-compute failover creates new resource provider removing the resource_provider_aggregates link " [Medium,Confirmed] - Assigned to Surya Seetharaman (tssurya)
15:28:02 mriedem are you seeing the error from this condition? https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L531
15:28:44 mriedem jroll: oath is cells v2 now right?
15:28:47 mriedem + placement
15:28:56 mriedem has to be if ocata
15:29:13 jroll mriedem: yeah
15:29:15 jroll single cell
15:29:25 mriedem is that cell VMs or BMs?
15:29:38 jroll VM
15:29:38 mriedem just wondering if you've seen similar issues to what belmiro is describing above
15:29:41 mriedem oh
15:29:44 jroll still working on the baremetal upgrade
15:29:47 mriedem ok
15:30:00 jroll about to jump in a meeting but will read back shortly
15:31:01 belmoreira mriedem: I didn't hit that because in that case doesn't rebalance
15:31:40 mriedem belmoreira: sure but you don't need to rebalance to hit this check
15:31:52 mriedem the code that creates the new compute node record with the new uuid is here https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L580
15:32:02 mriedem and right before that, we check the rebalance thing https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L574
15:32:25 mriedem which will query the db for existing compute node recoreds by nodename (ironic node uuid) https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L518
15:32:34 jroll mriedem: belmoreira: at a skim, this looks like https://bugs.launchpad.net/nova/+bug/1750450
15:32:35 openstack Launchpad bug 1750450 in OpenStack Compute (nova) "ironic: n-cpu fails to recover after losing connection to ironic-api and placement-api" [Low,Fix released] - Assigned to Jim Rollenhagen (jim-rollenhagen)
15:32:35 mriedem so unless the node uuid changed, we should find the existing compute node reord
15:33:13 jroll or similar
15:33:30 mriedem similar, but that says,
15:33:31 mriedem "When ironic-api and placement-api return, nova will see nodes, create compute_node records for them, and try to create new resource providers (as they are new compute_node records). This will fail with a name conflict, and the nodes will be unusable."
15:33:42 mriedem which, unless the node uuids change, should still find the existing compute node records in the db
15:33:49 mriedem here https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L518
15:34:49 tssurya mriedem: could be we return false here : https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L511 ?
15:35:00 mriedem tssurya: not if it's the ironic driver
15:35:12 mriedem that's the only one that has rebalances_nodes = True
15:35:22 mriedem for the libvirt driver, yes
15:35:26 tssurya oh okay
15:35:47 mriedem but for the libvirt driver, you should only be able to get into this weird scenario if the physical hostname changes
15:35:51 mriedem for the compute host
15:36:12 tssurya yea understood
15:37:29 mriedem we probably want/need https://review.openstack.org/#/c/545479/ in stable branches
15:37:35 belmoreira wouldn't the logic be simpler if compute_nodes aren't recreated and only have the "host" updated
15:38:04 mriedem belmoreira: that's what that _check_for_nodes_rebalance method tries to do

Earlier   Later