| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-05 | |||
| 13:54:38 | efried | - Subtract the second set from the first. This is the set of providers you need to delete. | |
| 13:54:38 | efried | - Construct a set of all providers you *expect* there to be - i.e. the ones you've discovered from the host_data. | |
| 13:54:38 | efried | - Construct a set of all providers in the tree whose names start with your prefix (GPUG_ or VGPU_ or whatever it was). | |
| 13:54:38 | efried | naichuans: not having looked at your update yet, the way I figured you would do it is: | |
| 13:54:45 | stephenfin | gotcha | |
| 13:55:08 | dansmith | stephenfin: there's another one in another file under libvirt/ there that also fails occasionally, but they seem to be unrelated | |
| 13:56:50 | dansmith | stephenfin: is that message indicating that some mock for the system's pinnable CPUs is not set and so it's empty? or something? | |
| 13:57:20 | stephenfin | That message indicates you're trying to pin an instance but the CPUs aren't available | |
| 13:57:29 | stephenfin | Generally because they're pinned to something else | |
| 13:57:31 | naichuans | efried: OK, looks a acceptable choice, thanks. | |
| 13:57:48 | dansmith | stephenfin: that list will be empty like that if the cpus are pinned elsewhere? | |
| 13:57:57 | stephenfin | Yup | |
| 13:58:40 | dansmith | so like another test not having deleted a server or something? | |
| 13:58:43 | mriedem | stephenfin: if you want, i can address my nits in https://review.openstack.org/#/c/541290/ | |
| 13:58:45 | dansmith | although they should all be getting an empty db | |
| 13:58:50 | stephenfin | You're saying I want to take CPUs Y from the total host set of X | |
| 13:59:00 | stephenfin | mriedem: Go for it | |
| 13:59:05 | stephenfin | mriedem: and thanks | |
| 14:00:05 | stephenfin | dansmith: It could also be that we're booting two instances consecutively from different tests | |
| 14:00:17 | dansmith | but they should have different databases and not see each other | |
| 14:00:19 | stephenfin | except, yeah, each test gets its own sqlite DB | |
| 14:00:20 | stephenfin | Hmm | |
| 14:00:52 | mriedem | do the fake hosts/nodes in the tests have the same name? | |
| 14:01:02 | mriedem | because the nova.tests.unit.virt.fake set_nodes or whatever is global | |
| 14:01:15 | mriedem | one of the tests might not be resetting the fake node on cleanup? | |
| 14:01:19 | dansmith | yeah | |
| 14:01:49 | dansmith | I don't see them using fake set_nodes | |
| 14:01:54 | dansmith | unless it's somewhere else | |
| 14:02:00 | mriedem | parent class? | |
| 14:02:42 | dansmith | I don't think so.. they | |
| 14:02:49 | dansmith | are starting their own compute service for some reason | |
| 14:02:58 | dansmith | maybe so they can control the topology | |
| 14:03:07 | stephenfin | dansmith: Yeah, that | |
| 14:04:12 | stephenfin | That's stored in nova.objects.compute_node.ComputeNode.numa_topology, which I think is created on startup for nova-compute | |
| 14:04:51 | dansmith | and they don't do fake node set in the parent class(es) | |
| 14:06:33 | tssurya | mriedem: yea that spec is going crazy | |
| 14:06:49 | dansmith | stephenfin: hmm, just got two failures from that module in one run, so something is definitely wonky there | |
| 14:07:03 | stephenfin | dansmith: Yeah, I'm taking a look at it now | |
| 14:07:15 | dansmith | I really can't think of why my patch would be affecting this kind of thing | |
| 14:09:41 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Add 'numa-aware-vswitches' spec https://review.openstack.org/541290 | |
| 14:09:57 | mriedem | stephenfin: fyi https://review.openstack.org/#/c/541290/17..18/specs/rocky/approved/numa-aware-vswitches.rst | |
| 14:10:33 | mriedem | stephenfin: also, i haven't gone through the code series in detail yet, but it would be best if the api changes for this come at the end of the series | |
| 14:10:37 | mriedem | so build everything from the bottom up | |
| 14:10:51 | stephenfin | mriedem: Which API? | |
| 14:10:55 | stephenfin | The neutron one? | |
| 14:10:58 | mriedem | i.e. create_pci_requests_for_sriov_ports populating InstanceNUMANetworks to set in the RequestSpec comes last | |
| 14:11:01 | stephenfin | gotcha | |
| 14:11:04 | mriedem | yeah | |
| 14:11:12 | mriedem | then nothing gets 'turned on' until the end | |
| 14:11:43 | dansmith | oh, I bet I know something related | |
| 14:11:46 | openstackgerrit | Naichuan Sun proposed openstack/nova master: xenapi(N-R-P): Add API to support vgpu resource provider create https://review.openstack.org/520313 | |
| 14:14:36 | dansmith | stephenfin: I'm calling update_available_resource() now after build success or failure.. is there something that might not be set up that would prevent that from working or something? | |
| 14:16:40 | stephenfin | dansmith: Well, reverting that chunk "fixes" things | |
| 14:16:49 | dansmith | the update? | |
| 14:16:52 | stephenfin | aye | |
| 14:17:01 | stephenfin | at least, locally | |
| 14:17:19 | stephenfin | I've run it a few times and it passed each time with the HEAD~ version of that file | |
| 14:17:38 | dansmith | I see it failing during _delete_server, FWIW, but I wonder if delete just fails because the create left something in a bad state | |
| 14:18:02 | dansmith | stephenfin: so....the update fails because we have a half-created instance with topology that we can't satisfy, yeah? | |
| 14:18:21 | dansmith | seems like we'd hit this in real life if that's the case, if a periodic ran during a failed schedule or something | |
| 14:18:31 | dansmith | because that update call happens all over the place | |
| 14:19:01 | stephenfin | Like you said, it could be a buggy test too | |
| 14:19:55 | dansmith | well, I'm not really sure where to go from here | |
| 14:20:31 | dansmith | this is kinda like the bug we saw with the DiskNotFound, | |
| 14:20:45 | dansmith | where that error for a single instance will block nova from updating the RT for everything on the host | |
| 14:21:00 | stephenfin | Hmm, now I can't get the tests to _fail_ again | |
| 14:21:01 | dansmith | so maybe we should catch this exception somewhere, log it at error and allow the RT to proceed? | |
| 14:21:30 | mriedem | the functional tests shouldn't need to do their own delete server, unless they are intentionally trying to make sure the server is gone - because the test tearDown will wipe the db | |
| 14:21:44 | dansmith | mriedem: yeah | |
| 14:22:09 | dansmith | removing that might make the test failure go away, but it feels kinda dirty | |
| 14:23:46 | dansmith | although the test checks that the instance goes active, so.. | |
| 14:23:54 | openstackgerrit | Naichuan Sun proposed openstack/nova master: xenapi(N-R-P): Add API to support vgpu resource provider create https://review.openstack.org/520313 | |
| 14:24:25 | mriedem | well, the test waits for the server to be != 'BUILD' | |
| 14:24:29 | mriedem | which has always bugged me | |
| 14:24:39 | mriedem | oh update_available_resource_for_node | |
| 14:24:40 | mriedem | oops | |
| 14:24:51 | mriedem | end_status='ACTIVE' i see | |
| 14:25:14 | stephenfin | dansmith: Yeah, I think that's what's happening | |
| 14:25:21 | dansmith | stephenfin: what? | |
| 14:25:26 | mriedem | what is calling update_available_resource_for_node ? | |
| 14:25:43 | dansmith | mriedem: update_available_resource() is, which is what I'm calling from build_success/failure | |
| 14:25:44 | stephenfin | dansmith: you're now calling 'update_available_resource' which iterates through instances to extra their numa_topology | |
| 14:25:58 | stephenfin | and updates utilization based on that | |
| 14:26:07 | dansmith | stephenfin: oh and hitting the incomplete instance or something? | |
| 14:26:17 | stephenfin | yeah, I'd imagine so | |
| 14:26:24 | dansmith | so that's a bug in the actual code then, yeah? | |
| 14:26:25 | mriedem | seems weird that the stacktrace in the test failure doesn't include _build_succeeded | |
| 14:26:38 | dansmith | mriedem: yeah, idk why that is | |
| 14:26:43 | openstackgerrit | Naichuan Sun proposed openstack/nova master: xenapi(N-R-P):Get vgpu info from `allocations` https://review.openstack.org/521717 | |
| 14:27:06 | openstackgerrit | Naichuan Sun proposed openstack/nova master: xenapi(N-R-P): support compute node resource provider update https://review.openstack.org/521041 | |
| 14:27:35 | dansmith | so removing the _delete_server call from that test seems to make it happy, but I think it's still failing to do that update under the covers, we're just not seeing it or whatever | |
| 14:29:03 | dansmith | yeah, so I get a NUMAServersTest failure now in _delete | |
| 14:29:25 | stephenfin | dansmith: This line is probably getting called multiple times https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L948-L951 | |
| 14:30:04 | dansmith | that's multiple lines.. do you mean L949? | |
| 14:30:06 | stephenfin | So we're double freeing, kind of. This is starting to feel familiar but I'm not sure why | |
| 14:30:10 | stephenfin | yup | |
| 14:30:35 | dansmith | okay, but that shouldn't be a problem with repeated calls to update_available_resource()... since it starts fresh when it does that right? | |
| 14:32:16 | stephenfin | Well if compute is thinking the instance is gone, it could be freeing on the first call to that function (the one you added) and then attempting to do so again on our call (the API call to delete) | |
| 14:32:25 | stephenfin | which triggers this, I imagine? https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1127 | |
| 14:32:42 | dansmith | mriedem: you know, unrelated (kinda) to this test failure, we could just increase the counter and depend on the next periodic run of update_available_resources() to update our counter, | |