| gmaan | gibi: replacing StaticallyDelayingWrapper looks good to me, if you can check it. https://review.opendev.org/c/openstack/nova/+/997410 | 00:27 |
|---|---|---|
| gmaan | polyedre: i left some comment for the event handlers stop fix, meanwhile I am reviewing the next fix of RPC servers wait() https://review.opendev.org/c/openstack/nova/+/1010011 | 01:58 |
| *** mhen_ is now known as mhen | 03:02 | |
| opendevreview | Thibaut Démaret proposed openstack/nova master: libvirt: add disk rotation_rate support for local disks https://review.opendev.org/c/openstack/nova/+/979693 | 08:15 |
| opendevreview | Kamil Sambor proposed openstack/nova master: Replace StaticallyDelayingWrapper with futurist DelayingExecutor https://review.opendev.org/c/openstack/nova/+/997410 | 08:37 |
| *** erlon28 is now known as erlon2 | 08:41 | |
| opendevreview | Matt Crees proposed openstack/nova master: Add test_update_devices_preserves_vf_network_caps https://review.opendev.org/c/openstack/nova/+/989121 | 10:08 |
| opendevreview | Matt Crees proposed openstack/nova master: Don't drop pci_dev network caps when VF is in use https://review.opendev.org/c/openstack/nova/+/982660 | 10:08 |
| opendevreview | Lajos Katona proposed openstack/nova master: libvirt: Add test for MTU overwrite during live migration https://review.opendev.org/c/openstack/nova/+/995803 | 10:44 |
| opendevreview | Lajos Katona proposed openstack/nova master: libvirt: Preserve source MTU during live migration https://review.opendev.org/c/openstack/nova/+/995809 | 10:44 |
| opendevreview | Lajos Katona proposed openstack/nova master: libvirt: Preserve source MTU during live migration https://review.opendev.org/c/openstack/nova/+/995809 | 11:58 |
| opendevreview | Lajos Katona proposed openstack/nova master: Add regression test to repoduce bug 1854844 https://review.opendev.org/c/openstack/nova/+/991772 | 11:58 |
| opendevreview | Lajos Katona proposed openstack/nova master: libvirt: Preserve queue_size during live migration https://review.opendev.org/c/openstack/nova/+/995841 | 11:58 |
| gibi | gmaan: sambork: I left comment in the StaticallyDelayingWrapper patch it is still different from memory consumption perspective than the baseline as we know we have to be careful about the number of threads due to memory consumption I would like to keep the baseline behavior where we *reuse* the default executor for the delayed processing as well. | 12:39 |
| gibi | a having a separate executor means having separate workers mean more threads | 12:39 |
| sambork | gibi, sure will get back to reused worker then | 12:57 |
| gibi | sambork: thanks | 13:23 |
| gibi | tkajinam: thanks for the libvirt/qemu version bump and cleanup patches. I'm +2 all the way up | 13:23 |
| bauzas | gibi: I can review the libvirt version patch, which gerrit change ? :) | 13:42 |
| gibi | https://review.opendev.org/q/topic:%22indri-libvirt-qemu-version%22 | 13:44 |
| gibi | (I had the link from the yesterday IRC meeting :D) | 13:44 |
| gibi | can some core other than gmaan look at https://review.opendev.org/c/openstack/nova/+/1007150 as well? | 13:47 |
| gibi | it is test only stabilization | 13:48 |
| gibi | dansmith: this can be interesting to you as I you might reuse this code from the in cclm https://review.opendev.org/q/topic:%22bug/2168874%22 - Instance obj cloning is incomplete in the cross cell cold migration task | 13:51 |
| gibi | (I blame the weather for my broken English) | 13:52 |
| dansmith | gibi: oh sorry I had started looking at this in September and never finished or even committed my draft comments | 14:50 |
| dansmith | I suck | 14:50 |
| gibi | no worries | 14:51 |
| gibi | I also stopped it when I realized the trusted cert stuff behind it | 14:51 |
| bauzas | gibi: dansmith: currently testing the compute services restarts (https://etherpad.opendev.org/p/nova-cclm items #2 and #3) | 15:28 |
| gibi | bauzas: ack | 15:29 |
| gibi | thanks | 15:29 |
| dansmith | cool | 15:31 |
| dansmith | my thinking is that after consumer types lands I will rebase the series on master, change the operation name and make a few other small changes that have come up | 15:32 |
| dansmith | I was thinking I would continue trying to close the functional test gap at that point, but perhaps it's best to focus on things we all find while poking manually and once those are settled go back to marching through the functional tests? | 15:33 |
| gibi | dansmith: +1 on rebase when consumer type lands | 15:41 |
| gibi | I lean towards fixing the issue we find in the meantime with poking at the edge cases | 15:41 |
| gibi | if we run out those then functional test next | 15:41 |
| gibi | also in the meantime gmaan or others might be interested to set up tempest testing | 15:42 |
| dansmith | cool | 15:42 |
| opendevreview | Max proposed openstack/nova master: Add regression test for bug 2173889 https://review.opendev.org/c/openstack/nova/+/1014906 | 15:55 |
| opendevreview | Max proposed openstack/nova master: fix: orphaned attachments on delete while evacuate https://review.opendev.org/c/openstack/nova/+/1014907 | 15:55 |
| opendevreview | Arnaud Morin proposed openstack/nova master: Close the placement client before forking service workers https://review.opendev.org/c/openstack/nova/+/1011901 | 16:22 |
| opendevreview | Arnaud Morin proposed openstack/nova master: Reset executors before fork in nova.service.serve() https://review.opendev.org/c/openstack/nova/+/1013086 | 16:22 |
| gmaan | gibi: dansmith ++, i can do tempest side work once operation name etc are done | 16:23 |
| gmaan | bauzas: i hope your compute service restart testing worked so magically and perfectly :P | 16:24 |
| bauzas | gmaan: apparently we stopped the migration instead of awaiting for it | 16:39 |
| bauzas | hmmm, apparently my devstack is misconfigured, no budget for awaiting | 16:44 |
| opendevreview | Arnaud Morin proposed openstack/nova master: Fix: avoid comparing dev_type if absent https://review.opendev.org/c/openstack/nova/+/964552 | 16:45 |
| opendevreview | Arnaud Morin proposed openstack/nova master: Reproducer for bug 2129570 https://review.opendev.org/c/openstack/nova/+/1014925 | 16:45 |
| gmaan | bauzas: did it wait for the 160 sec? | 16:47 |
| bauzas | gmaan: can you confirm I misconfigured devstack because I have 0 seconds here https://paste.opendev.org/show/bfVkOMfMPW8B2TWOwnwA/ | 16:48 |
| gmaan | bauzas: manager_shutdown_timeout=160 is good but graceful_shutdown_timeout=5 is reason it is not waiting | 16:49 |
| gmaan | in devstack, it override the default graceful_shutdown_timeout which is 180 because we want other jobs to quickly shutdown services | 16:49 |
| bauzas | yeah I'm redoing the test by giving more time | 16:49 |
| gmaan | if you remove the graceful_shutdown_timeout from the nova.conf on all comoute then it should work | 16:50 |
| bauzas | or setting to 180 N | 16:50 |
| gmaan | ++ | 16:50 |
| bauzas | OK, cool | 16:50 |
| gmaan | i wanted to remove graceful_shutdown_timeou5 from devstack and jobs can override to lower time if they are doing restart service and does not test graceful shutdown | 16:51 |
| dansmith | bauzas: so are you going to re-test with that fix to see if we wait properly? | 16:59 |
| bauzas | yup | 16:59 |
| dansmith | cool | 16:59 |
| bauzas | but now I have another issue | 16:59 |
| bauzas | I don't know how this happened but my instance is currently on migrating state, but I can't delete it because it's not found :) | 17:00 |
| bauzas | fun | 17:00 |
| dansmith | I thought it went back to active? | 17:00 |
| bauzas | I just tested again | 17:01 |
| bauzas | so that's another instance | 17:01 |
| bauzas | the migration completed, the instance is on the dest but the API show tells me it's on the source, doh | 17:01 |
| bauzas | (actually the migration never completed, only libvirt) | 17:02 |
| bauzas | the domain is on the dest | 17:02 |
| gmaan | i have experienced it in revert resize and stopping migration in between can have this situation | 17:02 |
| gmaan | abort migration will not work as it will have 404 on instance | 17:03 |
| dansmith | yeah it happens with regular live migration too | 17:03 |
| gmaan | maybe try with new instance. this is something we are improving in graceful shutdown part-3 anywyas | 17:03 |
| gmaan | yes, existing migrations has same issue | 17:04 |
| bauzas | I was able to delete it eventually with the UUID | 17:04 |
| dansmith | bauzas: was that a happy path migration or did you try to trigger a graceful shutdown in the middle? | 17:04 |
| bauzas | it was an happy path as a first check | 17:04 |
| bauzas | now, I'll redo happy path and then source restart (with updated shutdown timeout) | 17:05 |
| polyedre | gmaan: thanks for the review! | 17:05 |
| dansmith | are you sure you weren't trying to delete by name with the admin token? I always forget, but you can't do that since it won't show up in the list that osc does to try to resolve the name | 17:05 |
| gmaan | it says live migration v2 is in-progress task which did not finish "Graceful shutdown initiated with 1 in-progress tasks: live_migration_v2(" | 17:06 |
| gmaan | polyedre: no issue. I did not finish the RPC server ordering fix but let's get the event handler in shape first and I will try to review RPC one in parallel. let me know if any question on my comments | 17:06 |
| bauzas | fun, migration completed but my systemctl restart is still hanging :-) | 17:09 |
| gmaan | cool, logs should tell it shutdown came before migration completed or shutdown actually waited for it | 17:10 |
| gmaan | or any otehr tasks making shutdown stuck | 17:11 |
| bauzas | gmaan: it worked correctly AFAICT https://paste.opendev.org/show/bfIG7wsGQxnqiJ0RdqEX/ | 17:14 |
| bauzas | dansmith: I'd say graceful shutdown for source works for your POC | 17:14 |
| bauzas | item 2 closed :-) | 17:15 |
| bauzas | (or rather VERIFIED) | 17:15 |
| gmaan | bauzas: task wise good, live migration completed, server external event is accepted and compleetd 'Graceful shutdown: new task started after shutdown initiated: external_instance_event ' | 17:15 |
| gmaan | but | 17:16 |
| gmaan | " nova-compute manager graceful shutdown did not complete within the 160 second" | 17:16 |
| gmaan | i think that is because of libvirt event handler "Shutdown is requested while waiting for the deadline of Task(fn=<bound method Host._event_emit of <nova.virt.libvirt.host.Host" | 17:16 |
| dansmith | bauzas: that's source, right? so that's cool... | 17:17 |
| gmaan | which is one todo and addressed by this change which i am reviwing currently https://review.opendev.org/c/openstack/nova/+/1010011 | 17:17 |
| gmaan | so live migration perspective it worked fine | 17:17 |
| bauzas | gmaan: no worries, I don't want to test graceful shutdown, I'm just checking every item in https://etherpad.opendev.org/p/nova-cclm | 17:18 |
| gmaan | ++ :) | 17:18 |
| bauzas | dansmith: yup | 17:18 |
| bauzas | testing now graceful shutdown of destination | 17:18 |
| opendevreview | Lajos Katona proposed openstack/nova master: Use SDK for Neutron networks https://review.opendev.org/c/openstack/nova/+/928022 | 17:19 |
| opendevreview | Lajos Katona proposed openstack/nova master: Use SDK for Neutron subnets https://review.opendev.org/c/openstack/nova/+/962190 | 17:19 |
| opendevreview | Lajos Katona proposed openstack/nova master: Use SDK for Neutron extensions https://review.opendev.org/c/openstack/nova/+/962270 | 17:19 |
| opendevreview | Lajos Katona proposed openstack/nova master: Use SDK for Neutron floating IPs https://review.opendev.org/c/openstack/nova/+/962604 | 17:19 |
| gmaan | gibi: (sambork ) nice catch on the memory things, i did not realize it will double the number of threads. as delayed executor is used for STOPPED event only, can we use 2 as max_worker for now until we implement it in futurist? or you think we do not replace until we have futurist imp take executors as arg?https://review.opendev.org/c/openstack/nova/+/997410/comment/e6035666_c563e8a0/ | 17:28 |
| gmaan | sharing the default executors for the stopped event seems Overwhelming the default executor? those can run separately in 1 or 2 thread | 17:30 |
| gibi | gmaan: the event that are waiting for their delay are not occupying a worker. We have one single thread in the wrapper that does the scheduling of delayed tasks, the tasks are only sent to the executor after their delay passed | 17:31 |
| gibi | I have to look more tomorrow about what it means supporting it in futurist, I thought we had that there | 17:31 |
| gibi | stephenfin: melwitt: gmaan: you commented on the cclm spec in the past but not since the -2 is removed from it. I feel like we are close to land the spec so please add you remaining comments if any https://review.opendev.org/c/openstack/nova-specs/+/1008081 | 17:32 |
| gmaan | gibi: is it? i thought there can be more than one task submitted before previous one completed as delay executor only check remaining time not the previous task finished? | 17:32 |
| gmaan | gibi: sure, no hurry on that. | 17:32 |
| melwitt | gibi: thanks for the heads up, I will go look and comment | 17:33 |
| gmaan | gibi: ack on CCLM, need to go out for a bit but will check today | 17:33 |
| gibi | gmaan: sorry, that is a question I need to think about but I will only do that tomorrow :) please leave a comment in the patch if not yet done and I will take a look | 17:33 |
| gibi | tomorrow | 17:33 |
| gmaan | sure | 17:33 |
| gibi | gmaan: melwitt: thanks both :) | 17:34 |
| * gibi drops | 17:34 | |
| gibi | o/ | 17:34 |
| gmaan | o/ | 17:34 |
| *** gmaan is now known as gmaan_afk | 17:34 | |
| bauzas | destination graceful shutdown verified for CCLM, worked as expected | 17:36 |
| dansmith | woot | 17:38 |
| dansmith | sorry, let me translate that for you | 17:40 |
| dansmith | le woot | 17:40 |
| bauzas | I just assume a double smackdown for both computes will do the same (ie. we'll await for the migration to finish) but lemme test it | 17:40 |
| dansmith | I would hope :) | 17:40 |
| bauzas | dansmith: le hope | 17:41 |
| dansmith | maybe we need a new spec for graceful-double-smackdown-support | 17:41 |
| bauzas | I'd so love to see a graceful-double-smackdown game | 17:41 |
| bauzas | I could imagine gmaan_afk and yourselves on a ring :) | 17:42 |
| bauzas | yourself* | 17:42 |
| dansmith | I am not interested in smacking down gmaan_afk :) | 17:42 |
| bauzas | this is just spectacle | 17:42 |
| dansmith | oh all fake for the tv cameras? | 17:43 |
| bauzas | are you seriously imagining smackdown as non-fake ? :() | 17:43 |
| bauzas | :-) | 17:43 |
| dansmith | lolol | 17:43 |
| dansmith | I also believe in the tooth fairy and santa clause | 17:44 |
| bauzas | on a ring ? | 17:44 |
| bauzas | that could be fun | 17:44 |
| bauzas | OK, verified for double restarts, everything awaits for the live-migration to complete (minus the few bugs we have with graceful shutdown) | 17:49 |
| dansmith | nice | 17:49 |
| rm_work | Does https://review.opendev.org/c/openstack/nova/+/1010011 replace https://review.opendev.org/c/openstack/nova/+/1005022 ? I was tracking the latter and include it in my internal deployment but can swap which I track | 17:51 |
| rm_work | Oh and I switched up the approach on https://review.opendev.org/c/openstack/nova/+/1009885 after your comments last week and it definitely “works” but still happy to let someone do it the right way manually if you’d prefer, lmk if the LLM stuff is less helpful than just posting the bug+repro (also maybe https://review.opendev.org/c/openstack/nova/+/1014572 which reproduces it) | 17:54 |
| rm_work | I’ve generally found it to be pretty good quality, at least after some minimal review, but nova is the project I likely am the least comfortable with, so it’s hard to form a real opinion on this one | 17:59 |
| bauzas | OK, apart from spamming the n-cpu logs with InstanceNotFound, a VM deletion while a CCLM migration is running is not a probme | 18:09 |
| bauzas | problem | 18:09 |
| JayF | https://review.opendev.org/c/openstack/nova/+/1004890 and https://review.opendev.org/c/openstack/nova/+/1005115 are both good tested performance fixes, already landed before 26.2 was cut, trying to get the backports in. thanks o/ | 18:16 |
| dansmith | bauzas: which n-cpu? | 18:25 |
| bauzas | source host | 18:25 |
| bauzas | dansmith: https://paste.opendev.org/show/bpPDYPbSMsfzFzPRRMzJ/ | 18:25 |
| bauzas | there are some assumptions in the monitor that makes the source host trying to delete the instance while this is already deleted | 18:26 |
| dansmith | ah yeah I think that's fairly common for other operations as well.. both of those traces show what is likely shared code between the two, so I suspect the same would happen for legacy migratino | 18:26 |
| dansmith | yeah | 18:26 |
| bauzas | yup, I wouldn't call it a bug | 18:26 |
| bauzas | and yeah, probably the same for legacy migrations | 18:27 |
| bauzas | so I'll call out | 18:27 |
| bauzas | anyway, time to call it a day | 18:29 |
| bauzas | I'll be on holiday tomorrow so \o | 18:29 |
| dansmith | o/ | 18:39 |
| *** gmaan_afk is now known as gmaan | 19:04 | |
| gmaan | bauzas: cool. thanks for sharing results. | 19:11 |
| dansmith | I'm honestly kinda surprised the PoC holds up that well :D | 19:11 |
| gmaan | santa clause is cool but not interested in smackdown ring :) i never liked it even in my childhood :) | 19:12 |
| dansmith | hah | 19:12 |
| gmaan | dansmith: if PoC is that good, it means we can call the final code as bullet proof :P, can you mention that in spec | 19:13 |
| dansmith | I will definitely NOT make such claims :) | 19:14 |
| gmaan | :) | 19:14 |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!