| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-18 | |||
| 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 | |
| 14:30:45 | sean-k-mooney | https://github.com/openstack/nova/blob/e6aa6373d98103348a8ee3c59814350ea1556049/nova/scheduler/utils.py#L305-L321 | |
| 14:31:14 | sean-k-mooney | but those are free standing fucntions that budil up the resouce class request | |
| 14:31:28 | gibi | so that works as you never need to know from where the VPMEM resource was fulfilled | |
| 14:32:01 | sean-k-mooney | ya | |
| 14:32:25 | gibi | I will figure out something along the line of cyborg and qos requests | |
| 14:32:50 | gibi | both needs the request groups after the scheduling to drive the claim on the compute | |
| 14:33:50 | sean-k-mooney | ya | |
| 14:34:09 | sean-k-mooney | we do have some pci affintiy code there by the way added by https://github.com/openstack/nova/commit/db7517d5a8aaa5a24be12d9c3453dcd98d9a887e | |
| 14:34:59 | gibi | yeah that also only extends the unsuffixed request group | |
| 14:35:17 | sean-k-mooney | yep its just finding a host that supports it | |
| 14:35:35 | sean-k-mooney | but you would potentally have to do that per pci device now | |
| 14:35:44 | sean-k-mooney | or at least eventually | |
| 14:35:59 | sean-k-mooney | since the policy is setable per alias | |
| 14:36:17 | sean-k-mooney | we can proably pretend i did not mention this for now :) | |
| 14:36:38 | sean-k-mooney | we said in the spec that we would leave numa to the numa toplogy filter | |