| opendevreview | Merged openstack/os-vif master: Fix config option registration https://review.opendev.org/c/openstack/os-vif/+/1003173 | 00:51 |
|---|---|---|
| opendevreview | Adam McArthur proposed openstack/nova master: Use safe allocation deletion on instance delete https://review.opendev.org/c/openstack/nova/+/1003208 | 04:07 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Support cold migration with virtiofs shares https://review.opendev.org/c/openstack/nova/+/989633 | 07:11 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Reconcile stale Manila share access rules https://review.opendev.org/c/openstack/nova/+/1001916 | 07:11 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Support cold migration with virtiofs shares https://review.opendev.org/c/openstack/nova/+/989633 | 07:38 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Reconcile stale Manila share access rules https://review.opendev.org/c/openstack/nova/+/1001916 | 07:38 |
| opendevreview | Lajos Katona proposed openstack/nova master: Use SDK for Neutron subnets https://review.opendev.org/c/openstack/nova/+/962190 | 07:48 |
| opendevreview | Lajos Katona proposed openstack/nova master: Use SDK for Neutron extensions https://review.opendev.org/c/openstack/nova/+/962270 | 07:48 |
| *** Gues__________________________ is now known as mmagr | 08:05 | |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Support cold migration with virtiofs shares https://review.opendev.org/c/openstack/nova/+/989633 | 08:07 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Reconcile stale Manila share access rules https://review.opendev.org/c/openstack/nova/+/1001916 | 08:07 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Support cold migration with virtiofs shares https://review.opendev.org/c/openstack/nova/+/989633 | 08:28 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Reconcile stale Manila share access rules https://review.opendev.org/c/openstack/nova/+/1001916 | 08:29 |
| gibi | gmaan: https://github.com/openstack/nova/blob/d3a1c95d06c2d97368207c2728fc262796683ad8/nova/service.py#L435 this combined with the _get_manager_shutdown_timeout returning 0 is a big trap in the functional tests. It means test cases leaving a thread behind that finishes after the test case finished and a new one starts and they are calling things on the manager that calling things on the global | 08:33 |
| gibi | executor state. | 08:34 |
| gibi | I filed https://bugs.launchpad.net/nova/+bug/2165991 | 08:40 |
| gibi | btw new hit on TIMED_OUT and no new hit on DB locked. But I still checking out the rest of the functional-threading failures happend yesterday (zuul is non cooperative): | 08:54 |
| gibi | I mean no new hit | 08:55 |
| gibi | I belive the DB locked issue is real as I saw it locally once or twice in extreme scenarios | 08:55 |
| gibi | (extrem means by introducing lot of additional DB churn) | 08:55 |
| gibi | but still no stable reproducer | 08:56 |
| gibi | I have one suspicious functional-threading fauilure from yesterday https://5125be418d93521dc669-217c5d59ee9178b06ecc76d8426e75cb.ssl.cf1.rackcdn.com/openstack/1ff94c6e70154943aae8d53d58fbcdd5/testr_results.html I will look deeper into it | 09:03 |
| opendevreview | Ashish Gupta proposed openstack/nova master: Fix init_host crash migration test under native threading https://review.opendev.org/c/openstack/nova/+/1001369 | 09:33 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: [test]Ensure shutdown thread is waited on https://review.opendev.org/c/openstack/nova/+/1003245 | 10:36 |
| gibi | gmaan: ^^ fix for the above reported bug | 10:36 |
| gibi | gmaan: I will put a follow up on top with 3 functional test fixed that haning the shutdown | 10:37 |
| gibi | gmaan: also if I see that graceful shutdown logs a task as live_migration_at_dest which RPC handler it is? post...at_dest? rollback...at_dest? | 10:52 |
| gibi | I started logging stack trace at _record_task_start and I see something strange | 11:14 |
| gibi | the _record_task_start live_migration_at_dest actually happens from File "/home/gibi/upstream/git/openstack/nova/nova/compute/manager.py", line 9431, in pre_live_migration | 11:15 |
| gibi | is this correct that we name the task live_migration_at_dest when it is actually a pre_live_migration RPC? | 11:15 |
| gibi | we need at least some help for devs to know what hangs when we log that | 11:18 |
| gibi | as it was a pretty long goose chase for me to find out that pre_live_migration records a task named live_migration_at_dest but that task only finished when the whole live migration terminated. | 11:19 |
| gibi | which make sense after I discovered all this but was pretty strange when I did not know about that logic | 11:19 |
| gibi | btw I think we have a path that rolls back the live migration but never finishes the live_migration_at_dest task | 11:23 |
| gibi | filing a bug and then we can discuss if it is a realistic scenario | 11:23 |
| gibi | (it is hit by a functional test so it is at least triggerable) | 11:23 |
| gibi | (I cannot open a bug as launchpad is dead) | 11:34 |
| gibi | so https://github.com/openstack/nova/blob/d3a1c95d06c2d97368207c2728fc262796683ad8/nova/compute/manager.py#L10553-L10562 here if the code takes the else branch as do_cleanup is false then the task will never finish | 11:35 |
| gibi | https://bugs.launchpad.net/nova/+bug/2166009 | 12:00 |
| sean-k-mooney | _rollback_live_migration can be called in _cleanup_pre_live_migration and 2 places in _do_live_migration before and after the migrtion is called | 12:00 |
| sean-k-mooney | i dont really recall off the top of my head what the critia for do_cleanup is | 12:01 |
| sean-k-mooney | looks like its determeidn by https://github.com/openstack/nova/blob/d3a1c95d06c2d97368207c2728fc262796683ad8/nova/compute/manager.py#L9968 _live_migration_cleanup_flags | 12:01 |
| sean-k-mooney | i guess its tryign to figure out if its on shared storage? | 12:01 |
| sean-k-mooney | but we defintly dont want to alwasy do that | 12:02 |
| sean-k-mooney | do you think its only a testing issue or a production one | 12:02 |
| gibi | I feel like this is a real code path we can take | 12:04 |
| gibi | and it result in a task kept pending forever in the graceful shutdown task tracker | 12:04 |
| sean-k-mooney | yes if we are on shared storage i think we will take it (nfs) | 12:04 |
| sean-k-mooney | or perhasp if we fail in pre-live migrtion | 12:06 |
| sean-k-mooney | as there is nothign to clean up on the dest | 12:06 |
| gibi | the complication here as far as I see is that this task is in the dest compute's tracker and the source computes rolls the migration back and never calls the dest compute so the dest compute cannot finish the task | 12:07 |
| sean-k-mooney | it looks like do_cleanup, destroy_disks are only ever used inside that if | 12:07 |
| sean-k-mooney | well should it be in the dest tracker at that point | 12:08 |
| sean-k-mooney | i havent fully follosed you but if it roling back before the call to pre live migrate on dest it wont be there | 12:08 |
| gibi | if you want to trace it there are at least two function test cases that hitting the codepath (noted in the bugreport) | 12:09 |
| sean-k-mooney | so the edge case would have to haappend form pre live migratoin at soruce or the call to migrateToURI3 at the libivrt level | 12:09 |
| sean-k-mooney | if we are seing live_migration_at_dest i think that impleise we are rooling back at the libvirt migrate call | 12:11 |
| gibi | test nova.tests.functional.regressions.test_bug_1888395.TestLiveMigrationRollbackWithoutMultiplePortBindings this is the cleaner repro and it trigger dom.fail_job() | 12:11 |
| sean-k-mooney | as in this is happen right at the point of no return since we do not supprot rollback if that call complete without error | 12:12 |
| sean-k-mooney | thats still a little impresices we do not supprot calling rollabck if that libvirt job that was started by that call complete without failing | 12:13 |
| gibi | this also triggers the path and it uses abort not rollback nova.tests.functional.notification_sample_tests.test_instance.TestInstanceNotificationSampleWithMultipleCompute._test_live_migration_abort | 12:16 |
| sean-k-mooney | gibi: so we are recordign the task start here https://github.com/openstack/nova/blob/d3a1c95d06c2d97368207c2728fc262796683ad8/nova/compute/manager.py#L9426-L9432 | 12:17 |
| sean-k-mooney | but never callign https://github.com/openstack/nova/blob/d3a1c95d06c2d97368207c2728fc262796683ad8/nova/compute/manager.py#L9441-L9444 | 12:18 |
| gibi | yeah | 12:18 |
| gibi | I think rollback at dest and post at dest call that helper that finishes the task | 12:18 |
| gibi | but in this scenario there is no rpc to the dest that can call that helper | 12:18 |
| sean-k-mooney | if we just passed destory disks false when we woudl have had do_cleanup=false | 12:19 |
| gibi | the source handles the rollback / abort alone as far as I see | 12:19 |
| sean-k-mooney | it might be safe | 12:19 |
| sean-k-mooney | i.e. if we just alwasy call cleanup at des when not pre_live_migation | 12:20 |
| sean-k-mooney | actully no its recoreded in pre_live_migration | 12:20 |
| sean-k-mooney | so we proably woudl want to alwasy call it if we can make sure its non distuctive for shared sotrage | 12:20 |
| sean-k-mooney | after all this is being called or not called form the souce and the task is beint gract on the dest right | 12:21 |
| sean-k-mooney | so we cant just do _end_pending_dest_live_migration_task since that woudl remvoe it form our local dictionatry | 12:22 |
| gibi | yeah the livemigartion failure detected on the source but this task is pending at the dest | 12:22 |
| gibi | if the source can alway send and rpc to the dest then that rpc can remove the pending task on the dest | 12:23 |
| sean-k-mooney | i think instead of tghe if you could and do_cleanup with detstory_disks | 12:24 |
| sean-k-mooney | the remainign question is are any of the actions in https://github.com/openstack/nova/blob/d3a1c95d06c2d97368207c2728fc262796683ad8/nova/compute/manager.py#L10658-L10731 soemthing that are unsafe to attpemt alwasys | 12:25 |
| sean-k-mooney | i think the network cleanup is fine | 12:26 |
| sean-k-mooney | we defintly dont want to leak portbindings and do want to make suere we have cleaned up any ports | 12:26 |
| sean-k-mooney | freeing pci claims is also somthign we woudl prefr to do rather then waith for the resouce tracker | 12:27 |
| sean-k-mooney | its possibel we failed before they were claimed btu i tough it and we coudl handel that anyway | 12:27 |
| sean-k-mooney | so i think the manger level tasks all look ok | 12:27 |
| sean-k-mooney | we also liekly do want to do most if not all of this https://github.com/openstack/nova/blob/d3a1c95d06c2d97368207c2728fc262796683ad8/nova/virt/libvirt/driver.py#L12040-L12085 | 12:29 |
| sean-k-mooney | the domain may or may not exist on the dest | 12:30 |
| sean-k-mooney | but thats ok | 12:30 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: [func test]Catch hanging task at graceful shutdown https://review.opendev.org/c/openstack/nova/+/1003266 | 12:30 |
| gibi | this is basically a poor man's reproducer ^^ | 12:30 |
| gibi | I did not dig deep enough to say for sure but it sounds like an unconditional rpc to the dest during rollback is a potential solution for this | 12:32 |
| gibi | I let gmaan to chime in :) | 12:32 |
| gibi | and now that https://review.opendev.org/c/openstack/nova/+/1003245 I can go back to try to reproduce the DB locked case | 12:32 |
| sean-k-mooney | https://paste.opendev.org/show/borJKJA9ol8nnqWHqBru/ | 12:33 |
| sean-k-mooney | i would be tempted to do that | 12:33 |
| gibi | yeah it is not a bad idea on the surface for sure but I did not dig deep enough to say it has no unwanted side effect | 12:34 |
| sean-k-mooney | right i did skim pass torugh the manger eand driver but not enough to say it correct | 12:35 |
| sean-k-mooney | but its proably worht a try when we have more time | 12:35 |
| gibi | yepp | 12:35 |
| gibi | I hope gmaan will have the time to dig deeper | 12:35 |
| sean-k-mooney | ill comment on the bug and go back to post cybrog and watcehr release stuff | 12:36 |
| sean-k-mooney | at least in nova we have enough fucntial testign of live migraton to validate much fo this | 12:37 |
| sean-k-mooney | i.e. without going all the way to tempest and bespoke devstack deployment to verify all the edgecases | 12:38 |
| gibi | bah one more functional-threading instability (seen 3 times now) https://bugs.launchpad.net/nova/+bug/2166014 | 12:39 |
| gibi | sean-k-mooney: thanks for checking it out | 12:40 |
| *** mhen_ is now known as mhen | 13:53 | |
| gmaan | gibi: sean-k-mooney yes, live migration at DEST task start at "pre_live_migration" (which is on 2nd RPC server) and ends at two place 1. post_live_migration_at_destination 2. rollback_live_migration_at_destination so normal RPC rollbackl is covered (the if part) | 15:58 |
| gmaan | but _rollback_live_migration() happening on source and (in else block) it does not notify the dest about it and task on dest is not marked as ended | 15:59 |
| gmaan | so yea it is bug in this code path. solution is not easy because task is on dest (tracking list on dest process) and source cannot end that due to separate process | 16:00 |
| gmaan | let me dig into it more today | 16:00 |
| gmaan | btw thanks gibi for the functional tests bug https://bugs.launchpad.net/nova/+bug/2165991. just to make sure that this is not production issue as that thread is daemon and will go away with process. but agree to fix it in functional tests | 16:01 |
| gmaan | i really wanted to move all unit/functional tests pass with defaults, I will keep checking your series of fixing hanging tests or spend my time also to find/fix a few | 16:02 |
| sean-k-mooney | gmaan: well it can if it can safely call the rpc | 16:02 |
| sean-k-mooney | i think it can safely call the rpc unconditionally | 16:02 |
| sean-k-mooney | but we need to confirm | 16:02 |
| sean-k-mooney | there is no way to od it localy however | 16:02 |
| gmaan | yeah, RPC call will work but we have these kind of communication gap between source and dest, this live migration sand other i found in revert_resize and those can be handled in part-3 where we can try to make operations more self-healing or at least abort in good state | 16:04 |
| sean-k-mooney | worst case we will have to add a new rpc jsut for this case to notify the dest | 16:04 |
| gmaan | yeah | 16:04 |
| gmaan | let me dig into it today if that is similar to revert_resize case and ok to move to part-3 or has to be handled in this part-2 | 16:05 |
| sean-k-mooney | if we added a new rpc we might be able to make it a littel mroe generic i.e notify_task_compelte | 16:05 |
| gmaan | I will not add task complete or so but call per operation cleanup as actual operation is hanging in other node and it is not just task tracking | 16:06 |
| sean-k-mooney | as long as we have a solution for the test senario for now movign the larger change to pahse 3 i think is ok | 16:06 |
| gmaan | this is revert_resize case where dest can leave things in half way without notifying/doing revert at source https://bugs.launchpad.net/nova/+bug/2162090 | 16:07 |
| gmaan | and looks like this live migration issue is on similar issue. and i will not be surprise if we will have more such cases where source/dest are not talking to each other during failure and cleanup completely at both side | 16:08 |
| * gibi cannot focus here now due to downstream call | 16:11 | |
| gibi | ust to make sure that this is not production issue as that thread is daemon and will go away with process. but agree to fix it in functional tests | 16:17 |
| gibi | I disagree ^^ | 16:17 |
| gibi | this will make the gracefull shutdown a lot slower than it should be as it will wait for the timeout value | 16:18 |
| gibi | and it will be a warning log during graceful shutdown that sends support to a goose chase about a in progress migration that is not in progress | 16:19 |
| gmaan | is it but, it will not wait for timeout as graceful shutdown will terminate the process and thread will be stopped right? | 16:19 |
| gibi | if you sent the timeout to 300 then the test fill time out as it waits for the whole 300 seconds and that is more than the test case timeout | 16:20 |
| gibi | so it is waiting for the the timeout as it thinks there is a task to wait for | 16:20 |
| gibi | and when times out it exists | 16:20 |
| gibi | but that means we wait a 180 seconds by default | 16:21 |
| gmaan | yes for functional tests as process is not exit so i agree to fix the live migration task hanging. | 16:21 |
| gmaan | the live migration task left is I agree is the production bug but I am talking about hthttps://bugs.launchpad.net/nova/+bug/2165991 though it is because of migration task hanging | 16:22 |
| gibi | similarly the dest nova-compute will not exit until 180sec as it thinks there is an in progress live migration | 16:22 |
| gmaan | I think I mixed both in my question | 16:22 |
| gibi | I think that two things are the same :) | 16:23 |
| gibi | feel free to rename the bug title | 16:23 |
| gibi | to talk about the leaked task | 16:23 |
| gibi | the hanging functional test is just a symptom | 16:23 |
| gmaan | gibi: my question was if anything we need to fix on shutdown_thread creation and when it fail/timeout | 16:23 |
| gmaan | gibi: agree on that | 16:23 |
| gmaan | I am saying this part is all ok https://github.com/openstack/nova/blob/d3a1c95d06c2d97368207c2728fc262796683ad8/nova/service.py#L433-L435 | 16:24 |
| gmaan | and yes we need to fix that task leak | 16:24 |
| gibi | it is a trap for functional test due to the thread can leak, but I can cirumwent that with setting a non 0 timeout to wait for that thread | 16:25 |
| gibi | and when I set that timeout to 300 I saw the task leak in 3 functional tests I reported as the bug | 16:26 |
| gmaan | ack | 16:27 |
| gibi | also now I see that setting 300sec timeout also make a bunch of test sad in the eventlet job | 16:28 |
| gibi | so we might have more leaks of we might have differently hanging shutdown in eventlet job | 16:29 |
| gibi | https://review.opendev.org/c/openstack/nova/+/1003266/1 | 16:29 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Support cold migration with virtiofs shares https://review.opendev.org/c/openstack/nova/+/989633 | 16:42 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Reconcile stale Manila share access rules https://review.opendev.org/c/openstack/nova/+/1001916 | 16:42 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Reconcile stale Manila share access rules https://review.opendev.org/c/openstack/nova/+/1001916 | 16:56 |
| Uggla | dansmith, melwitt, gouthamr comment about the virtiofs migration and FFE: https://review.opendev.org/c/openstack/nova/+/1001916/comments/965cff03_8b75df37 | 17:09 |
| gouthamr | \o/ ty Uggla | 17:09 |
| Uggla | gouthamr, dansmith, melwitt I think Melanie is on PTO until tomorrow. I hope Melanie could jump on this review as soon as she will be back. | 17:11 |
| Uggla | gouthamr, dansmith, melwitt also forget to mention that the msg above contains a short summary of the tests I did. | 17:13 |
| gouthamr | ++ i read them, ty for testing those scenarios. I will try and add a multi-node job with cephfs to the manila gate, and one with LVM/NFS to the nova gate (and spread the test coverage between projects) | 17:14 |
| Uggla | gouthamr, it will be great. Just to note I also tried several way to break it, and it looks solid so far. | 17:16 |
| gouthamr | Uggla: ++ :) ty! | 17:41 |
| gmaan | gibi: for eventlet functional job, you mean tox cover jobs or nova-tox-functional-py313 or both? bth failing though but I am seeing only one failure in nova-tox-functional-py313 | 17:56 |
| melwitt | Uggla: thanks for adding the comment, I will be reading over everything | 19:33 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Use a per-instance cephx identity for CephFS shares https://review.opendev.org/c/openstack/nova/+/1001900 | 20:16 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Extract share management from ComputeManager https://review.opendev.org/c/openstack/nova/+/999349 | 20:17 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Support cold migration with virtiofs shares https://review.opendev.org/c/openstack/nova/+/989633 | 21:14 |
| opendevreview | Goutham Pacha Ravi proposed openstack/nova master: Reconcile stale Manila share access rules https://review.opendev.org/c/openstack/nova/+/1001916 | 21:16 |
| opendevreview | Ghanshyam Maan proposed openstack/nova master: Fix task leak for live migration rollback case https://review.opendev.org/c/openstack/nova/+/1003386 | 21:21 |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!