| gibi_ | ashigupt: I will try to get to it but at the moment it is somewhere in the middle of my todo lit | 07:03 |
|---|---|---|
| gibi_ | lis | 07:03 |
| gibi_ | list | 07:03 |
| *** bauzas7 is now known as bauzas | 07:54 | |
| bauzas | hey folks, back | 08:09 |
| bauzas | gibi_: I mentioned https://review.opendev.org/c/openstack/nova/+/1005727 but this requires a bit of cleanup on my side per the last comments, fwiw | 08:16 |
| gibi_ | ack | 08:25 |
| gibi_ | I push what I have and then start looking at yours | 08:26 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Fix PCI allocation leak on failed cold migration https://review.opendev.org/c/openstack/nova/+/1006003 | 09:00 |
| gibi_ | bauzas: that is mine ^^ it reuses an existing RPC call originally used for live migration during cold migration to clean up the leaked MoveClaim | 09:01 |
| bauzas | ack | 09:01 |
| gibi_ | bauzas: left questions in the fix | 09:13 |
| bauzas | ty, still trying to untangle all the potential issues | 09:13 |
| gibi_ | ack | 09:14 |
| *** gibi_ is now known as gibi | 09:14 | |
| bauzas | as a reminder, the best solution is something persisted | 09:14 |
| bauzas | but still trying to find a short-term fix for being backportable shortly | 09:14 |
| gibi | ahh and I realized that I asked the same question as Dan | 09:15 |
| gibi | about how the two parallel request can overlap | 09:15 |
| bauzas | np | 09:15 |
| gibi | OK moved my comment to the proper place | 09:26 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Reproduce bug 2166786 https://review.opendev.org/c/openstack/nova/+/1004658 | 09:27 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Fix PCI allocation leak on failed cold migration https://review.opendev.org/c/openstack/nova/+/1006003 | 09:27 |
| opendevreview | Sylvain Bauza proposed openstack/nova master: Guard against deleting a concurrent request's BDM on timeout https://review.opendev.org/c/openstack/nova/+/1005727 | 10:38 |
| opendevreview | Sylvain Bauza proposed openstack/nova master: Reject stale reserve_block_device_name RPC on compute https://review.opendev.org/c/openstack/nova/+/1006011 | 10:38 |
| opendevreview | Merged openstack/placement master: pre-commit: Add codespell https://review.opendev.org/c/openstack/placement/+/931806 | 11:36 |
| gibi | gmaan: does this clear_events_for_instance needs to run for failed resizes as well https://github.com/openstack/nova/blob/94de0576ca02e0668233bedfbe7e18b87316c708/nova/compute/manager.py#L6418 ? E.g. if driver.migrate_disk_and_power_off raises do we leak a pending resize task? | 13:11 |
| bauzas | gibi: gmaan: I barely paid attention to the gate issues with threading, but I just saw that one https://storage.gra.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_c5e/openstack/c5e5e75ba3ba4ea1bbc056c4f9299476/testr_results.html | 13:52 |
| bauzas | context : https://review.opendev.org/c/openstack/nova/+/1005727 | 13:53 |
| gibi | https://bugs.launchpad.net/nova/+bug/2165149 | 13:55 |
| gibi | https://review.opendev.org/c/openstack/nova/+/1003586 | 13:56 |
| opendevreview | ribaudr proposed openstack/nova master: Fix swap disk creation skipped on NFS during cold migration https://review.opendev.org/c/openstack/nova/+/1006036 | 13:56 |
| gibi | bauzas: that is also one of the todos I have | 13:56 |
| bauzas | ack ok | 13:56 |
| bauzas | I didn't wanted to blindly recheck | 13:56 |
| gibi | we have open bug and a local fix that does not work on the gate :) | 13:56 |
| bauzas | gibi: great questions by the way on my second patch for preveting BDM creation after RPC timeout | 14:00 |
| bauzas | I wasn't having that level of expertise, you're pushing me into looking at all the oslo.service details :) | 14:00 |
| gibi | yeah my feeling is that we are getting deeper and deeper into a rabbithole | 14:00 |
| gibi | but I haven't had the brain power to offer a real backportable alternative | 14:01 |
| bauzas | fwiw, I care about the first patch in the series, I just offered help for the other patch | 14:02 |
| bauzas | if we don't find a short-term solution for the BDMs being created without calling Cinder, I'm all good | 14:03 |
| bauzas | I just want nova to not delete the wrong BDM (the other case) :-) | 14:03 |
| gibi | OK. we can refocus on the first | 14:03 |
| gibi | I understand your priorities ;) | 14:03 |
| gibi | btw mine is also more complicated than what I pushed. we might want to fixes on that prevent the problem to reapper(that I pushed) and a fix in the periodic to clear the old leaks | 14:04 |
| gibi | *two fixes one that | 14:04 |
| gibi | so I'm poking at the periodic | 14:05 |
| gibi | while fighiting a race condition in the original reproduce as we revert stuff in an untestable order | 14:05 |
| gibi | aaand I will drop for a wile in 25mins so running out of time too | 14:06 |
| bauzas | yeah I need some brain cycles now to look at your patch | 14:06 |
| gibi | if you look just focus on the RPC implications, the rest might change | 14:09 |
| bauzas | I need to load a bit of context, that's it | 14:09 |
| eduardomorais[m] | Could someone review this PR and give me some tips? I'm new to contributing, thanks in advance! https://review.opendev.org/c/openstack/nova/+/1005881 | 15:48 |
| dansmith | eduardomorais[m]: reviewing other people's submissions is a good way to get familiar with the codebase _and_ get yourself karma you can use for other purposes. i.e. find some (legitimate) thing to -1 :) | 17:18 |
| gmaan | gibi: yes, resize has the sane issue of task leak in source resize_instance error in between. dest call resize_instance on source as 'RPC cast' so dest does not get to know if source failed as source does not call dest. As this is prep_resize time the new VM is not yet created on dest so we are good on dest host but task end is not done. | 17:21 |
| gmaan | revert_resize has the same which not jsut leak task but make VM.host in unrecoverable state. i reported that bug https://bugs.launchpad.net/nova/+bug/2162090 | 17:21 |
| gmaan | I am going to handle these error cases between source and dest as a genric way in part-3. | 17:23 |
| gmaan | gibi: if you want I can make previously listed bug as more generic for all migration tasks instead of just live migration task leak at dest. this one https://review.opendev.org/c/openstack/nova/+/1003386/5/releasenotes/notes/nova-graceful-shutdown-task-tracking-13485c8346512ae4.yaml#23 | 17:24 |
| dansmith | gmaan: do you have an easy way to disable waiting for graceful shutdown in functional workers? the thing I'm working on seems like it's getting stuck a *lot* and I suspect maybe it's because of graceful shutdown | 17:27 |
| dansmith | maybe set the wait conf knob to zero by default as a test? | 17:27 |
| gmaan | dansmith: we have config knob already set to 0 in configfixture so it should not stuck https://github.com/openstack/nova/blob/master/nova/tests/fixtures/conf.py#L65 | 17:29 |
| dansmith | okay but I see lots of messages about it waiting | 17:29 |
| dansmith | like he thread pool nova.tests.functional.test_servers.ServerMovingTestsWithNestedResourceRequests.test_resize_revert.default is shutting down | 17:30 |
| dansmith | is that just it noting that it's shutting down but not waiting? (I haven't looked to verify) | 17:30 |
| dansmith | it always seems to come after a long pause so I figured those were the ones that eventually did shut down after waiting | 17:30 |
| gmaan | oh, are you seeing hang or just logs. because we have noisy logs of thread pool and I am fixing those for tests https://review.opendev.org/c/openstack/nova/+/1004160 | 17:32 |
| gmaan | actual shutdown should not take time as we do not wait for in-progress tasks except in some graceful shutdown tests | 17:33 |
| dansmith | I'm seeing hangs, trying to determine why they're hanging and suspecting graceful shutdown (not my glorious code of course!) | 17:34 |
| dansmith | okay | 17:34 |
| gmaan | humm, can you check what is manager_shutdown_time value there (by printing ro so). note: graceful_shutdown_timeout is kept default which is 180 sec i hope nothing stuck after manager return immidiatly | 17:36 |
| gmaan | dansmith: one thing can take time is thead pool logs taking time, if anything still running in executors the it will wait | 17:37 |
| dansmith | yeah I'm waiting well past 180s to determine they're stuck, but I was thinking maybe there was some deadlock we're hitting that made the timeout not relevant.. I haven't figured out which test is the problem yet because all the candidates seem to run fine when run alone | 17:38 |
| dansmith | hence me suspecting some interaction | 17:38 |
| gmaan | i see. i remember oslo.service does not timeout the shutdown based on graceful_shutdown_timeout as there is no releah SIGTERM handler called on test process between tests | 17:40 |
| gmaan | so we basically retun from manager quickly with 0 sec wait but if something still running in exevcutor and hang then it will hang the shutdown also | 17:41 |
| gmaan | we does not stop without wait as we want to cathc the bad tests (not your glorious code of course :)) who leak the things - https://github.com/openstack/nova/blob/master/nova/tests/fixtures/nova.py#L1353 | 17:42 |
| gmaan | we fixed a few tests which were hanging and not always but when they face some race so there might be some more but not seen yet in CI | 17:44 |
| dansmith | reset_all_executors() will kill and continue not hang? | 17:53 |
| gmaan | no, it stop executor with wait=True so anything in executro queue will be finished | 17:53 |
| dansmith | but with timeout or no? | 17:54 |
| dansmith | (heh I see the comment right above, sorry) | 17:54 |
| gmaan | no timeout, because oslo.service SIGTERM handler does not timeout | 17:54 |
| gmaan | I try to kill them during cleanup and not wait but then liked the gibi idea to wait and let test hang to see why test leaving the things in-progress | 17:55 |
| gmaan | which exact tests hang or is it many tests take times | 17:57 |
| opendevreview | Ghanshyam Maan proposed openstack/nova master: Update graceful shutdown known issue to list resize case also https://review.opendev.org/c/openstack/nova/+/1006095 | 18:28 |
| gmaan | gibi: I updated the existing bug description to cover the resize case - https://bugs.launchpad.net/nova/+bug/2166009 | 18:37 |
| gmaan | gibi: dansmith ^^ and updating the 'known issue' in releae notes | 18:37 |
| opendevreview | Eduardo Henrique Lima de Morais proposed openstack/nova master: Enforce nullable=False on instance_uuid https://review.opendev.org/c/openstack/nova/+/1005881 | 22:01 |
| eduardomorais[m] | dansmithSorry for the late reply. I'll take a look at some other PRs to get a better understanding of how things work. Thanks for the help and the tips! | 22:11 |
| JayF | eduardomorais[m]: it's worth noting the PTG (Project Teams Gathering) planning session is happening soon (ptg.opendev.org) -- and that would be a good place in many projects (I assume this is true for Nova as well) to find something to volunteer to work on | 22:13 |
| JayF | eduardomorais[m]: IDK if you are just looking to drive by contribute or what, but if you want a longer engagement, attending PTG sessions of those projects is a GREAT way to get attention | 22:13 |
| eduardomorais[m] | JayFI'd like to start contributing a bit more to OpenStack services. I currently work as an OpenStack operator, and I'd love to learn more about the codebase and help improve it. I'll definitely keep an eye out for the PTG, thanks for the tip! | 22:18 |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!