| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2026-04-07 | |||
| 14:13:46 | sean-k-mooney | dansmith: oh ok and ya i think we can proceed with that ill loop back to it whne im finished with the cybrog team meeting | |
| 14:34:47 | MaxLamprecht[m] | sean-k-mooney: Exactly! | |
| 15:45:19 | melwitt | someone has reported a bug related to the tpool.Proxy wrapping of virNodeDevice of PCI devices during update_available_resource() https://bugs.launchpad.net/nova/+bug/2147424 saying that it causes update_available_resource() to increase time taken to complete over time. does anyone have any idea why this would be? if so, comments on the bug would be appreciated | |
| 15:48:41 | sean-k-mooney | did we fix that alreday | |
| 15:48:55 | sean-k-mooney | melwitt: that feels like something we did before | |
| 15:50:54 | melwitt | sean-k-mooney: they are saying that fix caused a different problem, that now update_available_resource() run time progressively increases over time. whereas the problem before the fix was that update_available_resource() could block other nova-compute eventlet threads. they are proposing a revert but I don't see how it could be that simple? | |
| 15:52:30 | melwitt | (i.e. wouldn't that return things to the blocked threads problem) | |
| 15:53:23 | gibi | clearly reverting until we know a bit more about the root cause of the increasing runtime feel premature to me | |
| 15:53:29 | melwitt | I don't really get how tpool.Proxy wrapping could result in a run time increase for the periodic task | |
| 15:53:47 | dansmith | ...if it increases contention | |
| 15:53:58 | gibi | if it is increases for each periodict then we probably leaking something somewhere that accumlates | |
| 15:55:10 | melwitt | yeah. they did test with and without the wrapping and said without the wrapping the run time does not increase | |
| 15:56:26 | gibi | I think we can try to reproduce the degradation. We just need to set the periodic delay to a short enough number and wait | |
| 15:58:55 | melwitt | yeah. I wonder would we need real PCI devices or can this be faked somehow? | |
| 15:59:51 | dansmith | I doubt it requires real PCI devices.. I don't think anything is actually touching the devices there, just getting the list result from libvirt | |
| 16:00:36 | gibi | we do filter for certain devices in that call | |
| 16:01:37 | gibi | https://github.com/openstack/nova/blob/b84864d9781ece8387daa967be5208b859e20d97/nova/virt/libvirt/driver.py#L8905-L8913 | |
| 16:02:15 | gibi | if the number of devs are not increasing over time in their env then that call should not increase its runtime over time either | |
| 16:07:49 | gibi | as crawlers killing our infra I cannot set up a new devstack at the moment to poke this | |
| 16:08:01 | sean-k-mooney | it shoudl in theory be repoducable in the functionalt tests | |
| 16:08:12 | gibi | but I added a comment to the bug after couple of retries | |
| 16:08:36 | sean-k-mooney | gibi: so we cache the actual devices form libvirt in that code path | |
| 16:08:45 | sean-k-mooney | so after the first call to libvirt to get the pci devices | |
| 16:08:53 | sean-k-mooney | we should be using it form memory | |
| 16:10:00 | gibi | I don't see the cache, but it is getting late here so... | |
| 16:10:01 | sean-k-mooney | but we might want ot dbouble check that we are not creating new cached copies due to self or similar breaking the caching | |
| 16:11:04 | melwitt | hm I wonder could the cache be making the tpool.Proxy threads stay around and not be cleaned up or something like that? | |
| 16:12:24 | sean-k-mooney | perhaps im trying to find where we do it today to confirm exactly how we are doing that | |
| 16:16:51 | dansmith | gibi: s/our/all/ | |
| 16:17:30 | sean-k-mooney | melwitt: im wondering if i am misser remembering and perhasp wrote a caching patch that we have not merged? | |
| 16:17:54 | gibi | dansmith: indeed | |
| 16:19:35 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/922855/6/nova/virt/libvirt/driver.py | |
| 16:21:09 | melwitt | oh ok so not yet a factor | |
| 16:21:37 | sean-k-mooney | ya | |
| 16:21:51 | sean-k-mooney | that is not the first time we have looket at that for waht its owrth | |
| 16:22:31 | sean-k-mooney | there are some other optimistaion patches like https://review.opendev.org/c/openstack/nova/+/925382 but i gouth we had merge a ttleast one o the cachign ones before | |
| 16:22:38 | sean-k-mooney | for now that not a factor i guess | |
| 16:23:04 | melwitt | I think the thing I keep focusing on is, why with tpool.Proxy only? seems the only way would be contention like Dan said | |
| 16:23:20 | sean-k-mooney | melwitt: do you know if they are seeing an increase in memory usage as well as time | |
| 16:23:39 | melwitt | I don't really know the details about how tpool.Proxy works. I pulled up the eventlet doc about it but it doesn't say much | |
| 16:24:05 | melwitt | sean-k-mooney: they did not mention memory usage but that would be good to know | |
| 16:24:37 | sean-k-mooney | if we were keeping more nad more info in memoyr over time we shoudl see htat increase. | |
| 16:24:39 | dansmith | "how it works" is "terribly" | |
| 16:24:53 | dansmith | I mean it does what it says but it's really a gross hack we should avoid whenever we can | |
| 16:25:08 | melwitt | well, that's not reassuring | |
| 16:25:12 | melwitt | (haha) | |
| 16:26:17 | dansmith | I can also see it just straight up hurting performance if we wrap an object and then have some really terrible code that hits a bunch of its properties/functions a thousand times, like in a sort routine or something | |
| 16:26:24 | sean-k-mooney | i did look tat the impl a long time ago but i dont rememebr the details | |
| 16:26:40 | gibi | ... OK it seems I got lucky and have at least an aio devstack built with PCI devices now. So I can start experimenting with the periodic | |
| 16:26:43 | dansmith | each of those would be a few instructions for regular code and with the proxy would be a spawn..wait..collect..return instead | |
| 16:27:24 | sean-k-mooney | ya so we are actully collecting the pci device 3 times per call today | |
| 16:27:28 | dansmith | unless we need the actual object to be wrapped, we should wrap the call to libvirt and return the objects, or wrap a wrapper that does the call and copies the objects into like NamedTuples or something | |
| 16:27:39 | sean-k-mooney | that is what https://review.opendev.org/c/openstack/nova/+/925382 fixes | |
| 16:28:20 | sean-k-mooney | but we also dont supprot adding/removing devices at runtime anyway without a agent restart | |
| 16:28:38 | sean-k-mooney | so we can cache this once on the first invocatoin safely today | |
| 16:28:48 | melwitt | it's that the call of virNodeDevice.listCaps() needs to be wrapped somehow to not block stuff. so my thinking was wrap the object and thus get the call wrapped? | |
| 16:29:33 | sean-k-mooney | thats in host.py right | |
| 16:30:21 | dansmith | melwitt: so we need to call listCaps() later and that makes another call into libvirt? | |
| 16:30:22 | melwitt | yes | |
| 16:30:48 | dansmith | ...and do we do that for all the returned devices? | |
| 16:31:14 | melwitt | all of the devices call their listCaps() method so I had thought so. I might be wrong | |
| 16:31:46 | dansmith | so that means each one of those listCaps() calls will be a thread spawn, wait, return with moving some data around for communication | |
| 16:31:50 | sean-k-mooney | we normaly convert them into dictonaries for pci devceis https://github.com/openstack/nova/blob/master/nova/virt/libvirt/host.py#L1593 | |
| 16:31:54 | melwitt | it's get a list of [dev, dev, dev, ...] and then for each dev: dev.listCaps() | |
| 16:32:04 | dansmith | without, those will just be sequential calls, so blocking but way faster | |
| 16:32:19 | sean-k-mooney | we try not to return the libvirt dev object out of host.py | |
| 16:32:54 | dansmith | sean-k-mooney: right converting them to dicts or NamedTuple or something all at once in a single tpool'd function would be the more efficient thing | |
| 16:33:26 | sean-k-mooney | _get_device_capabilities take sthe virNodeDevice list and converts them today for the pci devices in general | |
| 16:33:36 | sean-k-mooney | but i dont know if that pater is applied everywhere | |
| 16:33:52 | melwitt | yeah I mean maybe it's just that, that the calls are too many and too long and then the threads pile up? | |
| 16:34:20 | dansmith | not the threads piling up so much as just way more work doing it with threads | |
| 16:34:38 | sean-k-mooney | well this is 2025.1 | |
| 16:34:43 | melwitt | yeah I was thinking about the progressive increase over time | |
| 16:34:45 | sean-k-mooney | so that not using our threaded implemation | |
| 16:34:53 | sean-k-mooney | this is just for teh eventlet flow right | |
| 16:35:03 | melwitt | something has to be accumulating I thought, just not sure exactly what | |
| 16:35:21 | melwitt | yeah this should just be for eventlet | |
| 16:35:32 | sean-k-mooney | so we can create fake pci devices in the fucntional tests | |
| 16:35:41 | sean-k-mooney | and we can run the perodics explcitly in those | |
| 16:35:58 | sean-k-mooney | so it might be wroth callign it like 100 times and tiem each oen to see if we can repoduce | |
| 16:36:12 | sean-k-mooney | sort of like in my ironic simulator patch | |
| 16:36:17 | melwitt | gibi said he just got an env set up and will run some experiments | |
| 16:36:19 | sean-k-mooney | if we can we can then debug that | |
| 16:36:58 | sean-k-mooney | ack i have a env live with pci device too although i currently have the pci grouping serice deployed on taht but i can at least check my logs i gess and see what the time looks like | |
| 16:37:15 | sean-k-mooney | melwitt: the other thing of note was the fact they said more instance make it degrade faster | |
| 16:37:28 | sean-k-mooney | the number of isntance dont affect the livbirt retrival path | |
| 16:37:43 | sean-k-mooney | so i wonder if its related to the isntace objects instead or the instance part fo this | |
| 16:38:06 | melwitt | I guess I assumed more instance = more PCI devices but I guess you're saying not? | |
| 16:38:23 | sean-k-mooney | well if its the same hosts in both cases | |
| 16:38:27 | sean-k-mooney | or same server sku | |
| 16:38:31 | dansmith | sean-k-mooney: in eventlet with tpool.proxy you're creating worker threads for every property invocation on a wrapped obejct | |
| 16:38:39 | dansmith | nothing to do with our threaded implementation | |
| 16:39:05 | sean-k-mooney | dansmith: ack yes you are hookign get_atribute or simialr and dispatch the callables to a trhead pool | |
| 16:39:10 | melwitt | or yeah I guess I see. so if more instances = more other unrelated tpool.Proxy things maybe | |
| 16:39:36 | sean-k-mooney | melwitt: i think we pull all fo the domains for example in this periodic | |
| 16:39:59 | sean-k-mooney | so im wondifing if its related to the number of vms with passthough devices in some way | |
| 16:40:34 | sean-k-mooney | i.e. if on an empy host its fixed/stable but on a host with isntance it degrades over time | |
| 16:41:39 | sean-k-mooney | it is odd however that it reported against _get_pci_passthrough_devices if that was the case | |
| 16:41:43 | melwitt | I feel like it must be something less common ... like otherwise this complaint would have come up a long time ago I would think. and more people affected | |
| 16:41:59 | dansmith | if we iterate all the host pci devices once (or more) per instance, then that could be the increase that scales with instances | |