| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-08-27 | |||
| 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 | |
| 11:29:47 | sean-k-mooney | it passed on the next 2 | |
| 11:30:02 | sean-k-mooney | so either that test need to be after the fixture change | |
| 11:30:19 | sean-k-mooney | or its flaky under threading and need more investigation | |
| 11:30:30 | sean-k-mooney | so i was going to suggest adding it to the exclude list | |
| 11:30:45 | sean-k-mooney | i was tired last night so i didnt dign into the error much | |
| 11:31:50 | sean-k-mooney | gibi: i might do this as well but you may want to run it in a loop, im thinking of just letting the voting patch run in a loop executing unit test for an hour or so on my home server | |
| 11:32:27 | sean-k-mooney | actully i will kick that off now | |
| 11:42:51 | gibi | sean-k-mooney: I did run them in a loop locally. But I will investigate your comment | |
| 12:00:31 | sean-k-mooney | it was exactly 1 failng test so it could be just a one off cause by some test to test interaction | |
| 12:32:33 | opendevreview | Takashi Kajinami proposed openstack/nova master: Purge nested SEV RPs when SEV is disabled https://review.opendev.org/c/openstack/nova/+/958626 | |
| 12:39:27 | tkajinam | my bad. I though I run the whole functional tests but I didn't (and didn't caught some tests also needing update ) | |
| 12:39:32 | tkajinam | I thought * | |
| 12:53:55 | ralonsoh | sean-k-mooney, hello! maybe you missed my message about SR-IOV trusted VFs and MAC changing (https://specs.openstack.org/openstack/nova-specs/specs/rocky/implemented/sriov-trusted-vfs.html) | |
| 12:56:07 | sean-k-mooney | ralonsoh: ya i didnt see it but i think we talked about it in the past | |
| 12:56:28 | sean-k-mooney | trusted vf shoudl not allow the mac to be changed form the guest | |
| 12:56:46 | sean-k-mooney | i think we talked about there beign a bug in one of the intel drivers? | |
| 12:57:24 | sean-k-mooney | the tursted vf spec allow the VF to be in promisous mode | |
| 12:57:38 | sean-k-mooney | so it can recive traffic that is not destented for its mack | |
| 12:57:53 | sean-k-mooney | but its mac shoudl be still enforced by nova/neutron to be the neutron port mac | |
| 12:59:03 | sean-k-mooney | the kernel dirver is free to rest the mack to all 0000 when the vm is stopped but if it is started again it shoudl be programed by nova/libvirt to the requsted mac again | |
| 12:59:20 | sean-k-mooney | changes to things like link state in the guest or phsysical on the host shoudl not impact the mac at all | |
| 13:00:24 | sean-k-mooney | chanign the mac in the guest shoudl not propagate to the vf in trusted mode and if it does i woudl consider that to be a security issue | |
| 13:00:37 | sean-k-mooney | specificly in the vendors PF and VF driver | |
| 13:01:40 | ralonsoh | sean-k-mooney, but port bonding (and MAC changing) is also considered in the nova spec | |
| 13:01:49 | sean-k-mooney | yes an no | |
| 13:01:54 | sean-k-mooney | what we were descibeing there | |
| 13:02:11 | sean-k-mooney | was the ablity to create a bond port in the guest with a diffent mac then what is on the VF | |
| 13:02:17 | sean-k-mooney | on the host | |
| 13:02:36 | sean-k-mooney | how this used to work is if you change the mac on the guest interface the host mac woudl remain the same | |
| 13:02:45 | sean-k-mooney | in ip tools | |
| 13:02:50 | sean-k-mooney | or ifconfig | |
| 13:03:05 | sean-k-mooney | btu the mac on the packet that was sent on the wire would be updated | |