| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-02-07 | |||
| 17:11:16 | sean-k-mooney | in teh fucntional tests the libvirt python package should not be there | |
| 17:11:36 | sean-k-mooney | unless it has been baked into the ci image | |
| 17:11:56 | clarkb | it shouldn't be | |
| 17:11:58 | bauzas | sean-k-mooney: context is, the poison disappears when the thread is executed | |
| 17:12:04 | sean-k-mooney | we intentionally od not list libvirt in test-requirements.txt or requirements.txt | |
| 17:12:14 | bauzas | so the threads gets a None attribute for the import | |
| 17:12:40 | sean-k-mooney | ok so it raises an import error as we expect | |
| 17:12:49 | bauzas | to quote gibi "an existing test case that is properly poisoned and mocked libvirt. But an eventlet is leaked out from the test, the test finished and removed the mock. Then the leaked eventlet wakes up while a later test case runs and because the mock was removed in when the original test finished the leaked eventlet now imports libvirt and hits the poison set up by the current test." | |
| 17:12:53 | sean-k-mooney | that should fail the test bug im guessign we are using spawn_n | |
| 17:13:15 | gibi | bauzas: the code you injected does not help catching such case where the poison was not added | |
| 17:13:47 | gibi | bauzas: and our goal here now is to know what test leaked the eventlet | |
| 17:14:04 | gibi | to be able to reproduce the leak locally and fix it | |
| 17:14:06 | sean-k-mooney | it will just log an error with the orgianl evently id to help identigy the test that was not poisoned | |
| 17:14:09 | bauzas | gibi: yup, that's why I'm trying where to patch | |
| 17:14:43 | sean-k-mooney | gibi: is it a libvirt import in all cases or just some | |
| 17:15:04 | bauzas | sean-k-mooney: no the test that runs when the greenthread wakes up then turns into a failure | |
| 17:15:21 | bauzas | https://4dca9d38a541907e85e1-0253beca39d73a6e7192d5b32ed5edc2.ssl.cf2.rackcdn.com/860282/2/check/nova-tox-functional-py310/466e0d7/testr_results.html | |
| 17:15:28 | bauzas | (one of the many occurences) | |
| 17:15:34 | bauzas | or https://4dca9d38a541907e85e1-0253beca39d73a6e7192d5b32ed5edc2.ssl.cf2.rackcdn.com/860282/2/check/nova-tox-functional-py310/466e0d7/testr_results.html | |
| 17:15:46 | bauzas | or https://4dca9d38a541907e85e1-0253beca39d73a6e7192d5b32ed5edc2.ssl.cf2.rackcdn.com/860282/2/check/nova-tox-functional-py310/466e0d7/testr_results.html | |
| 17:16:06 | gibi | sean-k-mooney: depending on when the leaked eventlet weaks up it either hits the libvirt poison and fails the test, or just logs the stack traces and let the test passes if no poison is in place | |
| 17:16:11 | bauzas | or https://7ffaea22ff93fca2f0ea-bf433abff5f8b85f7f80257b72ac6f67.ssl.cf5.rackcdn.com/869900/7/gate/nova-tox-functional-py38/3b10d8a/testr_results.html (sorry) | |
| 17:16:28 | sean-k-mooney | gibi: ack | |
| 17:16:30 | bauzas | gibi: yup, I found some run | |
| 17:16:40 | gibi | sean-k-mooney: the poision acts like the global state the lets the leaked eventlet manipulate the running test case | |
| 17:16:51 | sean-k-mooney | gibi: an dis it spawn_n in all cases | |
| 17:17:17 | sean-k-mooney | gibi:yes but this is not a reulst of the poision its just highlighign an exisitng issue | |
| 17:17:32 | gibi | sean-k-mooney: yes the poison is good | |
| 17:17:42 | sean-k-mooney | we did have an existing thing like this related to noticiation i think in the past right | |
| 17:17:48 | gibi | sean-k-mooney: yes | |
| 17:17:50 | sean-k-mooney | and we checked the eventlet id | |
| 17:18:06 | gibi | sean-k-mooney: that embeds the testcase id to the eventlet | |
| 17:18:12 | sean-k-mooney | yep | |
| 17:18:14 | gibi | and checks it during the notification code path | |
| 17:18:17 | sean-k-mooney | which is what bauzas is logging now | |
| 17:18:18 | gibi | and that path is fixed | |
| 17:18:38 | gibi | sean-k-mooney: yes, we try to log that now for this poison / live_migration_abort() codepath | |
| 17:18:44 | bauzas | sean-k-mooney: yes, I'm trying to see what's firing the greenthread | |
| 17:18:48 | sean-k-mooney | so longterm i still wonder if we should make nova use a green pool | |
| 17:19:00 | sean-k-mooney | and then in the tests we can make each test use there own greenpool | |
| 17:19:18 | sean-k-mooney | and call wait on that in the test cleanup | |
| 17:19:53 | sean-k-mooney | i think that would be relitvly simple to do | |
| 17:20:05 | sean-k-mooney | im just not sure we want to do it 2 weeks before FF | |
| 17:20:12 | bauzas | no | |
| 17:20:17 | bauzas | please :) | |
| 17:20:30 | gibi | sean-k-mooney: we would still need a reproduce for the current failure to see that if the pooling fixes it :) | |
| 17:20:43 | bauzas | gibi: I missed your top comment | |
| 17:20:49 | sean-k-mooney | gibi: yes we would :) | |
| 17:20:50 | bauzas | I'll amend .zuul.yaml | |
| 17:21:26 | sean-k-mooney | gibi: but it would allow use to piosion direct calls to spawn/spaw_n potentially and ensure we cant leek eventlets between cases | |
| 17:21:27 | gibi | so let's get a reproducer first by figuring out the leak tests (we know that there is more than one as simply intersecting testcase lists from failed test workers did not result in a single test case but an empty list) | |
| 17:21:52 | gibi | sean-k-mooney: I'm not against fixing this via pooling :) | |
| 17:21:56 | opendevreview | Sylvain Bauza proposed openstack/nova master: DNM: Add logging for leaking out the non-poisoned libvirt testcase https://review.opendev.org/c/openstack/nova/+/872975 | |
| 17:22:59 | bauzas | gibi: sean-k-mooney: I'm not against fixing our concurrency mechanism for func tests, I'm just against doing it *now* :) | |
| 17:23:36 | gibi | I will disappeare soon. I think we can continue this tomorrow. I will look at the patch and call rechecks time to time during my evening | |
| 17:25:33 | bauzas | gibi: if only I was able to reproduce it locally, I could just call tox with -- --until-failure | |
| 17:27:03 | gibi | bauzas: yeah | |
| 17:27:21 | gibi | that is the key. If we have it locally I can add as much runtime to it as I want | |
| 17:27:32 | bauzas | anyway, have a good evening | |
| 17:27:39 | bauzas | and thanks for the help | |
| 17:27:47 | bauzas | I think I'll shortly stop too | |
| 17:28:02 | gibi | bauzas: thanks for the work, I think we made good progress today. I was not able to do that without you. | |
| 17:31:53 | opendevreview | Maksim Malchuk proposed openstack/nova stable/xena: Fix to implement 'pack' or 'spread' VM's NUMA cells https://review.opendev.org/c/openstack/nova/+/829804 | |
| 17:37:04 | opendevreview | Maksim Malchuk proposed openstack/nova stable/wallaby: Fix to implement 'pack' or 'spread' VM's NUMA cells https://review.opendev.org/c/openstack/nova/+/861832 | |
| 20:49:18 | opendevreview | Maxim Monin proposed openstack/nova master: Server Rescue leads to Server ERROR state if base image is deleted https://review.opendev.org/c/openstack/nova/+/872385 | |
| 21:06:11 | opendevreview | Balazs Gibizer proposed openstack/nova master: DNM: Add logging for leaking out the non-poisoned libvirt testcase https://review.opendev.org/c/openstack/nova/+/872975 | |
| #openstack-nova - 2023-02-08 | |||
| 01:14:50 | opendevreview | Merged openstack/nova master: Abort startup if nodename conflict is detected https://review.opendev.org/c/openstack/nova/+/872432 | |
| 01:14:58 | opendevreview | Merged openstack/nova master: Stable compute uuid functional tests https://review.opendev.org/c/openstack/nova/+/872441 | |
| 05:58:11 | opendevreview | Maxim Monin proposed openstack/nova master: Server Rescue leads to Server ERROR state if base image is deleted https://review.opendev.org/c/openstack/nova/+/872385 | |
| 07:19:20 | opendevreview | sean mooney proposed openstack/nova master: introduce global greenpool https://review.opendev.org/c/openstack/nova/+/873061 | |
| 07:30:17 | opendevreview | sean mooney proposed openstack/nova master: introduce global greenpool https://review.opendev.org/c/openstack/nova/+/873061 | |
| 07:39:33 | opendevreview | sean mooney proposed openstack/nova master: introduce global greenpool https://review.opendev.org/c/openstack/nova/+/873061 | |
| 07:52:11 | opendevreview | sean mooney proposed openstack/nova master: introduce global greenpool https://review.opendev.org/c/openstack/nova/+/873061 | |
| 08:12:39 | opendevreview | Pierre Libeau proposed openstack/nova master: Add mechanism to manage snapshot during nc init https://review.opendev.org/c/openstack/nova/+/873062 | |
| 08:15:49 | plibeau4 | hello guys, I have push https://review.opendev.org/c/openstack/nova/+/873062 to have your feedback before to start written some tests. I have explain the issue also in this bug: https://bugs.launchpad.net/nova/+bug/2006559 | |
| 08:31:46 | sean-k-mooney | gibi: bauzas when ye are around take a look at https://review.opendev.org/c/openstack/nova/+/873061 and specificaly the failing tests here https://2f622fb6915ea6772a94-26db3adf591a82ec37c96a7d3180086f.ssl.cf2.rackcdn.com/873061/4/check/openstack-tox-py310/be4df4f/testr_results.html those are the unit tests that are leaking | |
| 08:35:54 | bauzas | sean-k-mooney: okay but fwiw the leaked tests for https://launchpad.net/bugs/1946339 | |
| 08:35:55 | sean-k-mooney | ha ok i also know why | |
| 08:35:59 | bauzas | are functional tests | |
| 08:36:19 | sean-k-mooney | https://github.com/openstack/nova/blob/ea0526d959f7246c7d741ea24c207b52417d224a/nova/virt/libvirt/host.py#L497 | |
| 08:36:44 | sean-k-mooney | the test is calling h.initialize() | |
| 08:37:13 | sean-k-mooney | which calls init_event which does utils.spawn(self._dispatch_thread) | |
| 08:38:40 | sean-k-mooney | bauzas: here are the functionla failures https://storage.gra.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_3e4/873061/4/check/nova-tox-functional-py310/3e45bc4/testr_results.html | |
| 08:38:45 | sean-k-mooney | we have many more of them | |
| 08:40:15 | bauzas | sean-k-mooney: we agreed yesterday on merging it after the RC1, right? | |
| 08:40:32 | bauzas | for Bobcat I mean | |
| 08:41:40 | sean-k-mooney | maybe i created it because i tought it would find all the tests that were broken | |
| 08:43:03 | sean-k-mooney | i was more concenred with teh test fallout then any impact it woudl nova on nova in production | |
| 08:43:41 | sean-k-mooney | the code change is pretty low risk | |
| 08:44:11 | sean-k-mooney | fixign all the broken test could be a lot of work. | |
| 08:45:03 | sean-k-mooney | bauzas: the libvirt event dispatch thread is curently while True | |
| 08:45:05 | sean-k-mooney | https://github.com/openstack/nova/blob/ea0526d959f7246c7d741ea24c207b52417d224a/nova/virt/libvirt/host.py#L209-L218 | |
| 08:45:33 | sean-k-mooney | to fix that we need a way to stop that thread for the functionl tests at least | |
| 08:46:32 | sean-k-mooney | well or we need to make sure its mocked out properly | |
| 08:48:09 | sean-k-mooney | https://github.com/openstack/nova/blob/ea0526d959f7246c7d741ea24c207b52417d224a/nova/virt/libvirt/host.py#L607 is always creating 2 greenthreads that never exit if its called today | |
| 08:49:36 | sean-k-mooney | so the libvirt fixture need to be enhanced to stub that out | |
| 08:49:38 | bauzas | sean-k-mooney: the problem is that given we don't know which test is causing trouble, we won't be able to make sure your change can fix it | |
| 08:49:54 | sean-k-mooney | my patch show which tests are causing the issue | |
| 08:50:39 | bauzas | I see | |
| 08:50:45 | sean-k-mooney | bauzas: i added a fixture that detects when tests leak green threads https://review.opendev.org/c/openstack/nova/+/873061/4/nova/tests/fixtures/nova.py#1138 | |