| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-08-26 | |||
| 16:21:23 | dansmith | gibi: I guess I expected to see at least something running this code in both modes, but perhaps I'm jumping the gun there. I know we don't have that for everything we're thread-ifying, but this is also sort of a self-contained example | |
| 16:21:49 | sean-k-mooney | so it would be nice to have the zuul change to enable it in the condutore in the patch | |
| 16:21:49 | bauzas | gibi: I agree with the fact that until some env var is set, nothing changes | |
| 16:22:10 | sean-k-mooney | but i dont consider that an invasive change either for what its worth | |
| 16:22:11 | gibi | dansmith: yeah we did not transformed the unit test to both modes as part of the changes but we did it separately. | |
| 16:22:24 | bauzas | but yeah, I'd have appreciated some guideline to temper my guts | |
| 16:22:32 | sean-k-mooney | its adding a lock that shoudl techenically have been there even under eventlet + swapting the executor we use | |
| 16:22:34 | dansmith | unlike general thread spawning, this is a self-contained create/destroy of a single executor pool, which means we could test both modes here and now which is kinda what I was expecting | |
| 16:23:01 | dansmith | but that's fine | |
| 16:23:22 | gibi | dansmith: I will check my unit test enalbing series if it enables the related tests already... | |
| 16:23:42 | gibi | with threading in a separate tox target | |
| 16:24:37 | sean-k-mooney | what was that job called by the way | |
| 16:24:45 | sean-k-mooney | i tough we already enabled that in ci | |
| 16:24:53 | sean-k-mooney | is that still pending? | |
| 16:24:57 | gibi | nova-tox-py312-threading | |
| 16:25:00 | gibi | it is pending | |
| 16:25:04 | gibi | https://review.opendev.org/c/openstack/nova/+/955791/9 | |
| 16:25:06 | sean-k-mooney | ah ok | |
| 16:25:26 | sean-k-mooney | sorry i tought that was earlier in the series | |
| 16:25:36 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/953475/20 that one right | |
| 16:25:45 | Uggla | assuming gibi is working on the eventlet refactor any chance to land something in the above list ? Can we define some priorities/owners about them ? | |
| 16:26:21 | sean-k-mooney | there are a number of the ci/testing changes that woudl be nice to also land | |
| 16:26:25 | gibi | dansmith: I see multiple image cache unit test passing in https://4d426b9917a9f7962308-d3c6167007a6d8477c4cec901604a1c7.ssl.cf1.rackcdn.com/openstack/4beb6e364cb74aaeb2b8ff0b475d966f/testr_results.html with threading. So if we move the refactor under the unit test series we can verify the change | |
| 16:26:37 | sean-k-mooney | but i wonder if we woudl be oke with some of those happening post FF | |
| 16:27:01 | dansmith | gibi: ack | |
| 16:27:17 | gibi | sean-k-mooney: https://review.opendev.org/c/openstack/nova/+/953475/20 adds the job, but later patches enables more and more tests | |
| 16:27:21 | sean-k-mooney | the 6 patches including and up to https://review.opendev.org/c/openstack/nova/+/955791/9 are strech goals for me | |
| 16:27:42 | gibi | yeah the unit test enablement was planned post FF as streach for Flamingo | |
| 16:28:13 | gibi | but if we block the nova-conductor refactor on it then I will at least make an arrangement to have test results | |
| 16:28:31 | gibi | I mean unit test results with image cache tests running in threading mode | |
| 16:28:42 | gibi | do you want me to do that? | |
| 16:29:47 | bauzas | I agree with the statement that I feel that the patches above n-api seem more important for now | |
| 16:29:58 | opendevreview | Max proposed openstack/nova master: db: add indexes for source/dest migrations https://review.opendev.org/c/openstack/nova/+/958556 | |
| 16:30:36 | gibi | bauzas: so you say lets prioritize the unit test enablement over the nova-conductor work. | |
| 16:30:39 | gibi | ? | |
| 16:30:58 | gibi | (I have no problem with it if that is what the team wants) | |
| 16:31:00 | bauzas | for me, at least, I'd prefer reviewing those patches | |
| 16:31:47 | sean-k-mooney | i was going to suggest rebasign the 6 packages on top of the conductor one | |
| 16:31:55 | bauzas | and I think those would be more beneficial for the rest of the work we have to do | |
| 16:32:09 | bauzas | sean-k-mooney: you mean, on the bottom ? | |
| 16:32:13 | sean-k-mooney | no | |
| 16:32:18 | gibi | sean-k-mooney: yeah that what I meant by arranging to get test results | |
| 16:32:36 | sean-k-mooney | because i disagree that havign the ablity to run the condcutor in treading mdoe is less valuable | |
| 16:33:03 | gibi | sambork: Are you around? ^^ :) | |
| 16:33:04 | sean-k-mooney | if we have the conductor patch we can test it and ask operator to test it and report bugs | |
| 16:33:08 | bauzas | if wre're not able to run it in CI, what's the benefit ? | |
| 16:33:17 | sean-k-mooney | we can trivially | |
| 16:33:26 | sean-k-mooney | its liktrally a 1 line change to the zuul job | |
| 16:33:37 | sean-k-mooney | to set the var in devstack to enabel threading mode | |
| 16:33:44 | bauzas | but we won't execute that code, right? | |
| 16:33:50 | gibi | sean-k-mooney: I think what bauzas imply is that we don't have tempest coverage for the changed code path | |
| 16:34:07 | sean-k-mooney | ok that should nto block the code change | |
| 16:34:07 | bauzas | I mean, the zuul config change will be meaningless | |
| 16:34:23 | bauzas | if we don't have zuul jobs that call image caching | |
| 16:34:25 | sean-k-mooney | its wont be it makes sure everythign else in the conductor works | |
| 16:34:39 | sean-k-mooney | i.e. that we are not implcitly relying on eventlet indrectly | |
| 16:34:53 | bauzas | that is a good point | |
| 16:35:12 | bauzas | we could have missed other eventlet dependencies | |
| 16:35:25 | bauzas | I then agree, | |
| 16:35:27 | sean-k-mooney | by the way i think we could land both if we are willing to merge the unit test change up to RC1 | |
| 16:35:51 | sean-k-mooney | my suggesting is for gibi to rebase them so we get more results today/tomorrow | |
| 16:36:01 | bauzas | agreed, I like that approach | |
| 16:36:03 | sean-k-mooney | and then we can decied if we are comfrotabel with merging the conductor changes | |
| 16:36:05 | gibi | I will do the rebase today to get unit test results with threadin on image cache | |
| 16:36:11 | bauzas | thanks gibi | |
| 16:36:14 | gibi | sambork: ^^ FYI | |
| 16:36:29 | sean-k-mooney | gibi: can we add the devstack job change too if you or sambork have time | |
| 16:36:32 | gibi | I will do local testing of the lock explicitly | |
| 16:36:37 | gibi | sean-k-mooney: I will do so | |
| 16:36:41 | sean-k-mooney | +1 | |
| 16:37:41 | gibi | I have nothing further | |
| 16:38:05 | Uggla | asking again, assuming gibi is working on the eventlet refactor any chance to land something in the above list ? Can we define some priorities/owners about them ? | |
| 16:38:37 | bauzas | about the other things to review, I want to speak about SEV-ES | |
| 16:38:46 | bauzas | but tkajinam is not here I guess (and hope) | |
| 16:38:56 | tkajinam | I am | |
| 16:38:59 | bauzas | wow | |
| 16:39:01 | bauzas | okayu | |
| 16:39:01 | tkajinam | :-) | |
| 16:39:27 | sean-k-mooney | stephenfin: and i both were interested in the sev one as well | |
| 16:39:38 | sean-k-mooney | or at least started looking at ti | |
| 16:39:46 | dansmith | Uggla: land...in the next two days? | |
| 16:39:46 | sean-k-mooney | so i think taht woudl be good to try and land | |
| 16:40:10 | bauzas | my concern is that we're on a non-SLURP release | |
| 16:40:20 | sean-k-mooney | but the main problem with that serise is reviewing the reshape | |
| 16:40:43 | bauzas | tkajinam: I assume you know that you'll have to continue to handle the reshape case for at least two releases, right? | |
| 16:40:49 | Uggla | dansmith yes if possible. | |
| 16:40:58 | Uggla | or at least try. | |
| 16:41:03 | tkajinam | bauzas, yes | |
| 16:41:23 | dansmith | that seems like a big ask to me, | |
| 16:41:28 | tkajinam | I agree with the concern about reshape but I'm hoping that I at least get any feedback to hear how far it is from merge. | |
| 16:41:35 | dansmith | but I'm not really positioned to review a reshape | |
| 16:42:01 | tkajinam | we can discuss if that can be a subject to FFE or I'm even ok if that is eventually postponed to 2026.1 | |
| 16:42:10 | bauzas | I feel reasonable being able to review a reshape since I wrote two of them | |
| 16:42:26 | tkajinam | my main concern is that we've been saying the same for recent 2~3 cycles (if that's too much for this cycle and then put it to next) | |
| 16:42:36 | bauzas | but that's a very old knowledge part of myself I need to go with | |
| 16:42:41 | sean-k-mooney | bauzas: that would help because the rest of the serise is much more straght forward | |
| 16:43:00 | bauzas | sean-k-mooney: I can help with providing pointers to how the reshape mechanism works | |
| 16:43:18 | bauzas | from the very first glance I made on tkajinam's series, he perfectly managed it | |
| 16:43:36 | bauzas | I mean, he's triggerring the signal, and then using the signal to reshape | |
| 16:43:43 | sean-k-mooney | well thats encuraging at least. | |