| *** mhen_ is now known as mhen | 01:59 | |
| jlejeune | sean-k-mooney: hello, is that better: https://review.opendev.org/c/openstack/nova/+/963368 ? | 07:01 |
|---|---|---|
| elodilles | hi nova-stable-maint members o/ i've sent a mail to ML yesterday about 2024.1 Caracal transition to Unmaintained in 2 weeks (!), we could probably do a final stable release of 2024.1 Caracal, but we should be quick about it (but be careful not to break anything! as this will be final!) | 07:46 |
| elodilles | but before we cut a release, maybe a couple of backports can land, see the etherpad: https://etherpad.opendev.org/p/nova-stable-caracal-eom | 07:47 |
| bauzas | ack will review those | 07:49 |
| jlejeune | elodilles: bauzas: it could be great also to backport that topic: https://review.opendev.org/q/topic:%22bug/2085135%22 :) | 07:55 |
| elodilles | jlejeune: once they got merged to master branch and backports are proposed, then feel free to ping me / add me as reviewer to those backports :) | 08:22 |
| jlejeune | elodilles: got it, thanks | 08:27 |
| opendevreview | Julien Le Jeune proposed openstack/nova master: Reset the mapped field of nodes at service deletion https://review.opendev.org/c/openstack/nova/+/933022 | 09:41 |
| opendevreview | Svein-Erik Skjelbred proposed openstack/nova stable/2025.1: lvcreate fails now and then. https://review.opendev.org/c/openstack/nova/+/963513 | 10:48 |
| sean-k-mooney | jlejeune: yep thanks | 11:15 |
| sean-k-mooney | jlejeune: that is now followign or standar repoeducer and fix pattern using a dedicated regression test in the regression folder | 11:16 |
| opendevreview | Julien Le Jeune proposed openstack/nova master: Reset the mapped field of nodes at service deletion https://review.opendev.org/c/openstack/nova/+/933022 | 11:35 |
| opendevreview | Julien Le Jeune proposed openstack/nova master: Adds regression test for bug LP#2085135 https://review.opendev.org/c/openstack/nova/+/963368 | 12:31 |
| opendevreview | Julien Le Jeune proposed openstack/nova master: Reset the mapped field of nodes at service deletion https://review.opendev.org/c/openstack/nova/+/933022 | 12:31 |
| opendevreview | Julien Le Jeune proposed openstack/nova master: Adds regression test for bug LP#2085135 https://review.opendev.org/c/openstack/nova/+/963368 | 12:58 |
| opendevreview | Julien Le Jeune proposed openstack/nova master: Reset the mapped field of nodes at service deletion https://review.opendev.org/c/openstack/nova/+/933022 | 12:58 |
| opendevreview | Julien Le Jeune proposed openstack/nova master: Reset the mapped field of nodes at service deletion https://review.opendev.org/c/openstack/nova/+/933022 | 13:18 |
| jlejeune | sean-k-mooney: I've updated my changes following your reviews | 13:21 |
| sean-k-mooney | cool ill re reivew. di that work locally for you https://review.opendev.org/c/openstack/nova/+/933022/9/nova/db/main/api.py | 13:23 |
| *** haleyb_ is now known as haleyb | 13:28 | |
| jlejeune | sean-k-mooney: yes it works as expected | 13:45 |
| opendevreview | Kamil Sambor proposed openstack/nova master: Switch nova-conductor to use global executor https://review.opendev.org/c/openstack/nova/+/962478 | 13:56 |
| ratailor | sean-k-mooney, gibi could you please review https://review.opendev.org/c/openstack/nova/+/963348 https://review.opendev.org/c/openstack/nova/+/961352 and https://review.opendev.org/c/openstack/nova/+/961211 | 14:27 |
| opendevreview | Balazs Gibizer proposed openstack/placement master: Prune a_c search space by invalid prefixes https://review.opendev.org/c/openstack/placement/+/962776 | 14:32 |
| gibi | dansmith: I will accept the heat for it but I ended up changing the whole pruning logic while tried to pull out the capacity check function call and the odometer logic. I ended up dropping the odometer logic in favor of a recusive tree search as it results in a lot cleaner code and allows one extra optimization out of the box that is dropping half of the steps out in the symmetric case. So the 3by3 | 14:32 |
| gibi | test now checks only 30 partial candidates not 60. This gives us even more performance buffer. I also managed to avoid changing the conditional ARR copy logic which is a nice risk reduction alone. ^^ | 14:32 |
| dansmith | ooof | 14:32 |
| sean-k-mooney | gibi: long term do you think we will want to keep the fallback to itertools.product | 14:38 |
| sean-k-mooney | gibi: if the performace is simialr for small trees i wonder if we coudl jsut alwasy use the new soltuion | 14:39 |
| sean-k-mooney | per haps somethignto let bake for a while and evolve too in a future release | 14:39 |
| dansmith | I asked for the fallback specifically to avoid switching the world to this (well, the previous) much more complicated solution until it had seen some time on the battlefield, especially when the stdlib version works fine for everyone else (so far) | 14:40 |
| gibi | sean-k-mooney: dansmith asked to put this behind a workaround flag to mitigate risk for the majority of our users who are not using PCI in Placement (or other sources of wide provider trees) so I think we want to bake it | 14:40 |
| sean-k-mooney | ack | 14:40 |
| sean-k-mooney | so after our converstaion on this topic i was thinkign that we may want to consider changign the default of https://docs.openstack.org/placement/latest/configuration/config.html#placement.allocation_candidates_generation_strategy to breadth-first and deprecating randomize_allocation_candidates (possibley replacing it with a random allocation_candidates_generation_strategy | 14:43 |
| sean-k-mooney | policy if needed) | 14:43 |
| sean-k-mooney | i also was wodnerign if optimize_for_wide_provider_trees really shoudl be a workaround option | 14:45 |
| sean-k-mooney | to me this feel much more fitign to be grouped with allocation_candidates_generation_strategy | 14:45 |
| sean-k-mooney | or randomize_allocation_candidates in the palcement section | 14:46 |
| gibi | yeah workarond make sense if we know we will go back and redesign the modeling of PCI devices and that make the current optimization unnecessary | 14:46 |
| dansmith | workaround because we expect to make it the default eventually, and right now this is basically for one very specific use case...as a workaround.. if we're going to keep it forever, then fine to put it in main config, but it feels like something we plan to drop when we're comfortable (or redesign this entirely) | 14:46 |
| sean-k-mooney | ok fair, i was thinking about the fact we dont want to ship with a workaroudn enabled by defalt | 14:47 |
| sean-k-mooney | so i was tryign to see how we woudl evovel this to on by default without forcing that in a single release | 14:47 |
| sean-k-mooney | im not opposed to puting it in workarounds | 14:48 |
| sean-k-mooney | just wanted to raise that | 14:48 |
| sean-k-mooney | i was kind fo thinking add in 2026.1 make default in 2026.2 and remove in 2027.1 somethign like that but we dont need to rush this either | 14:49 |
| gibi | moving a config between sections is not hard, oslo has deprecated section arg. | 14:49 |
| sean-k-mooney | that was partly why i was asking about the perfroamce for small treese vs itertools.product | 14:49 |
| gibi | so if we decide to make it default true, then we can also move it to [placement] section | 14:49 |
| sean-k-mooney | ya that also fair, it handels the legacy name for us | 14:50 |
| dansmith | we're not going to ship with a workaround defaulted to true | 14:50 |
| sean-k-mooney | yep i know but im not really considerign this a wrokaround at this point it feel more liek a real performafce optimisation/mini feature | 14:51 |
| sean-k-mooney | anyway im ok with the current proposal | 14:51 |
| sean-k-mooney | i.e. to have it be a workaroudn for now. ill try and make some time to look at the revised patch in detail | 14:51 |
| dansmith | gibi: tbh, even though I'm impressed at getting more performance out, I'm (as you guessed) a bit sad to have reset the progress a week | 14:52 |
| dansmith | how much better is it for the current known-worst case of like 20ish providers? | 14:52 |
| gibi | dansmith: test_many_non_viable_candidates_21_21 went from 8.3 sec (odometer) to 3 sec (recursive tree search). But I would not pivot the impl because performance (alone) I pivoted due to how much cleaner the code now. (I guess it is matter of taste a bit if a maintaining a set of indexes is better than maintaining stack for recursion) | 15:02 |
| gibi | test_many_non_viable_candidates_32_32_two_computes went from 20 to 2.5 | 15:02 |
| sean-k-mooney | v15 test_many_non_viable_candidates_64_64_two_computes [8.489979s] v16 test_many_non_viable_candidates_64_64_two_computes [0.485396s] | 15:03 |
| dansmith | but the immediate goal here was to avoid it being longer than the scheduler timeout to fix the actual failure, which even at 64x on the previous version was well accomplished correct? | 15:04 |
| dansmith | anyway, I preempted vtpm stuff for the reviews recently and was geared up to get back to that today and feel like I'm now reset back a week :( | 15:04 |
| sean-k-mooney | well that is with 10 allcoation candiate max | 15:04 |
| sean-k-mooney | but yes if you tune small enough you can with v15 | 15:05 |
| sean-k-mooney | v16 woudl allow you to have that higher | 15:05 |
| sean-k-mooney | for the test gibi ran i see v15 test_many_non_viable_candidates_21_21 [15.881115s] v 16 test_many_non_viable_candidates_21_21 [4.901830s] | 15:06 |
| dansmith | I get that it's faster, I'm just not sure it's faster enough to matter _that_ much when we're trying to get something landed to fix a thing.. and I can already see from a skim that it's cleaner, | 15:07 |
| dansmith | I just don't know that it's important enough to have reset the game here | 15:07 |
| sean-k-mooney | you saying restore v15 and make v16 a patch on top? | 15:08 |
| gibi | I think the cleaner code worth the re-review effort as probably need to read this piece of code multiple times in the future | 15:08 |
| dansmith | I'm not saying anything. | 15:08 |
| sean-k-mooney | gibi: given nova-next is currently testing thread mode can i suggest enablign this in nova-ovs-hybrid-plug. i have mentioned befor ethat i woudl like to rename this to nova-alt-config to beterer refect that we use ti to test non defautl config liek ml2/ovs or spice instead of vnc. | 15:33 |
| sean-k-mooney | i really dont expect that to be a stablity issue given the job wont reallly exersie teh new codepaths | 15:35 |
| sean-k-mooney | i.e. we dont have a lot of child providers in that job today | 15:35 |
| gibi | we have no jobs with child providers really in upstream CI | 15:35 |
| sean-k-mooney | we will if we ever finish enabling the mtty change or similar but ya we dotn really today | 15:36 |
| gibi | I even feel like enabling it in nova-next is OK as this is a placement config and placement is not changed threading | 15:36 |
| sean-k-mooney | that is fine too | 15:36 |
| sean-k-mooney | i just didnt want to intoduce a diffent variabel to have to debug in that job | 15:37 |
| sean-k-mooney | basiclly im suggstign tweak a job just to have it enabeld | 15:37 |
| gibi | even the breadth-first config is supposed to be enabled in nova-next I just never managed to get back to the devstack patch that make it possible | 15:37 |
| sean-k-mooney | but im not too pushed on which job | 15:37 |
| gibi | https://review.opendev.org/c/openstack/devstack/+/939825 | 15:38 |
| sean-k-mooney | oh is that still open | 15:39 |
| gibi | https://review.opendev.org/c/openstack/nova/+/937275 | 15:40 |
| gibi | yepp | 15:40 |
| gibi | but I will use the time now to fix that up | 15:40 |
| opendevreview | Alan Bishop proposed openstack/nova-specs master: Show swap_progress in attachment details https://review.opendev.org/c/openstack/nova-specs/+/937485 | 16:25 |
| dansmith | melwitt: hmm, so I moved to the next patch (host), configured a flavor for host, booted a server which clearly has host set (in sysmeta) and I get the same failure to restart it as admin | 17:21 |
| dansmith | host means the user still owns the key in barbican, but we don't need to hit barbican to start anymore because it's persisted in libvirt right? | 17:22 |
| melwitt | dansmith: oh, maybe that's an oversight on my part. admin should be able to live migrate it but I don't believe I changed the code in hard reboot to not go to barbican if 'host' | 17:24 |
| melwitt | so yeah I think I just need to bypass that if 'host'. I had been focusing on the live migration only. I should also add a hard reboot by admin to my tempest tests | 17:26 |
| opendevreview | Merged openstack/placement master: Reproduce GET a_c slowness bug/2126751 https://review.opendev.org/c/openstack/placement/+/962596 | 17:27 |
| dansmith | melwitt: you claim it in the config can be rebooted :) | 17:46 |
| melwitt | dansmith: uh oh | 17:48 |
| dansmith | oh, well, it says "host reboot" actually, so maybe that works because of libvirt? | 17:56 |
| dansmith | anyway, I think we want that to be a feature of host security, so we should probably make a note to fix it even if not in this patch | 17:57 |
| melwitt | yeah, I'll try to fix it in the same patch since i'm changing stuff anyways | 17:58 |
| opendevreview | melanie witt proposed openstack/nova master: Dump conf to debug log earlier in the WSGI app pipeline https://review.opendev.org/c/openstack/nova/+/963619 | 18:48 |
| *** haleyb is now known as haleyb|out | 23:22 | |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!