Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2024-08-13
14:30:49 dansmith also in the ceph plugin
14:31:01 sean-k-mooney oh ok then yes for others too
14:31:22 dansmith I think cinder's ceph import job might be based on it too, I forget
14:49:29 gibi_ dansmith: +A thanks for the tempest tests
14:49:36 dansmith thanks!
15:44:36 opendevreview Artom Lifshitz proposed openstack/nova master: POC: Work around lbvirt reporting offline cores as being on socket 0 https://review.opendev.org/c/openstack/nova/+/926218
16:02:42 sean-k-mooney @artom that not what we discussed doing to fix that
16:03:19 sean-k-mooney artom_: we dicussed makeign usre we first online all the cores at startup. then gather the info and then offline them when using the cpu state policy
16:04:26 gibi_ hehe, just commented that to the review:)
16:04:26 artom I don't think I was there during those discussions :)
16:04:36 artom But... the effect is the same, no?
16:05:00 gibi_ artom: nova-compute cannot assume that the pcpus are online after a restart
16:05:14 gibi_ especially as nova-compute offline them
16:05:14 artom Ah, good point.
16:06:35 sean-k-mooney artom: please also decorrage the get capablities with https://docs.python.org/3/library/functools.html#functools.lru_cache
16:06:52 sean-k-mooney the content of get_capabilities shoudl never change while the agent is running
16:07:02 sean-k-mooney we done actully need to call libvirt every time
16:07:16 artom Well that's a different thing
16:07:27 artom An improvement, but not related to fixing the bug
16:07:29 sean-k-mooney artom: we need to make this change to fix the bug
16:07:33 sean-k-mooney no its required
16:07:40 artom Explain yourself :)
16:07:47 sean-k-mooney because depenign on the livbirt verion once we turn off the cores the output will change
16:08:15 sean-k-mooney in some cases it will return the cached values form the first invocation in other version it will update and the info will be wrong
16:08:28 artom Oh, so you're saying memoize the entire return value instead of manually preserving the socket IDs?
16:08:34 sean-k-mooney yep
16:08:45 artom OK... can any other caps change while the agent is running?
16:08:56 sean-k-mooney not without breaking nova :)
16:09:37 sean-k-mooney we do not supprot cpu/memory hotplug or chaning the amount of hugepages while nova-comptue is running or chaning the cpu flags your cpu reprot at runtime
16:10:08 sean-k-mooney which is what this funciton is basicaly providing
16:10:33 sean-k-mooney if you want to do any of the above you shoudl restart nova-compute to pick up the change
16:10:50 gibi_ we basically already memoizing caps as there is a self._caps to save it and an early return
16:10:57 sean-k-mooney you proably shoudl not do any of the above on a host with instnaces either
16:11:14 sean-k-mooney oh true
16:11:22 artom There's a cache size limit, though. What happens when we hit it, and end up actually calling libvirt's caps and potentially getting the wrong socket?
16:11:53 sean-k-mooney artom: that now how that work but gibi is write
16:12:13 sean-k-mooney we are intenrlaly making this a proprty/singelton and early outing already
16:12:19 sean-k-mooney https://review.opendev.org/c/openstack/nova/+/926218/1/nova/virt/libvirt/host.py#802
16:13:54 artom I'll be honest, I understand gibi_'s concern, but the lru_cache thing is confusing me
16:14:26 sean-k-mooney pretent i didn mention it
16:14:41 artom You -1'ed the patch and are asking for it :P
16:14:53 sean-k-mooney i -1 because the rest of the patch is wrong
16:14:55 sean-k-mooney even without htat
16:15:15 artom Is addressing gibi_'s comment enough to fix it?
16:15:27 sean-k-mooney yep
16:15:30 artom \o/
16:15:52 sean-k-mooney we just neeed to make sure this work if you restart the agent multiple times
16:22:31 sean-k-mooney from an upstream point of view i think we can more or less vlaidate this with unit/funcitonal tests
16:22:44 sean-k-mooney we cant test this in tempest without modifyting the cpu toplogy fo the nodepool vms
16:22:52 sean-k-mooney we could chat to infa about that
16:23:13 sean-k-mooney we just need to hw_cpu_sockets=1 hw_cpu_threads=1
16:23:33 sean-k-mooney on the nodepool images so that we do not have multipel sockets per numa node
16:23:51 sean-k-mooney to fully test it we would also need multiple numa nodes and maybe pci
16:24:09 sean-k-mooney so realistically i think we are going to have to follow up with third party ci and manual testing
16:24:32 sean-k-mooney and jsut test this with unit/functional test for now
16:25:24 sean-k-mooney we may want to consier if we shoudl alter the libvirt fixutre to emulate the observed behavior but im not sure if that is warrented or not
16:25:30 gibi_ a functional test for upstream is OK to me and we can manual test / whitbox test is downstream
16:25:35 sean-k-mooney i feel like that will be overly invasive
16:26:08 gibi_ sean-k-mooney: yeah, growing this logic into the fixture is too much, just mock the part of the fixture in the test case to simulate it oncec
16:27:44 sean-k-mooney ack we would have to modify https://github.com/openstack/nova/blob/master/nova/tests/fixtures/libvirt.py#L808-L822
16:27:50 sean-k-mooney to be aware of the online state
16:28:12 opendevreview Stephen Finucane proposed openstack/nova master: hardware: Correct log https://review.opendev.org/c/openstack/nova/+/926223
16:28:16 sean-k-mooney ... this https://github.com/openstack/nova/blob/master/nova/tests/fixtures/libvirt.py#L808-L847
16:28:22 stephenfin sean-k-mooney: gibi: dead simple log fix there ^
16:29:15 sean-k-mooney stephenfin: ah you also explaine what -2 is
16:29:30 sean-k-mooney for a second i tough you were just droping kb
16:29:49 stephenfin yeah, I've just copied what we do in the success case
16:30:03 gibi stephenfin: I have a bugreport for that https://bugs.launchpad.net/nova/+bug/2075959
16:30:04 sean-k-mooney +1 while i wait of ci results
16:32:22 opendevreview Stephen Finucane proposed openstack/nova master: hardware: Correct log https://review.opendev.org/c/openstack/nova/+/926223
16:32:32 stephenfin gibi: Sweet. Updated with a link
16:33:27 gibi stephenfin: +2. It feels like you read my mind and fixed my low hanging todo :)
16:35:05 gibi Thanks!
16:41:43 opendevreview Artom Lifshitz proposed openstack/nova master: POC: Work around lbvirt reporting offline cores as being on socket 0 https://review.opendev.org/c/openstack/nova/+/926218
16:42:07 artom gibi, sean-k-mooney, so something like that ^^? A whole lot still missing, obviously, but the basic idea.
16:43:40 artom I wonder if we should be remembering what CPUs were offline? I feel like when we offline the dedicated ones later, that's enough?
16:45:02 gibi artom: the current logic should offline all the unused pcpus, so we can relay on that without remembering explicitly
16:45:24 artom OK, that keeps thing tidy
16:47:03 gibi artom: at a first read what you proposed seems OK to me. I need to look at it once more with a fresh set of eyes probably after we have some test results too
16:47:25 artom For sure :) Good to know that nothing obvious stands out
16:47:43 artom And there's a whitebox DNM patch running, so that should weed out any issues
16:53:52 sean-k-mooney artom: you proably should not be touching cores outside fo cpu_dedicated_set
16:54:39 sean-k-mooney we only powermanage cores in cpu_dedicated_set and its a hard error for cores in cpu_shared_set to be offline
16:55:03 sean-k-mooney but any cores out side of those two sets are valid for the admin to ower on/off as they desire
16:55:05 artom Yeah... I guess the only edge case where it might make sense is if the host has been trained, and nova-compute restarted to change the cpu_dedicated_set
16:55:08 sean-k-mooney as nova is not ment to use them
16:55:17 artom s/trained/drained/
16:55:40 artom And so some of the offline cores could be outside the _current_ dedicated set, but part of the _previous_ dedicated set
16:55:48 sean-k-mooney well not you missing the fact that core outside fo cpu_shared_set adn cpu_dedicated_set can be used for other usecases
16:56:43 sean-k-mooney artom: so it would be incorrect for use to turn on any cores not in the currnt cpu_dedicated_set
16:57:11 sean-k-mooney as long as the offline cores are not added to cpu_shared_set we dont care if they were formally in cpu_dedicated_set
16:57:28 artom We do if they're offline and we read their socket as 0 :)
16:57:34 sean-k-mooney no we dont
16:57:40 sean-k-mooney because nova will never use them
16:58:23 artom Oh... yeah? NUMA affinity but with shared CPUs is not a thing?
16:58:43 sean-k-mooney it is but we have a sperat check for cpu_shared_set to ensure they are online
16:58:59 sean-k-mooney we dont allow you to have any offlien cores in cpu_shared_set
16:59:25 artom Ah ok, in that case it's even tidier, I don't need the new power_up_unconditionally()
17:00:34 sean-k-mooney right you can resue the existing _power_up although you might want to drop the _ or add a "public" wraper
17:03:28 artom Yeah

Earlier   Later