| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-05-07 | |||
| 06:56:40 | opendevreview | Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes https://review.opendev.org/c/openstack/nova/+/945204 | |
| 06:58:22 | ishanwar[m] | <ishanwar[m]> "Hi Everyone..." <- I want to get this merged so need your help | |
| 09:25:00 | sean-k-mooney | ishanwar[m]: the failed indicate taht your missing mocking | |
| 09:26:17 | sean-k-mooney | you added | |
| 09:26:19 | sean-k-mooney | @mock.patch( | |
| 09:26:21 | sean-k-mooney | "nova.network.neutron.API.has_quota_details_extension", | |
| 09:26:23 | sean-k-mooney | new=mock.Mock(return_value=True), | |
| 09:26:25 | sean-k-mooney | ) | |
| 09:26:27 | sean-k-mooney | but you only did that to your test | |
| 09:26:43 | sean-k-mooney | the function you added is called in several other test that do not have the mock in place | |
| 09:27:35 | sean-k-mooney | so for the 5 failing test you also need to mock the call to has_quota_details_extension | |
| 12:05:37 | gibi | sean-k-mooney: dansmith: FYI I noted some memory consumption numbers with the reduced scatter-gather pool size in https://review.opendev.org/c/openstack/nova/+/948450/comments/9ff461df_ec6381b2 It seems we have a good handle on managing memory consumption by managing the pool size. | |
| 12:07:27 | gibi | 40 threads -> 122MB RSS, 5 threads -> 85MB RSS, eventlet based scheduler 56MB RSS | |
| 12:14:12 | sean-k-mooney | cool that looks reasonable. although we are planning to have multiple pools in some cases | |
| 12:14:34 | sean-k-mooney | i guess for conductor api and schduelr we likely will only need the scater gater pool | |
| 12:14:50 | sean-k-mooney | and for the compute we will only need the io one | |
| 12:14:57 | sean-k-mooney | out side of the oslo.messaging one | |
| 12:15:21 | sean-k-mooney | so we are proably fine | |
| 12:16:18 | sean-k-mooney | i belive in my paches i reused the generic io threadpool for ceph but if we are removing that expict usage anyway and leavign the librbd/rados package handel that internally then that fine too | |
| 12:16:29 | gibi | yeah currently I have no data abou the pool in oslo.messaging. But obviously that is changed too as soon as we switched off monkey patching | |
| 12:17:06 | gibi | but we have a config option to limit that pool too (default is 64) | |
| 12:17:09 | sean-k-mooney | i assume that if that is configurable it will be expsoed as an oslo.messaging config option which we will jsut inherit | |
| 12:17:15 | gibi | yepp | |
| 12:17:17 | gibi | it is | |
| 12:17:25 | sean-k-mooney | cool | |
| 12:17:47 | sean-k-mooney | 64 seam a little higher then i woudl expect but we can role with it and see | |
| 12:18:34 | sean-k-mooney | for the conductor or schduler i guess that might be reasonable to have 64 RPCs in flight at once | |
| 12:18:53 | sean-k-mooney | the api and comptue dont really need it that large | |
| 12:19:29 | sean-k-mooney | tooling and admins can tune it however once we gain experince iwth the impact on scaling | |
| 12:23:46 | gibi | yepp | |
| 12:23:55 | gibi | I will include a list of tunables in a new doc | |
| 15:40:04 | opendevreview | Balazs Gibizer proposed openstack/nova master: Translate scatter-gather to futurist https://review.opendev.org/c/openstack/nova/+/947966 | |
| 15:40:05 | opendevreview | Balazs Gibizer proposed openstack/nova master: Use futurist for _get_default_green_pool() https://review.opendev.org/c/openstack/nova/+/948072 | |
| 15:40:05 | opendevreview | Balazs Gibizer proposed openstack/nova master: Replace utils.spawn_n with spawn https://review.opendev.org/c/openstack/nova/+/948076 | |
| 15:40:06 | opendevreview | Balazs Gibizer proposed openstack/nova master: Add spawn_on https://review.opendev.org/c/openstack/nova/+/948079 | |
| 15:40:06 | opendevreview | Balazs Gibizer proposed openstack/nova master: Move ComputeManager to use spawn_on https://review.opendev.org/c/openstack/nova/+/948186 | |
| 15:40:07 | opendevreview | Balazs Gibizer proposed openstack/nova master: Move ConductorManager to use spawn_on https://review.opendev.org/c/openstack/nova/+/948187 | |
| 15:40:07 | opendevreview | Balazs Gibizer proposed openstack/nova master: Make nova.utils.pass_context private https://review.opendev.org/c/openstack/nova/+/948188 | |
| 15:40:08 | opendevreview | Balazs Gibizer proposed openstack/nova master: Rename DEFAULT_GREEN_POOL to DEFAULT_EXECUTOR https://review.opendev.org/c/openstack/nova/+/948086 | |
| 15:40:09 | opendevreview | Balazs Gibizer proposed openstack/nova master: Make the default executor configurable https://review.opendev.org/c/openstack/nova/+/948087 | |
| 15:40:09 | opendevreview | Balazs Gibizer proposed openstack/nova master: Print ThreadPool statistics https://review.opendev.org/c/openstack/nova/+/948340 | |
| 15:40:11 | opendevreview | Balazs Gibizer proposed openstack/nova master: WIP: allow service to start with threading https://review.opendev.org/c/openstack/nova/+/948311 | |
| 15:40:11 | opendevreview | Balazs Gibizer proposed openstack/nova master: DNM:Run nova-next with n-sch in threading mode https://review.opendev.org/c/openstack/nova/+/948450 | |
| 19:58:22 | opendevreview | Balazs Gibizer proposed openstack/nova master: Add spawn_on https://review.opendev.org/c/openstack/nova/+/948079 | |
| 19:58:22 | opendevreview | Balazs Gibizer proposed openstack/nova master: Move ComputeManager to use spawn_on https://review.opendev.org/c/openstack/nova/+/948186 | |
| 19:58:23 | opendevreview | Balazs Gibizer proposed openstack/nova master: Move ConductorManager to use spawn_on https://review.opendev.org/c/openstack/nova/+/948187 | |
| 19:58:23 | opendevreview | Balazs Gibizer proposed openstack/nova master: Make nova.utils.pass_context private https://review.opendev.org/c/openstack/nova/+/948188 | |
| 19:58:24 | opendevreview | Balazs Gibizer proposed openstack/nova master: Rename DEFAULT_GREEN_POOL to DEFAULT_EXECUTOR https://review.opendev.org/c/openstack/nova/+/948086 | |
| 19:58:24 | opendevreview | Balazs Gibizer proposed openstack/nova master: Make the default executor configurable https://review.opendev.org/c/openstack/nova/+/948087 | |
| 19:58:25 | opendevreview | Balazs Gibizer proposed openstack/nova master: Print ThreadPool statistics https://review.opendev.org/c/openstack/nova/+/948340 | |
| 19:58:25 | opendevreview | Balazs Gibizer proposed openstack/nova master: WIP: allow service to start with threading https://review.opendev.org/c/openstack/nova/+/948311 | |
| 19:58:26 | opendevreview | Balazs Gibizer proposed openstack/nova master: DNM:Run nova-next with n-sch in threading mode https://review.opendev.org/c/openstack/nova/+/948450 | |
| #openstack-nova - 2025-05-08 | |||
| 02:28:56 | opendevreview | melanie witt proposed openstack/nova-specs master: Add reminder about Tempest API response schemas https://review.opendev.org/c/openstack/nova-specs/+/949113 | |
| 02:30:24 | melwitt | Uggla, gmaan: this is a spec template update to add a note about API response schemas in Tempest ^ | |
| 02:30:43 | melwitt | (fyi) | |
| 10:02:04 | opendevreview | Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes [FIX]: added additional unit tests changes to mock validate_networks https://review.opendev.org/c/openstack/nova/+/945204 | |
| 10:04:01 | ishanwar[m] | opendevreview: sean-k-mooney: made a few changes as your requests UTs seems to be working fine now kindly have a look | |
| 11:20:13 | sean-k-mooney | gmaan: question for you https://docs.openstack.org/nova/latest/user/flavors.html | |
| 11:20:39 | sean-k-mooney | we currently default group_policy to non if request groups come form neutron ports | |
| 11:21:06 | sean-k-mooney | but we require group_policy to be passed if you pass multiple groups in the flavor | |
| 11:21:36 | sean-k-mooney | it almost never makes sesne to use isolate because it will break port based request if you ever want more then one port form the same nic | |
| 11:21:43 | sean-k-mooney | i.e. 2 ovs prot or 2 sriov ports | |
| 11:21:52 | sean-k-mooney | so we never recommend using that in general | |
| 11:22:14 | sean-k-mooney | so my question is if we jsut want to extend the defaulting to None to the falvor | |
| 11:22:50 | sean-k-mooney | can tha tbe done without a microverison. i think the answer is know althoguh im hopeing i can argure that we coudl because we are relaxing the constraint | |
| 11:29:00 | sean-k-mooney | its a trivial spec to write if we decied we need a microversion for tha that but honestly i thinik havign server create fail if a user uses the old microvtion with a flavor and pass with the new versiuon and exactly the same flavor is terrible ux | |
| 11:29:46 | sean-k-mooney | we have previouysly said that new extra specs dont need a microvetion and that changing the default shoudl not either | |
| 11:30:16 | sean-k-mooney | so i think we can do this withou one simiarl to the fix for neutron prots https://github.com/openstack/nova/commit/ad4f79836264ec79068f087008367e2d2ae8cbea | |
| 11:31:12 | sean-k-mooney | gibi: ^ you wrote that for the neutron port qos policy i belive do you see any reaons not to extend the defauling to none ot flavor based groups too. | |
| 11:34:13 | ishanwar[m] | <ishanwar[m]> "sean-k-mooney: made a few..." <- Build has passed as well. I think it looks good now | |
| 11:37:08 | sean-k-mooney | ishanwar[m]: i ment to push my comment a while ago but its going to fail our code style checks | |
| 11:37:53 | sean-k-mooney | ishanwar[m]: i got pinged for soemthign else so i didnt finish the review but i pushed my comment about the whitespace sissue | |
| 11:37:58 | ishanwar[m] | sean-k-mooney: pep checks seem to have passed though | |
| 11:37:58 | ishanwar[m] | sorry new to this | |
| 11:38:01 | sean-k-mooney | i think i agree that this is good to go | |
| 11:38:31 | sean-k-mooney | you shoudl stil fix https://review.opendev.org/c/openstack/nova/+/945204/6/nova/tests/unit/network/test_neutron.py#4101 | |
| 11:38:49 | sean-k-mooney | but i think the logic is correct | |
| 11:39:31 | ishanwar[m] | sean-k-mooney: yeah I have double checked | |
| 11:39:31 | ishanwar[m] | let me fix the style | |
| 11:39:31 | ishanwar[m] | i'll ping back | |
| 11:40:53 | opendevreview | Stephen Finucane proposed openstack/nova master: wsgi: Don't create, use lock in same line https://review.opendev.org/c/openstack/nova/+/949161 | |
| 11:45:00 | sean-k-mooney | ishanwar[m]: so running ti locally it fails both the trim trailign spaces check adn the lighleght check when i run tox -e pep8 | |
| 11:45:33 | sean-k-mooney | ishanwar[m]: but it also fixes if for you | |
| 11:46:14 | sean-k-mooney | well it fixes the white space you will need to manually fix the linelenght issue | |
| 11:53:29 | gibi | sean-k-mooney: I think if the groups are coming from the flavor then we can simply ask the flavor creator to add policy according to the needs. I.e. if they want two vgpus from two different pgpus and they won't use qos ports then it is a case when isolate policy is needed. For the neutorn port case we need a defaulting as the person creating the port might not be the same as the one creating the | |
| 11:53:35 | gibi | flavor and we dont support adding policy to the port | |
| 11:57:23 | sean-k-mooney | gibi: yes im aware | |
| 11:57:30 | sean-k-mooney | we currently require you to set it | |
| 11:57:43 | sean-k-mooney | but i think it woudl be a better ux if we jsut defautl to none | |
| 11:58:01 | sean-k-mooney | partly because i honestly think we shoudl delete the suprpot for group_polcy in the flavor in its currnt form | |
| 11:58:23 | sean-k-mooney | i know its possibel to use isloate i fyou know exactly how the flavor will be used | |
| 11:59:07 | sean-k-mooney | i.e. if you have a flavor for a specific aplication that that will only be used with prots that either dont hav eresouce request or wont be fullfield form the same rp | |
| 11:59:45 | sean-k-mooney | but i think adding it to the flavor in its current form was a mistake so if we can avoid admin ever settign it by defaulting to None | |
| 11:59:53 | sean-k-mooney | it hink that will server better longterm | |
| 12:00:31 | sean-k-mooney | gibi: there is a similar but diffent issue with it too in that it breakes our filter because it not namespaced | |
| 12:00:49 | sean-k-mooney | stephen and i fixed that in train for the only other extra spec that did not have a namespace | |
| 12:01:23 | sean-k-mooney | we created a namespaced version as an alias and maded the non name "deprecated" | |
| 12:01:58 | sean-k-mooney | i think we shoudl do the same here and add group:policy=isolate|none | |
| 12:02:32 | sean-k-mooney | but again that for a slightly diffent reasons | |
| 12:03:23 | sean-k-mooney | until we can supprot expressign the policy betwen sepcific groups to me isolate is not practiclly usable unless you a nova/placment core or have spent a very very long time lookign at all the details | |