| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-18 | |||
| 10:08:12 | gibi | so I will replace support with apply | |
| 10:08:25 | gibi | that will enhance the scheduling logic | |
| 10:08:39 | sean-k-mooney | you proably want to call both no | |
| 10:08:49 | sean-k-mooney | support then apply | |
| 10:09:13 | gibi | apply does what support do but also decrease counts | |
| 10:09:16 | sean-k-mooney | supporst is just a simple dict comprehention so its cheap | |
| 10:09:32 | sean-k-mooney | ack but is apply much more complex or about the same | |
| 10:09:35 | sean-k-mooney | have not looked in a while | |
| 10:09:35 | gibi | I can call both but I think apply is a superset of support | |
| 10:10:05 | gibi | support is basically _filter_pools, apply is _filter_pools + _decrease_pool_count | |
| 10:10:10 | sean-k-mooney | ya im just wondering about cost but i guess apply is already called for multi create | |
| 10:10:21 | gibi | apply is called at the end yes | |
| 10:10:25 | gibi | anyhow | |
| 10:10:37 | sean-k-mooney | ack so i guess that is fine | |
| 10:11:02 | sean-k-mooney | will apply decresase the pools if they all dont fit | |
| 10:11:20 | sean-k-mooney | i.e. can it handel partial cases | |
| 10:11:45 | sean-k-mooney | im just wonderign do we need to do an atomic swap | |
| 10:12:10 | gibi | I can call apply on a local copy of stats | |
| 10:12:14 | gibi | then drop the copy | |
| 10:12:23 | sean-k-mooney | ack | |
| 10:12:24 | gibi | so no interference between parallel requests | |
| 10:12:42 | sean-k-mooney | well i was thinking swap the copy with the orginal if it succeeeds | |
| 10:12:51 | sean-k-mooney | so that multi create works | |
| 10:13:16 | sean-k-mooney | althogh | |
| 10:13:18 | sean-k-mooney | maybe not | |
| 10:13:29 | sean-k-mooney | droping it might be better to not break the numa toplogy filters | |
| 10:13:33 | gibi | we have two phases for multicreate 1) running filters (this will apply on a copy) 2) consume the selected host (this already apply on a shared stats) | |
| 10:13:57 | sean-k-mooney | right we will need to do apply 3 times | |
| 10:14:08 | sean-k-mooney | pci filter, numa toplogy filter and host manager | |
| 10:14:16 | gibi | yes | |
| 10:14:20 | sean-k-mooney | the first two should be copies to not break each other | |
| 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 | |