Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-08-17
15:34:57 melwitt elodilles: thank you for getting stable/train unblocked. it's a miracle! 😂
15:54:19 opendevreview Balazs Gibizer proposed openstack/nova master: Reproduce bug 1986838 https://review.opendev.org/c/openstack/nova/+/853516
15:54:39 gibi sean-k-mooney: ^^ another interesting bug fall out from the pci work
15:56:44 gibi the scheduler expects the compute will fail and the compute says the scheduler should have done a better job
15:57:06 gibi but eventually nobody does its job and we have broken instances
15:57:08 gibi story of nova :D
16:08:49 elodilles melwitt: np, it was your patch anyway o:) so thanks too! :)
16:09:55 elodilles i still don't know what is causing the original problem, but at least with zuul v3 jobs the gate works \o/
16:16:36 melwitt same. glad you thought to try that patch again, I don't think I would have thought of it :)
16:19:34 elodilles well, it was not even my idea as neutron team fixed the same issue with the same solution: changing to zuul v3, so that is why i also tried it o:)
16:24:21 melwitt :D
16:53:25 sean-k-mooney gibi: i tought we had a check to prevent two alisis beign the same
16:54:08 gibi sean-k-mooney: we dont have. And with the traits in alias we can create different aliases that matches the same device from nova perspective
16:54:49 gibi but at least with placement we in the above case there would be zero allocation candidates
16:54:58 gibi so the scheduling would fail accordingly
16:55:06 gibi anyhow I will propose a simple fix that is backportable
16:55:22 gibi and on master this could never happen with placement
16:55:29 gibi after the pci in placement lands
16:55:52 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/tests/unit/pci/test_request.py#L190-L231
16:56:01 sean-k-mooney this is what i was remembering
16:56:29 sean-k-mooney we prevent the same alis beign defiend twice with confliging numa_policies or device types
16:57:04 gibi yeah we prevent defining the same alias twice
16:57:10 gibi but we allow two aliases matching the same device
16:57:18 sean-k-mooney yep the second is valid
16:57:27 sean-k-mooney what is not valid is having 2 claims agaisnt one device
16:57:50 sean-k-mooney the two alisas shoudl have created two pci request objects
16:58:12 sean-k-mooney the bug is that we dont ensure the all pci_request objects are fullfiled by unique host pci adresses
16:59:18 gibi there will be two InstancePCIRequest objects that is OK. the bug is either a) the scheduler does not fail when it fails to consume both requests b) pci_claim does not fail when fails to consume both requests
16:59:26 sean-k-mooney for example if the alias name is differnt its fine for the addres/(product_id/vendor_id) to be the same with different numa polcies
16:59:48 sean-k-mooney ya so its A
17:00:06 sean-k-mooney we should not get to the claim on the compute host if there is only one device
17:00:28 sean-k-mooney by the way the code for the schdluler and the comptue node is basically the same
17:00:38 sean-k-mooney excpt the schduler path uses a copy of the data
17:00:56 sean-k-mooney when its checking if you can consume the pci device
17:01:10 sean-k-mooney so if you fix it it will fix it for both
17:01:12 gibi but even if #a) passes as there is two free devices at that time. #b) shouldt detect and fail if a parallel claim request consumed one of the devices and the new request cannot be fulfilled
17:01:25 sean-k-mooney yes
17:01:38 sean-k-mooney but a is impletnend by calling the code for b on a copy of the data
17:01:47 sean-k-mooney so we fix the later it fixes both
17:02:27 gibi while in general the logic of the scheduler and the pci claim is the same for pci, in reality this part differs. I will push the fix and show that we need to fix this in two separate places
17:04:28 gibi I think we are the same as using _filter_pools but then that is called from two differnt paths by the two service
17:04:33 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/pci/stats.py#L622-L647
17:04:44 sean-k-mooney support_resuts just calls filter_pools
17:04:58 sean-k-mooney correct
17:05:06 sean-k-mooney on the compute node it calls consume_requests
17:05:20 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/pci/stats.py#L226-L269
17:05:21 gibi btw, https://github.com/openstack/nova/blob/master/nova/pci/stats.py#L645 is broken as it individually matches requests to pools and does not consume pools by requests
17:05:47 sean-k-mooney but fixing it in _filter_pools is what i ment by fixing it in one place to fix both
17:05:48 gibi the scheduler calls apply_requests that detects the double consumption
17:06:14 gibi sean-k-mooney: fixing it in filter_pools is tricky as filter_pools does not meant to consume things
17:06:26 gibi can be done though if needed
17:06:33 sean-k-mooney right but apply is not ment to be called in the sculer
17:06:43 sean-k-mooney it will work but we need to make sure not to commit to it on the db
17:07:00 sean-k-mooney so it will need to be a deep copy
17:07:05 sean-k-mooney with no save
17:07:31 sean-k-mooney ok it does not save
17:07:33 gibi nova.scheduler.manager.SchedulerManager._consume_selected_host
17:07:40 sean-k-mooney so as long as we do a deep copy of the pool we are good
17:07:51 gibi that is the one calling apply
17:08:07 sean-k-mooney really for multi create
17:08:14 sean-k-mooney we might already be using a copy by the way
17:08:19 gibi yes probably
17:08:22 sean-k-mooney we had to fix this in the past
17:15:00 sean-k-mooney gibi: https://opendev.org/openstack/nova/src/branch/master/nova/scheduler/host_manager.py#L309
17:15:20 gibi yepp that one
17:15:39 gibi as you said we need to consume in the scheduler to handle multi create
17:15:51 sean-k-mooney ya ok
17:16:03 sean-k-mooney the host manager has a copy of the pci stats
17:16:21 sean-k-mooney so i guess doing the apply in teh filter might be ok
17:16:40 sean-k-mooney i would have expect _locked_consume_from_request
17:16:46 sean-k-mooney to fail
17:16:57 sean-k-mooney if calling apply_requests was enough
17:17:02 gibi it fails
17:17:13 sean-k-mooney oh so we never get to the compute
17:17:18 gibi but @set_update_time_on_success catches everything
17:17:29 sean-k-mooney i tought you said we got to the compute
17:17:38 gibi https://opendev.org/openstack/nova/src/branch/master/nova/scheduler/host_manager.py#L266
17:17:50 gibi https://opendev.org/openstack/nova/src/branch/master/nova/scheduler/host_manager.py#L81
17:18:03 gibi this is where scheduler points to the compute :D
17:18:34 sean-k-mooney ok but we dont actully select the host and proced to we
17:18:50 sean-k-mooney i guess we might if noting depend on teh result of the consume
17:19:05 sean-k-mooney ok it returns nothing
17:19:11 sean-k-mooney and since that just logs
17:19:14 sean-k-mooney we continue and fail
17:19:16 sean-k-mooney on the compute
17:19:17 gibi yepp that just logs
17:19:30 sean-k-mooney ok so that should raise an instance build excption or similr
17:19:47 sean-k-mooney so we retry the next alternit host
17:20:02 sean-k-mooney well no
17:20:08 sean-k-mooney we need to fail in the filter
17:20:13 sean-k-mooney so call apply there
17:20:38 sean-k-mooney but we proably should aslo adress the fact that eats valid excptions
17:21:19 gibi and this is where compute detects the error and only logs, does not fail the instance build https://github.com/openstack/nova/blob/master/nova/pci/stats.py#L244
17:21:29 gibi it point to the scheduler :D
17:21:46 sean-k-mooney well its ment to return none
17:21:58 sean-k-mooney and the caller shoudl error but i guess it does not
17:22:31 sean-k-mooney i.e. the caller proably shoudl be checkign the lenght of the allocation to the requests
17:22:33 gibi also apply does not care about dependent devices, but consume_requests does, so we might actually need to call consume_requests
17:22:59 sean-k-mooney so suports request only exists to not call consume
17:23:09 sean-k-mooney but we proably could remvoe it can call consume

Earlier   Later