Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2026-04-07
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
16:42:26 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L8921-L8925
16:42:50 dansmith melwitt: this change happened just a year ago right? so real production people are probably just starting to run it
16:42:57 sean-k-mooney we are iterating over raw nodedev defivce form libvirt here
16:43:15 melwitt dansmith: yeah a little over a year ago
16:43:28 sean-k-mooney list_all_device here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L8913
16:43:32 melwitt but it got backported everywhere also
16:43:57 dansmith ack about the backports, that's fair
16:44:04 sean-k-mooney well no that wraped into a proxy in list_all_devices
16:44:13 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/virt/libvirt/host.py#L1744
16:44:15 melwitt I had seen a lot of positive feedback about the change (since before that people were getting nova-compute lockups) but I hadn't seen negative until today
16:44:51 dansmith yeah, I mean.. it improves one thing at the expense of another, so not surprising I think
16:45:13 dansmith and if you have fewer PCI devices or slower periodic config you probably appreciate the change :)
16:45:22 melwitt yeah. I just meant as far as, the conditions under which it happens being less common or more common
16:45:28 MaxLamprecht[m] I remember about a similar problem. See my patch and comment https://review.opendev.org/c/openstack/nova/+/922855/comment/a0361266_9c58663c/. There was a bug/performance regression in libvirt < 10.5.
16:46:35 melwitt oh interesting
16:46:39 sean-k-mooney dansmith: so we are only calling it once
16:46:47 sean-k-mooney not once per isntance as we are callign it here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L10506-L10560
16:47:20 dansmith sean-k-mooney: ack, I hadn't looked I was just saying _if_ it scales with instances and _if_ we are calling once per instance...
16:47:39 sean-k-mooney yep i was just doublecking that we were not
16:48:25 melwitt MaxLamprecht[m]: I guess though, the bug reporter did say they tested with and without tpool.Proxy wrap (assuming same libvirt version) and said they didn't get the slowdown as long as tpool.Proxy wrap was not used
16:49:27 melwitt I'm not sure if that means it could not be related. might be worth asking them on the bug what their libvirt version is regardless
16:49:45 dansmith that's expected because of the overhead it introduces.. but MaxLamprecht[m]'s change just makes us never call it again which is also an improvement regardless of the proxy
16:50:44 sean-k-mooney yes we coudl also jsut impvoe this to convert the data to dicts earlier
16:51:23 MaxLamprecht[m] ack, for us upgrading libvirt to 10.5 (which includes https://gitlab.com/libvirt/libvirt/-/commit/98f1cf88fa7e0f992d93f376418fbfb3996a9690). I would start to check this :)
16:51:44 MaxLamprecht[m] *solved it
16:51:44 sean-k-mooney list_all_devices is only uses in 2 places
16:52:26 sean-k-mooney it would not be hard to replace it with 2 function one that return the lsit of mdev names and a second that returns the capablites as a dict and just remove the tpool wrapper entirly
16:53:01 dansmith you had me at "remove the tpool wrapper entirely"
16:54:05 sean-k-mooney hum well not quite https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L1225-L1242 but that call can be moved to host.py and it does not return the obejct so there is no long lived usage

Earlier   Later