| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-03-14 | |||
| 11:01:14 | sean-k-mooney | so this is just a performance optimisation ot reduce the amount of data sent over the wire and reduce the database load on the neutron side | |
| 11:02:33 | sean-k-mooney | ishanwar: so the idea is, 1 try and use the neutron api exteion for quota details, if that works great we can skip the extra work of counting the ports ourselves, if not fall back to what we do today | |
| 11:06:09 | ishanwar | Hi Sean | |
| 11:06:21 | ishanwar | Thanks for your response | |
| 11:06:51 | ishanwar | The big mentions that the quota api seems to be controlled by Also, the extension seems to be controlled by the cfg.CONF.QUOTAS.quota_driver config option: https://github.com/openstack/neutron/blob/6fa9142ced0eed8de9a89042747438b93de7202d/neutron/extensions/quotasv2_detail.py#L37 | |
| 11:07:19 | ishanwar | So all neutron servers may not have this, how can we check the availability of the api ? | |
| 11:11:12 | sean-k-mooney | there are two way, we can assuem its there and just catch the excption when not and fall back | |
| 11:11:20 | sean-k-mooney | or we can call the extention list api | |
| 11:11:59 | sean-k-mooney | https://docs.openstack.org/api-ref/network/v2/#show-extension-details | |
| 11:12:28 | sean-k-mooney | using the extension list api is probaly better. in either case we have a convention of caching that result in nova | |
| 11:13:31 | sean-k-mooney | like this https://github.com/openstack/nova/blob/master/nova/network/neutron.py#L1435-L1455 | |
| 11:39:07 | ishanwar | thanks let me try to gather more details before create a gerrit review request | |
| 13:28:56 | opendevreview | mitya-eremeev-2 proposed openstack/nova master: Add more thorough handling of evacuated instances https://review.opendev.org/c/openstack/nova/+/944058 | |
| 13:39:28 | dansmith | gibi: I don't understand your response to my comment | |
| 13:41:06 | dansmith | _find_pool() is not where things get sorted into pools, right? It looks through the already-sorted pools and finds the one that _dev_pool (which I think is just the device, right?) matches? | |
| 13:41:14 | opendevreview | mitya-eremeev-2 proposed openstack/nova master: Remove instance directory on live-migration https://review.opendev.org/c/openstack/nova/+/944058 | |
| 13:47:26 | gibi | dansmith: yes, find_pool tries to find a matching pool, the callers of _find_pool creating a logic that if a pool is not found for a dev then a new pool is created for the dev | |
| 13:48:02 | gibi | but as matching logic was broken in _find_pool the upper layer created new pool for the devs | |
| 13:48:05 | gibi | during deallocation | |
| 13:48:21 | dansmith | ohhh, I see okay, let me follow that thread | |
| 13:49:00 | dansmith | since you're changing it and the allocation part I was mostly linking those in my head (and because _find_pool is almost like _filter_pools) | |
| 13:50:13 | dansmith | so if I comment out the _find_pool() part, I fail this part of the test: | |
| 13:50:22 | gibi | yeah the two changed code path are only linked from afar. the _find_pool runs during new device discovery and device deallocatin (putting back to pool). The _apply runs during scheduling of a VM, the consume codepath runs during the pci claim from the pci tracker in the compute | |
| 13:50:24 | dansmith | # assert that the single pool is not broken into two during the de-allocation | |
| 13:50:33 | dansmith | which makes sense based on what you said | |
| 13:50:34 | gibi | yeas | |
| 13:51:28 | gibi | and the _find_pool fix is the only thing that is necessary, the other code change is just safetynet | |
| 13:54:43 | dansmith | gibi: okay and the place where we do that (add a pool if we don't find it) is in add_device().. | |
| 13:54:55 | dansmith | it doesn't look like we run remove_device() when we consume it from the pool or anything, | |
| 13:55:19 | dansmith | so I'm not sure why we're re-running add_device() there, except maybe just as idempotentcy/healing or something? | |
| 13:56:46 | gibi | I think add_device is used both during new device discover at startup and during deallocation when we place the device back to the pool | |
| 13:57:09 | dansmith | I see it used in consume_requests() but the only other place is update_device() which isn't called from here at all | |
| 13:57:40 | dansmith | anyway, it doesn't matter that much because I see the problem now, it just seems confusing to me | |
| 13:58:13 | dansmith | however, at least it's consistent with all the other confusing terminology in here (devs, pools, dev_pools, stats, all of which are kinda the same :) | |
| 13:59:06 | gibi | _allocate_devs pops the device from the pool | |
| 13:59:17 | dansmith | we also call add_device() from _free_device() in compute manager while we're deallocating | |
| 13:59:18 | gibi | during consume_request | |
| 13:59:41 | dansmith | oh, it doesn't call remove_device() but it removes it some other way? | |
| 13:59:46 | gibi | yeah | |
| 13:59:48 | gibi | :/ | |
| 14:00:14 | sean-k-mooney | the way we test in teh schduler if a host passes is we call consume_requests on a copy of the pools and just dont save it from Supprots_requests() | |
| 14:00:21 | gibi | literarily pci_dev = pool['devices'].pop() | |
| 14:00:27 | dansmith | ack | |
| 14:00:48 | dansmith | okay so the add_device is adding it back in after it's no longer used I guess instead of just being marked as in-use... | |
| 14:00:49 | gibi | sean-k-mooney: nope, we call _apply, consume is for the pci tracker | |
| 14:01:04 | gibi | dansmith: yes that is my understanding | |
| 14:01:32 | sean-k-mooney | i may have mixed up the names but the point i wanted to make is, we test if we can allcoate the device by trying to do it on a copy of the data) | |
| 14:01:52 | gibi | sort of yes | |
| 14:03:03 | gibi | even in the scheduler (host manager) we do an allocation on the stats via apply to allow mutlicreate to see a proper resource availability | |
| 14:03:47 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/pci/stats.py#L779-L821 | |
| 14:04:01 | sean-k-mooney | yep | |
| 14:05:48 | gibi | bah we have 3 entry point not just two: | |
| 14:06:17 | gibi | 1. filters do supports_requests https://github.com/openstack/nova/blob/master/nova/scheduler/filters/pci_passthrough_filter.py#L62 | |
| 14:06:29 | gibi | 2. host manager do apply_request https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L313 | |
| 14:06:47 | gibi | 3. pci_tracker do cosume_request https://github.com/openstack/nova/blob/master/nova/pci/manager.py#L311 | |
| 14:07:27 | gibi | but you are right 1 is basically 2 on a copy of the pools | |
| 14:09:27 | sean-k-mooney | that not all of them | |
| 14:09:35 | sean-k-mooney | it kind of is but | |
| 14:09:42 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/scheduler/filters/numa_topology_filter.py#L105-L112 | |
| 14:10:08 | sean-k-mooney | numa_fit_instance_to_host also takes into acocunt the pci requests | |
| 14:10:38 | gibi | yeah that is 1 with extras, it calls support_request and look into the pools as well | |
| 14:11:40 | sean-k-mooney | ya so supprots_request is the entry point from the filters effectivly | |
| 14:11:44 | gibi | this is not for the faint-hearthed | |
| 14:12:40 | dansmith | yup | |
| 14:14:11 | sean-k-mooney | i could have sworn that we had factor the common logic out by the way | |
| 14:14:27 | sean-k-mooney | i.e. that supprot called consume or that support and consume both called apply | |
| 14:14:41 | sean-k-mooney | i dont recall why that does nto seam to be the case | |
| 14:15:18 | gibi | it needs some divergence as consume needs to return a specific device while apply only decrase pool count | |
| 14:15:30 | sean-k-mooney | oh right | |
| 14:15:37 | sean-k-mooney | consume is what we run on the compute | |
| 14:15:58 | gibi | yepp | |
| 14:16:01 | sean-k-mooney | its actully operating not just on the ppoos but the pci_device records and selecting the device | |
| 14:16:52 | dansmith | gibi: another thing that seems messy is that we take a thing, which is a dict'ing of an object, and pop out things that we ignore, and then see if "everything else is the same" right? | |
| 14:17:32 | dansmith | seems like we should opt-in to the things that distinguish them so that new things that get added in later don't cause fracturing of the pools if you don't also update the exclusion list | |
| 14:17:36 | dansmith | (or just do this differently) | |
| 14:18:26 | sean-k-mooney | that would be more maintianable wehn this was first writtin jay was pretty again adding new tag in the future | |
| 14:18:33 | sean-k-mooney | since then we have added several | |
| 14:18:47 | sean-k-mooney | btu never really revisited that orgianl approch | |
| 14:19:48 | gibi | dansmith: I think the concept is that a pool is a set of devs with a set of common field values. So it is the defintion of the interesting fields is the question here | |
| 14:20:10 | sean-k-mooney | gibi: yep | |
| 14:20:26 | gibi | and yes I share you feeling that ignoring flags is less maintainable than listing interesting flags | |
| 14:20:54 | dansmith | right | |
| 14:21:02 | gibi | I'm not motivated to change this logic in a bugfix though. I can queue up a refactor after it. | |
| 14:21:04 | sean-k-mooney | and this is what i mean when i say tis placement like a pool is effectivly an rp with and inventory fo common "fungable" dvices | |
| 14:22:06 | gibi | I would even wrap these dev and pool dicts in proper objects in memory (not ovos) for more readability (and mypy) | |
| 14:23:20 | gibi | I'm bit affraid of what can of worms this opens up | |
| 14:24:12 | gibi | and I have the ovo version bump preemptive unit test and refactor series to get back to as well | |
| 14:24:24 | gibi | I need more time for actually writing code | |
| 14:24:27 | dansmith | gibi: no I'm not suggesting you fix it in this bugfix or even later, I'm just lamenting the design | |
| 14:24:35 | gibi | dansmith: OK | |
| 14:25:10 | dansmith | _especially_ if it means more mypy in this code :) | |
| 14:26:03 | gibi | sh*t I have this from the PCI in Placement work remaining as well https://review.opendev.org/q/topic:%22bp/pci-device-spec-cleanup%22 (danger, it contain mypy changes as well) | |
| 14:26:39 | dansmith | going on three years old there... | |
| 14:26:52 | gibi | yeah it is sad | |
| 14:27:21 | sean-k-mooney | ya... | |
| 14:27:46 | sean-k-mooney | this https://review.opendev.org/c/openstack/nova/+/844491 i think was one fo the more valuable changes | |
| 14:28:28 | sean-k-mooney | although the seise of a whole impoved alot of things | |
| 14:30:48 | gibi | maybe in Flamingo :) | |
| 14:31:02 | gibi | less golang more python :) | |
| 14:33:39 | sean-k-mooney | :) | |