| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-08-27 | |||
| 08:51:53 | opendevreview | Balazs Gibizer proposed openstack/nova master: Allow to start unit test without eventlet https://review.opendev.org/c/openstack/nova/+/953436 | |
| 08:51:53 | opendevreview | Balazs Gibizer proposed openstack/nova master: Run nova-conductor in native threading mode https://review.opendev.org/c/openstack/nova/+/958575 | |
| 08:51:54 | opendevreview | Balazs Gibizer proposed openstack/nova master: [test]RPC using threading or eventlet selectively https://review.opendev.org/c/openstack/nova/+/953815 | |
| 08:51:54 | opendevreview | Balazs Gibizer proposed openstack/nova master: Run unit test with threading mode https://review.opendev.org/c/openstack/nova/+/953475 | |
| 08:51:55 | opendevreview | Balazs Gibizer proposed openstack/nova master: [CI]Make nova-tox-py312-threading voting https://review.opendev.org/c/openstack/nova/+/955791 | |
| 08:51:56 | opendevreview | Balazs Gibizer proposed openstack/nova master: Make RBD Tpool usage conditional https://review.opendev.org/c/openstack/nova/+/956089 | |
| 08:51:56 | opendevreview | Balazs Gibizer proposed openstack/nova master: Do not yield in threading mode https://review.opendev.org/c/openstack/nova/+/950994 | |
| 08:51:58 | opendevreview | Balazs Gibizer proposed openstack/nova master: Fix ProviderTree copying with threading Lock https://review.opendev.org/c/openstack/nova/+/956091 | |
| 08:51:58 | opendevreview | Balazs Gibizer proposed openstack/nova master: Make libvirt Tpool proxying conditional https://review.opendev.org/c/openstack/nova/+/956090 | |
| 08:52:00 | opendevreview | Balazs Gibizer proposed openstack/nova master: [vncproxy]Handle ssl.wrap_socket removal in py312 https://review.opendev.org/c/openstack/nova/+/955915 | |
| 08:52:00 | opendevreview | Balazs Gibizer proposed openstack/nova master: [test]Further categorization of disabled unit tests https://review.opendev.org/c/openstack/nova/+/956092 | |
| 08:52:02 | opendevreview | Balazs Gibizer proposed openstack/nova master: Warn on long task wait time for executor https://review.opendev.org/c/openstack/nova/+/952666 | |
| 08:53:03 | gibi | bauzas: done. you can go wild now :) | |
| 09:03:54 | bauzas | gibi: ack | |
| 09:06:11 | tkajinam | bauzas, the HW_CPU_X86_AMD_SEV trait is added even in compute nodes. https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L13223 | |
| 09:06:53 | tkajinam | bauzas, the trait hasn't been unused really in scheduling but it has been added to compute nodes with sev enabled. so it does not break scheduling to old compute nodes with SEV | |
| 09:06:54 | bauzas | oh, you're right, and in the reshape, you moved it to the nested RP | |
| 09:06:59 | tkajinam | yes | |
| 09:07:15 | bauzas | my brain forgot that while I was still reviewing it | |
| 09:07:20 | bauzas | sorry | |
| 09:07:23 | tkajinam | no, np | |
| 09:07:30 | bauzas | changing my vote then | |
| 09:07:49 | bauzas | tkajinam: I also have a slight concern, about some deprecation release value you change | |
| 09:07:55 | bauzas | not related to SEV | |
| 09:08:07 | tkajinam | I'll revert that change and push a new version | |
| 09:08:08 | tkajinam | one sec | |
| 09:08:19 | tkajinam | (the others are just rebased so +2 should be kept | |
| 09:09:30 | bauzas | cool | |
| 09:09:41 | opendevreview | Takashi Kajinami proposed openstack/nova master: Detect AMD SEV-ES support https://review.opendev.org/c/openstack/nova/+/925685 | |
| 09:09:42 | opendevreview | Takashi Kajinami proposed openstack/nova master: libvirt: Launch instances with SEV-ES memory encryption https://review.opendev.org/c/openstack/nova/+/926106 | |
| 09:09:42 | opendevreview | Takashi Kajinami proposed openstack/nova master: Add hw_mem_encryption_model image property https://review.opendev.org/c/openstack/nova/+/927706 | |
| 09:09:43 | opendevreview | Takashi Kajinami proposed openstack/nova master: Add functional test scenario for mixed SEV RPs https://review.opendev.org/c/openstack/nova/+/958562 | |
| 09:09:44 | bauzas | and thanks for being present | |
| 09:10:41 | tkajinam | np and thank you for the review ! | |
| 09:17:09 | gibi | sambork: bauzas: sean-k-mooney: dansmith: local testing of the image cache shows that we have a bug somewhere around executor shutdown as only one of the two hosts get the image if the executor size is 1, If I increase the executor size to 2 then both gets it. See logs in https://review.opendev.org/c/openstack/nova/+/957088/7#message-a1b56a536b88c1f4372a2d986364a1f61d5c75af | |
| 09:19:36 | bauzas | hmmm | |
| 09:19:52 | sambork | gibi, ack, checking | |
| 09:20:20 | gibi | If I have to guess then it is a difference in behavior in executor shutdown when there are queued tasks between eventlet and threading executors | |
| 09:20:42 | gibi | but I have to prove that... | |
| 09:24:30 | bauzas | tkajinam: saw my other concern about the nested RP deletion when the config changes ? | |
| 09:25:32 | bauzas | we should mention that in the docs | |
| 09:29:24 | opendevreview | Takashi Kajinami proposed openstack/nova master: Purge nested SEV RPs when SEV is disabled https://review.opendev.org/c/openstack/nova/+/958626 | |
| 09:29:34 | tkajinam | bauzas, yes I saw it... and think we can try ^^^ | |
| 09:30:33 | sambork | gibi, it maybe logic is a little bit different and in case of threadPoolExecutor we are not using work_queue in shutdown logic | |
| 09:30:36 | bauzas | I had that approach first when fixing the GPU SRIOV RPs | |
| 09:30:41 | tkajinam | bauzas, I noticed one difference between vGPU RP and SEV RP. The former is named according to definition in nova.conf while the other is named statically so purging SEV rp should be possible | |
| 09:30:59 | bauzas | tkajinam: yes, you're correct | |
| 09:31:16 | bauzas | we statically define the list of RPs based on what the op provided | |
| 09:31:30 | bauzas | while here, we automatically create the RPs based on what libvirt driver detects | |
| 09:32:11 | tkajinam | yeah | |
| 09:32:22 | bauzas | so yeah, we could do what you wrote (reporting total=0 so ensure_rps() will wipe it) | |
| 09:32:33 | bauzas | nice catch | |
| 09:32:37 | tkajinam | one challenge with vGPU is that it loose the name of RP when a record is completely removed from nova.conf | |
| 09:32:45 | tkajinam | I mean nova loose * | |
| 09:32:50 | bauzas | yes, this is config-driven | |
| 09:33:12 | tkajinam | s/loose/lose/ | |
| 09:33:13 | tkajinam | yeah | |
| 09:33:27 | tkajinam | but for sev we know the exact names, as you said | |
| 09:33:48 | bauzas | anyway, I'll quickly review your patch and then I'll see how much I can help gibi and sambork on their eventlet conductor patches | |
| 09:33:49 | tkajinam | (I updated the functional tests to prove it, too | |
| 09:33:56 | bauzas | tkajinam: perfect, thanks | |
| 09:35:31 | gibi | I confirmed it is the executor.sshutdow's behavior that is different https://paste.opendev.org/show/bmZZr707naiWYHMt7qim/ | |
| 09:36:38 | gibi | I feel this is a bug in futurist as python says all pending tasks should finish https://docs.python.org/3/library/concurrent.futures.html#concurrent.futures.Executor.shutdown | |
| 09:39:42 | gibi | yepp python's own concurrent.future.ThreadPoolExecutor works as expected so this is a futuris bug. I will file it | |
| 09:40:52 | bauzas | gibi: excuse my ignorance but I'm getting hard to find how you found that only one host got the image when comparing paste.opendev.org/show/b1TPrh7ESmcIqWuTZx3D/ and paste.opendev.org/show/bGwpu8My66GcyR8T12sW/ | |
| 09:41:31 | bauzas | I see that compute1 emitted a notification saying it cached the image but I have no clue where the other compute said nooope | |
| 09:41:42 | sambork | ok, so I assume that we are block here (creating some class witch will overwrite shutdown behaviour sounds like to much) | |
| 09:41:53 | gibi | Aug 27 09:07:07 aio nova-conductor[117076]: INFO nova.conductor.manager [None req-8fc723e3-ecc0-4ee3-a4aa-fa33aa2eaf94 admin admin] Preparing to request pre-caching of image(s) 342349b7-351a-470d-92ee-c99f581e55e7 on 2 hosts across 1 cells. | |
| 09:41:58 | bauzas | I mean, I trust you but I want to be able to recognize that in logs | |
| 09:41:59 | gibi | Aug 27 09:07:11 aio nova-conductor[117076]: INFO nova.conductor.manager [None req-8fc723e3-ecc0-4ee3-a4aa-fa33aa2eaf94 admin admin] Image pre-cache operation for image(s) 342349b7-351a-470d-92ee-c99f581e55e7 completed in 4.54 seconds; 1 cached, 0 existing, 0 errors, 0 unsupported, 0 skipped (down) hosts | |
| 09:42:24 | bauzas | oh the "1 cached, gotcha" | |
| 09:42:34 | bauzas | thanks | |
| 09:42:35 | gibi | if you check the same log in the non threading case or in the executor=2 case the you will see the second log showing two cached hosts | |
| 09:42:50 | bauzas | yup, saw it thanks | |
| 09:42:56 | gibi | also the ls command from the two nodes shows that only aio got the image in _base compute1 does not | |
| 09:43:45 | gibi | sambork: yeah I feel the same, I don't think we should try to fix it in one and a half days and even if we have the fix it will be more surgery that I can expect folks will be able to review in short notice | |
| 09:44:17 | gibi | I have to drop for a lunch now sorry... | |
| 09:44:51 | sambork | gibi thanks for testing sorry that I didn't catch it | |
| 09:46:26 | gibi | no worries this is team work :) | |
| 10:04:29 | opendevreview | Merged openstack/nova stable/2024.1: Ignore metadata tags in pci/stats _find_pool logic https://review.opendev.org/c/openstack/nova/+/945101 | |
| 10:04:43 | opendevreview | Merged openstack/nova stable/2024.1: Update Nova bdm with updated swap info https://review.opendev.org/c/openstack/nova/+/937611 | |
| 11:02:25 | sean-k-mooney | gibi: so i belive if you call shutdwon on an executor pool that has pending task it just discards them | |
| 11:03:17 | sean-k-mooney | gibi: at least i belive that how the tread pools in the std lib concurrent.threadpool and process pool | |
| 11:04:49 | sean-k-mooney | gibi: whil i could not express point to this in doc this is part of the reason i orginallys asked that we do not rely on shutdown for the pool to finish all work. i wanted to keep a list of all the future for the exectuted tasks then wait for them to complete | |
| 11:06:42 | bauzas | sean-k-mooney: the SEV-ES is fully reviewed by me, I think we have a good opportunity to add that in Flamingo | |
| 11:07:07 | sean-k-mooney | bauzas: ok ill take a pass on it soon then | |
| 11:07:43 | sean-k-mooney | just sarting a littel late today, i got new ubiquit netowrking gear and may have bene up till 4 tweaking it.... | |
| 11:22:38 | gibi | sean-k-mooney: the python stdlib and the greenpool in futurist behaves the same and waits for all pending tasks, the futurist threadpool only waits for the running tasks not the queued ones. I think this is a bug in futurist as it should mimick the stdlib behavior | |
| 11:22:55 | gibi | sean-k-mooney: and yes we agreed that we should refactor the logic to be nicer than shutdown | |
| 11:23:02 | gibi | now we will have plenty of time for it | |
| 11:23:03 | gibi | :) | |
| 11:27:10 | gibi | still I will file a bug to futurist as this is a problem even for others switching from GreenThreadPoolExecutor to ThreadPoolExecutor within futurist | |
| 11:28:05 | sean-k-mooney | gibi: are you goign ot unparent the untit test form the conductor change if we are not proceedign with it | |
| 11:28:22 | gibi | yepp I will | |
| 11:28:40 | sean-k-mooney | ack did you seem my comment about the db transaction issue | |
| 11:28:44 | gibi | conductor will wait for Gazpatcho the unit test can go a bit if we want into F after FF | |
| 11:29:05 | sean-k-mooney | i think im just going to call that the GG release | |
| 11:29:10 | gibi | haven't seen the db comment yet, I will check, I assume it is abotu an unit test issue | |
| 11:29:20 | gibi | sean-k-mooney: hehe GG sounds good :) | |
| 11:29:38 | sean-k-mooney | so there was only one failing test case in the non voting zuul job patch | |