Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-08-18
10:14:23 gibi yes
10:14:34 gibi so I will keep the support call to do the apply on a copy
10:14:49 gibi and keep the apply work on the shared stats
10:14:56 sean-k-mooney you could make supports call apply internally on a copy
10:15:23 gibi but then I need to signal to apply when to copy and when not to copy
10:15:47 sean-k-mooney i have not looked at the signiture but do we pass in the pools to apply
10:15:55 sean-k-mooney or does it get them itself
10:16:03 sean-k-mooney i was assumign we passed them in
10:16:11 sean-k-mooney so supports could do the copy for it
10:17:02 sean-k-mooney _apply takes the pools https://github.com/openstack/nova/blob/master/nova/pci/stats.py#L622-L653
10:17:48 sean-k-mooney its up to you which you think is cleaner
10:17:50 gibi yes, but apply_requests doesnt
10:17:58 gibi anyhow I will push the code soon
10:17:59 sean-k-mooney let me know when its ready to review
10:18:02 gibi and we can look at that
10:18:03 sean-k-mooney cool
10:18:09 gibi thanks for the discussion
10:24:40 sean-k-mooney im going to try an power through and finish the vdpa patches today just an fyi
10:24:48 sean-k-mooney its mainly just tests and docs at this point
10:24:50 opendevreview efineshi proposed openstack/python-novaclient master: Fix nova host-evacuate won't work with hostname is like a.b.c https://review.opendev.org/c/openstack/python-novaclient/+/853465
10:25:15 sean-k-mooney althouhg i have one downstream bug to look at first...
10:27:07 gibi sean-k-mooney: sure I can review the rest of the vdpa series when it is ready
10:29:29 opendevreview Balazs Gibizer proposed openstack/nova master: Trigger reschedule if PCI consumption fail on compute https://review.opendev.org/c/openstack/nova/+/853611
10:35:24 opendevreview Balazs Gibizer proposed openstack/nova master: Trigger reschedule if PCI consumption fail on compute https://review.opendev.org/c/openstack/nova/+/853611
12:31:16 opendevreview Merged openstack/nova master: imagebackend: default by_name image_type to config correctly https://review.opendev.org/c/openstack/nova/+/826526
12:31:23 opendevreview Merged openstack/nova master: image_meta: Add ephemeral encryption properties https://review.opendev.org/c/openstack/nova/+/760454
12:31:31 opendevreview Merged openstack/nova master: BlockDeviceMapping: Add encryption fields https://review.opendev.org/c/openstack/nova/+/760453
12:31:39 opendevreview Merged openstack/nova master: BlockDeviceMapping: Add is_local property https://review.opendev.org/c/openstack/nova/+/764485
12:31:47 opendevreview Merged openstack/nova master: compute: Update bdms with ephemeral encryption details when requested https://review.opendev.org/c/openstack/nova/+/764486
12:47:18 opendevreview sean mooney proposed openstack/nova master: add sorce dev parsing for vdpa interfaces https://review.opendev.org/c/openstack/nova/+/841016
12:52:10 opendevreview sean mooney proposed openstack/nova master: add sorce dev parsing for vdpa interfaces https://review.opendev.org/c/openstack/nova/+/841016
13:49:52 dansmith gibi: can you look at my reply here real quick? https://review.opendev.org/c/openstack/nova/+/852900
13:50:13 dansmith if you agree that I need to chase down the tests. that fail because of shared state, I'll give that a shot
13:50:33 gibi dansmith: sure I will look in 5
13:50:35 dansmith but if you have some other idea about why that might be, I'll be glad to have it before getting into that rabbit hole :)
13:50:37 dansmith thx
13:53:15 gibi OK I need to run it locally to see the issues
13:55:12 dansmith the libvirt reshape test is the one I remember
13:58:35 dansmith hmm, maybe resetting the client during restart_compute_service is all I need
13:58:46 dansmith I need to run a full set now to see if the reshape ones are the only ones
14:00:13 dansmith what I really wanted to say in that comment was something like "this compute service stack is where we use a client with all the complex internal state and thus maybe we shouldn't share that with anything else that isn't part of this set of objects"
14:00:23 dansmith which is maybe still a good idea, I dunno
14:00:43 dansmith but re-using the same init code has the benefit of the similar error messages and things
14:02:58 gibi hm restart_compute_service could will trigger a creation of a new ComputeManager instance
14:03:05 gibi -could
14:03:51 gibi so you are right that behaves differently in the test where the two ComputeManager will share report client state, from the reality where a compute restart will reset the client
14:03:58 gibi s/reset/recreate/
14:04:26 dansmith yeah, so I put a call there to reset the global state and it passed the few reshape tests I had in my history
14:04:29 dansmith running the full set now
14:05:32 gibi buut, in func test we run all of our computes in the same process, so now they will share the same report client across compute hosts
14:06:09 dansmith will that work because of the multi-root functionality you mentioned?
14:06:23 sean-k-mooney i kind of feel like we shoudl seperate the singelton changes form the lazy loading
14:06:41 sean-k-mooney if you have not already done that
14:06:50 dansmith but as I said above, I'm also fine keeping them separate for the compute manager part if you think that's better
14:06:52 sean-k-mooney just so there is less change to condier
14:06:54 gibi dansmith: I'm not sure about the scope of the multi root functionality
14:07:12 dansmith and I'll try to write something more concise than my verbose sentence above, but more useful than the one I put in there that caused the confusion
14:07:49 dansmith sean-k-mooney: they're already together, and can't be separated without losing some of the behavior that gibi wanted to keep
14:07:50 gibi I think in short term keep a separate client per ComputeManager instance and note why we are doing it.
14:08:05 dansmith gibi: ack, sounds good
14:08:40 gibi I dont like the global but I checked and in most cases it is safe based on how we use it
14:08:52 gibi the compute manager is an exception
14:09:14 gibi at least due to the func test, but also might be due to ironic multiple node per compute
14:09:52 dansmith the global results in fewer hits to keystone and also fewer places we could fail if a call to keystone fails, and mirrors our other client behaviors
14:09:59 dansmith (the internal state does not, of course, but...)
14:10:16 gibi yeah I accept the compromise
14:10:24 gibi the global has pros and cons
14:10:35 dansmith if you really hate the global I can switch it to per-use lazy load
14:10:48 dansmith but aside from the state thing, I don't know why we would perfer that
14:11:02 gibi yeah, only the share state that makes it complicated.
14:11:05 dansmith (or prefer even)
14:11:45 dansmith ack, so we could also make each call to get the singleton generate a new local state object so that that part is not shared everywhere
14:11:57 dansmith I could leave a node in there with the idea in case it becomes problematic in the future
14:12:05 gibi if I had time I would also trim the report client to have only those methods there that depend on the shared state and move the independent functions somehere else to make it clear what is problematic
14:12:30 dansmith ack, there are several things we *could* do to make this cleaner for sure
14:12:49 gibi the extra note in the singletone works for me
14:13:31 dansmith ack will do that, no singleton for the compute manager and add that test
14:13:50 gibi ack
14:13:51 gibi thanks
14:17:43 gibi sean-k-mooney: I moved forward with the allocation candidate filtering in hardware.py step. That made me realize two things 1) placement tells us RP uuids in the allocation candidates and the stats hardware.py code needs to map those RP uuids to pools. For that I need extra information in the pool. 2) the prefilters are run in the scheduler so creating RequestGroups there are good for scheduling but
14:17:49 gibi not good for using those RequestGroups (especiall the provider mapping in them) later to drive the PCI claim as those RequestGroups are not visible outside of the scheduler process. For the QoS port we create the groups in the compute.api so those are visible to the conductor and the compute
14:19:05 sean-k-mooney i thinik for cpu pinning we do it else where too
14:19:43 gibi as prefilters are acting on the RequestSpec I'm tempted to move the prefilter run to the conductor
14:20:05 gibi I can try and see what falls out
14:20:12 gibi how do you feel about it?
14:20:21 sean-k-mooney moving part of the schduleing out of the schduler
14:20:29 sean-k-mooney that feel like more then we need to adress this
14:20:48 gibi alternatively I can try to return the modified request spec from the scheduler to the conductor
14:20:55 gibi but that is an RPC change
14:20:59 gibi afaik
14:21:29 sean-k-mooney im looking to see what we do for PMEM and cpu pinnign i rememebr we did it differntly for one of them
14:21:36 sean-k-mooney before the prefileters
14:22:34 sean-k-mooney im fine with moving the request group creation to before the call to the schduler
14:22:56 sean-k-mooney moving running the prefilters to the conductor however i think it probaly mre then we want to do
14:23:06 gibi ack, that would be another alternative, to do the request group generation not in a prefilter
14:23:23 gibi that would make this similar to how QoS works
14:28:15 sean-k-mooney this is where we do it for cpu pinning https://github.com/openstack/nova/blob/e6aa6373d98103348a8ee3c59814350ea1556049/nova/scheduler/utils.py#L80
14:29:15 gibi I think that also runs in the scheduler
14:30:20 sean-k-mooney the implmenation i think i sin the request spec object
14:30:42 sean-k-mooney oh its not

Earlier   Later