)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c167066f003942b9a8c0233fe77f3623f5b498d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"242139be_5d939cfe","updated":"2026-09-23 08:30:21.000000000","message":"recheck \n\nunit test now shows a leaking task. that is probably a good find but not the one we are looking for as we saw functional hangs not unit test hangs. Strange that this does not hit any functional failures due to the new detection.","commit_id":"d9c969e75a718abd6e3fa9e49719887109ac01ef"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d4fe6e3fc71b929e3775ae71d7e62434a70d91cb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b840f773_262f6b2a","updated":"2026-09-23 05:29:20.000000000","message":"recheck image mirror issue","commit_id":"d9c969e75a718abd6e3fa9e49719887109ac01ef"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"cd47536d6ddafb09abe1457725f5e70914361e43","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"1a17f5b4_047b5f9a","updated":"2026-09-23 13:02:40.000000000","message":"recheck still hunting for failing / hanging functional test case","commit_id":"d9c969e75a718abd6e3fa9e49719887109ac01ef"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"03c4b46656bcc6de52ccc4ee546aa53d233a1bf7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9d331561_c70749bc","in_reply_to":"242139be_5d939cfe","updated":"2026-09-24 05:07:11.000000000","message":"yeah, unit test catch is valid one and that test is leaking the 2 tasks in queue.\n\nFor functional tests, I am also surprised how it is passing; instead, it should fail if cleanup is hanging for more than 10 sec. One suspect I am thinking is that if any task running submit the another task and if that 2nd task (basically WorkItem[1]) is submitted after shutdown, then executor does not raise error to test thread instead, it store it in future object[2] which does not be coming to test thread and so it does not fail. I am adding a cathing logic for those if any test doing it.\n\nALso, let me add some log around cleanup if test is passing silently.\n\n[1] https://github.com/openstack/futurist/blob/de8158746c66314b49928fb3d1980438d5466f50/futurist/_futures.py#L250\n\n[2] https://github.com/openstack/futurist/blob/de8158746c66314b49928fb3d1980438d5466f50/futurist/_utils.py#L81","commit_id":"d9c969e75a718abd6e3fa9e49719887109ac01ef"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6c1974a7db5ba20920be08d671014538fae4a617","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ed53f61d_079d1d82","in_reply_to":"9d331561_c70749bc","updated":"2026-09-24 06:57:12.000000000","message":"one thing I realized yesterday that at least one of the functional hangs are not due to a leaked never finishing thread, but a dead lock. See @rene.ribaud@gmail.com\u0027s work here https://review.opendev.org/c/openstack/nova/+/1006935","commit_id":"d9c969e75a718abd6e3fa9e49719887109ac01ef"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d1096e2d6de480ce6446ba4601c6b851999c8dcd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"9545e5e0_ac8f89c4","updated":"2026-09-30 00:54:03.000000000","message":"still trying to find the timeout rootcause occuring in https://review.opendev.org/c/openstack/nova/+/1004160","commit_id":"d908e48e43ac730a192d28d97ba0cbbe63c6457e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d4d5996d59fdb59b1c9fdeba6f653ddc2b0172f8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"37ae7908_a3f6d27e","updated":"2026-09-30 06:31:04.000000000","message":"need more updates as per review comments.","commit_id":"55861f1e83fb9233913eac9497041a752b04b4ce"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2a41bd683a48da94dd8edbc181ccb1c21a7ba806","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"0b7671f6_d022d1bc","updated":"2026-10-05 09:34:31.000000000","message":"couple of small tnings","commit_id":"4a6574fea1111d1e066a9350440895bc3d4cd041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4d0e0e9bfb75e55b5465bf6f246941a028440601","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"6650cf77_d9d2f774","updated":"2026-10-07 18:51:40.000000000","message":"A couple of suggestions inline but nothing critical so I\u0027ll +2","commit_id":"7f9dbeb5e408489daaf7205a58c09ee9d82d85bb"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"854212e4fcc740746991031c4d940d6d660edd62","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"9c4ad71e_252b461c","updated":"2026-10-08 02:09:24.000000000","message":"Thanks Dan for review","commit_id":"7f9dbeb5e408489daaf7205a58c09ee9d82d85bb"}],"nova/tests/fixtures/nova.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"75676749702fac80f22664902239365f19f1af55","unresolved":true,"context_lines":[{"line_number":1359,"context_line":"    # all the executors test created."},{"line_number":1360,"context_line":"    EXECUTORS_SHUTDOWN_TIMEOUT \u003d 10"},{"line_number":1361,"context_line":""},{"line_number":1362,"context_line":"    def _reset_all_executors(self, executors):"},{"line_number":1363,"context_line":"        threading_executors \u003d ["},{"line_number":1364,"context_line":"            e for e in executors if isinstance(e, futurist.ThreadPoolExecutor)"},{"line_number":1365,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":1,"id":"98d310d1_113e5b81","line":1362,"updated":"2026-09-21 08:29:10.000000000","message":"Can we add a shutdown(wait\u003dFalse) as a first step just to make sure no new tasks can be submitted while we wait for the existing ones?","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d273d1ad789c91c7bdd37157d09f235da6e5d718","unresolved":false,"context_lines":[{"line_number":1359,"context_line":"    # all the executors test created."},{"line_number":1360,"context_line":"    EXECUTORS_SHUTDOWN_TIMEOUT \u003d 10"},{"line_number":1361,"context_line":""},{"line_number":1362,"context_line":"    def _reset_all_executors(self, executors):"},{"line_number":1363,"context_line":"        threading_executors \u003d ["},{"line_number":1364,"context_line":"            e for e in executors if isinstance(e, futurist.ThreadPoolExecutor)"},{"line_number":1365,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":1,"id":"a9a71129_a8ca03ee","line":1362,"in_reply_to":"98d310d1_113e5b81","updated":"2026-09-23 03:41:01.000000000","message":"yeah, that is needed. done","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"75676749702fac80f22664902239365f19f1af55","unresolved":true,"context_lines":[{"line_number":1361,"context_line":""},{"line_number":1362,"context_line":"    def _reset_all_executors(self, executors):"},{"line_number":1363,"context_line":"        threading_executors \u003d ["},{"line_number":1364,"context_line":"            e for e in executors if isinstance(e, futurist.ThreadPoolExecutor)"},{"line_number":1365,"context_line":"        ]"},{"line_number":1366,"context_line":""},{"line_number":1367,"context_line":"        def _get_executors_with_pending_tasks():"}],"source_content_type":"text/x-python","patch_set":1,"id":"bb25c73f_c1677471","line":1364,"updated":"2026-09-21 08:29:10.000000000","message":"I think this is fine right now, but we need to be careful as we have wrappers like https://github.com/openstack/nova/blob/94de0576ca02e0668233bedfbe7e18b87316c708/nova/virt/libvirt/host.py#L550-L552 Fortunately that still wraps a normal ThreadPoolExecutor handled here, and the thread it starts is not in the executor and marked daemon.\n\nBtw, independently form this change we might need a fixture that stop that wrapper if started at the end of every test case otherwise we can have surprises. :)","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2a41bd683a48da94dd8edbc181ccb1c21a7ba806","unresolved":false,"context_lines":[{"line_number":1361,"context_line":""},{"line_number":1362,"context_line":"    def _reset_all_executors(self, executors):"},{"line_number":1363,"context_line":"        threading_executors \u003d ["},{"line_number":1364,"context_line":"            e for e in executors if isinstance(e, futurist.ThreadPoolExecutor)"},{"line_number":1365,"context_line":"        ]"},{"line_number":1366,"context_line":""},{"line_number":1367,"context_line":"        def _get_executors_with_pending_tasks():"}],"source_content_type":"text/x-python","patch_set":1,"id":"dc3205a0_3a0470e6","line":1364,"in_reply_to":"39f0d7ad_ac974985","updated":"2026-10-05 09:34:31.000000000","message":"lol I honestly forgot about that I added that fixture. :)","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d273d1ad789c91c7bdd37157d09f235da6e5d718","unresolved":true,"context_lines":[{"line_number":1361,"context_line":""},{"line_number":1362,"context_line":"    def _reset_all_executors(self, executors):"},{"line_number":1363,"context_line":"        threading_executors \u003d ["},{"line_number":1364,"context_line":"            e for e in executors if isinstance(e, futurist.ThreadPoolExecutor)"},{"line_number":1365,"context_line":"        ]"},{"line_number":1366,"context_line":""},{"line_number":1367,"context_line":"        def _get_executors_with_pending_tasks():"}],"source_content_type":"text/x-python","patch_set":1,"id":"39f0d7ad_ac974985","line":1364,"in_reply_to":"bb25c73f_c1677471","updated":"2026-09-23 03:41:01.000000000","message":"executor from that wrapper is handled by the factory and it will be present in \u0027executors\u0027 in this method. For wrapper itself, we do have fixture handling that, anything else we need here ? DelayingExecutorWrapperCleanupFixture","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"75676749702fac80f22664902239365f19f1af55","unresolved":true,"context_lines":[{"line_number":1366,"context_line":""},{"line_number":1367,"context_line":"        def _get_executors_with_pending_tasks():"},{"line_number":1368,"context_line":"            return [e for e in threading_executors"},{"line_number":1369,"context_line":"                    if e._work_queue.unfinished_tasks"},{"line_number":1370,"context_line":"            ]"},{"line_number":1371,"context_line":""},{"line_number":1372,"context_line":"        deadline \u003d time.monotonic() + self.EXECUTORS_SHUTDOWN_TIMEOUT"}],"source_content_type":"text/x-python","patch_set":1,"id":"d779bad5_f73bdddc","line":1369,"updated":"2026-09-21 08:29:10.000000000","message":"1. I\u0027m OK to land this but lets start adding improvement to futurist to avoid depending on its internals.\n\n2. Does unfinished_tasks is an internal? I don\u0027t see it in https://docs.python.org/3/library/queue.html But probably qsize() is a public proxy for that. If so I prefer the public interface.\n// later \nIt is not a proxy for it\n```\n\u003e\u003e\u003e e \u003d futurist.ThreadPoolExecutor(max_workers\u003d2)\n\u003e\u003e\u003e import time\n\u003e\u003e\u003e def sleep():\n...     time.sleep(10000)\n...     \n\u003e\u003e\u003e e._work_queue\n\u003cqueue.Queue object at 0x7fa4bcf67620\u003e\n\u003e\u003e\u003e e._work_queue.unfinished_tasks\n0\n\u003e\u003e\u003e e._work_queue.qsize()\n0\n\u003e\u003e\u003e e.submit(sleep)\n\u003cFuture at 0x7fa4bcf67b60 state\u003dpending\u003e\n\u003e\u003e\u003e e._work_queue.qsize()\n0\n\u003e\u003e\u003e e._work_queue.unfinished_tasks\n1\n\u003e\u003e\u003e \n```\nI\u0027m sad that python does not have a timeout param on the Queue.join() function or something equivalent.","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d273d1ad789c91c7bdd37157d09f235da6e5d718","unresolved":false,"context_lines":[{"line_number":1366,"context_line":""},{"line_number":1367,"context_line":"        def _get_executors_with_pending_tasks():"},{"line_number":1368,"context_line":"            return [e for e in threading_executors"},{"line_number":1369,"context_line":"                    if e._work_queue.unfinished_tasks"},{"line_number":1370,"context_line":"            ]"},{"line_number":1371,"context_line":""},{"line_number":1372,"context_line":"        deadline \u003d time.monotonic() + self.EXECUTORS_SHUTDOWN_TIMEOUT"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a33d359_d34f6419","line":1369,"in_reply_to":"d779bad5_f73bdddc","updated":"2026-09-23 03:41:01.000000000","message":"yeah, I was trying to find out some public interface to know queue state/pending tasks but seems it is not. I know this is not best but at least i can try to add one in futurist for _work_queue but that will again depends on queue.unfinished_tasks. I will add a todo here to do that.\n\nadded a todo","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"75676749702fac80f22664902239365f19f1af55","unresolved":true,"context_lines":[{"line_number":1374,"context_line":"            active_executors \u003d _get_executors_with_pending_tasks()"},{"line_number":1375,"context_line":"            if not active_executors or time.monotonic() \u003e\u003d deadline:"},{"line_number":1376,"context_line":"                break"},{"line_number":1377,"context_line":"            time.sleep(0.05)"},{"line_number":1378,"context_line":""},{"line_number":1379,"context_line":"        # If executors still has tasks to finish after timeout"},{"line_number":1380,"context_line":"        # then clean executors manually and fail the tests."}],"source_content_type":"text/x-python","patch_set":1,"id":"c99797b7_702669dd","line":1377,"updated":"2026-09-21 08:29:10.000000000","message":"this is a fairly busy loop. I\u0027m wondering how much we loop here in otherwise passing test. \n\nIf we need to depend on internals then we can also use `active_executor[0]._work_queue.all_tasks_done.wait(self.EXECUTORS_SHUTDOWN_TIMEOUT)` instead of the sleep. If that returns True then we need to loop, if it returns false we know we hit the timeout.","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2a41bd683a48da94dd8edbc181ccb1c21a7ba806","unresolved":true,"context_lines":[{"line_number":1374,"context_line":"            active_executors \u003d _get_executors_with_pending_tasks()"},{"line_number":1375,"context_line":"            if not active_executors or time.monotonic() \u003e\u003d deadline:"},{"line_number":1376,"context_line":"                break"},{"line_number":1377,"context_line":"            time.sleep(0.05)"},{"line_number":1378,"context_line":""},{"line_number":1379,"context_line":"        # If executors still has tasks to finish after timeout"},{"line_number":1380,"context_line":"        # then clean executors manually and fail the tests."}],"source_content_type":"text/x-python","patch_set":1,"id":"7d8600ef_e1ab1840","line":1377,"in_reply_to":"25fffa5a_27c4b044","updated":"2026-10-05 09:34:31.000000000","message":"\u003e sure, we anyways replying on \u0027unfinished_tasks\u0027, wait with timeout is better.\n\u003e \n\u003e //later\n\u003e \n\u003e I tried and found that all_tasks_done.wait is sync call and will be blocked until all tasks done or timeout. here, we need to wait for all the executors\u0027s queue task and all_tasks_done.wait will make them waiting serially.\n\u003e \n\u003e I thought of adding remaining_time logic where we can wait for longest executors but that will make it more complicated.\n\nOK, fine. \n\n\u003e \n\u003e Answer to your question about passing tests, for passing tests it will return immiduatly as there will not be any \u0027active_executors\u0027\n\nI guess I meant the test that schedule some cleanup to the executors, i.e. what is mentioned by your comment at the top and why we have the 10 sec gracefull period here.","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"7f1d95bd9acc1036ec3ec5fec0d7ef43a6be35cd","unresolved":true,"context_lines":[{"line_number":1374,"context_line":"            active_executors \u003d _get_executors_with_pending_tasks()"},{"line_number":1375,"context_line":"            if not active_executors or time.monotonic() \u003e\u003d deadline:"},{"line_number":1376,"context_line":"                break"},{"line_number":1377,"context_line":"            time.sleep(0.05)"},{"line_number":1378,"context_line":""},{"line_number":1379,"context_line":"        # If executors still has tasks to finish after timeout"},{"line_number":1380,"context_line":"        # then clean executors manually and fail the tests."}],"source_content_type":"text/x-python","patch_set":1,"id":"906c5c50_8c69201f","line":1377,"in_reply_to":"7d8600ef_e1ab1840","updated":"2026-10-06 20:51:27.000000000","message":"ok, yeah 10 sec should be enough for test to finish the cleanup things.","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d273d1ad789c91c7bdd37157d09f235da6e5d718","unresolved":true,"context_lines":[{"line_number":1374,"context_line":"            active_executors \u003d _get_executors_with_pending_tasks()"},{"line_number":1375,"context_line":"            if not active_executors or time.monotonic() \u003e\u003d deadline:"},{"line_number":1376,"context_line":"                break"},{"line_number":1377,"context_line":"            time.sleep(0.05)"},{"line_number":1378,"context_line":""},{"line_number":1379,"context_line":"        # If executors still has tasks to finish after timeout"},{"line_number":1380,"context_line":"        # then clean executors manually and fail the tests."}],"source_content_type":"text/x-python","patch_set":1,"id":"25fffa5a_27c4b044","line":1377,"in_reply_to":"c99797b7_702669dd","updated":"2026-09-23 03:41:01.000000000","message":"sure, we anyways replying on \u0027unfinished_tasks\u0027, wait with timeout is better.\n\n//later\n\nI tried and found that all_tasks_done.wait is sync call and will be blocked until all tasks done or timeout. here, we need to wait for all the executors\u0027s queue task and all_tasks_done.wait will make them waiting serially.\n\nI thought of adding remaining_time logic where we can wait for longest executors but that will make it more complicated.\n\nAnswer to your question about passing tests, for passing tests it will return immiduatly as there will not be any \u0027active_executors\u0027","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"75676749702fac80f22664902239365f19f1af55","unresolved":true,"context_lines":[{"line_number":1379,"context_line":"        # If executors still has tasks to finish after timeout"},{"line_number":1380,"context_line":"        # then clean executors manually and fail the tests."},{"line_number":1381,"context_line":"        if active_executors:"},{"line_number":1382,"context_line":"            thread_pool_factory.FACTORY._all_executors.clear()"},{"line_number":1383,"context_line":"            for executor in active_executors:"},{"line_number":1384,"context_line":"                for worker in executor._workers:"},{"line_number":1385,"context_line":"                    # A hanged executor will still hang the test process and"}],"source_content_type":"text/x-python","patch_set":1,"id":"b00f0269_083867e9","line":1382,"updated":"2026-09-21 08:29:10.000000000","message":"I guess we can still want to stop the non active_executors in this branch too before we drop the active ones","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"96c5e48ed8af5ebd952f51d916786b8ea1528464","unresolved":false,"context_lines":[{"line_number":1379,"context_line":"        # If executors still has tasks to finish after timeout"},{"line_number":1380,"context_line":"        # then clean executors manually and fail the tests."},{"line_number":1381,"context_line":"        if active_executors:"},{"line_number":1382,"context_line":"            thread_pool_factory.FACTORY._all_executors.clear()"},{"line_number":1383,"context_line":"            for executor in active_executors:"},{"line_number":1384,"context_line":"                for worker in executor._workers:"},{"line_number":1385,"context_line":"                    # A hanged executor will still hang the test process and"}],"source_content_type":"text/x-python","patch_set":1,"id":"43d28c3d_2b676a26","line":1382,"in_reply_to":"a86a1f52_f288cd47","updated":"2026-09-23 04:14:54.000000000","message":"I mean yes, we still need cleanup in factory so that new tests get fresh executors.","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d273d1ad789c91c7bdd37157d09f235da6e5d718","unresolved":true,"context_lines":[{"line_number":1379,"context_line":"        # If executors still has tasks to finish after timeout"},{"line_number":1380,"context_line":"        # then clean executors manually and fail the tests."},{"line_number":1381,"context_line":"        if active_executors:"},{"line_number":1382,"context_line":"            thread_pool_factory.FACTORY._all_executors.clear()"},{"line_number":1383,"context_line":"            for executor in active_executors:"},{"line_number":1384,"context_line":"                for worker in executor._workers:"},{"line_number":1385,"context_line":"                    # A hanged executor will still hang the test process and"}],"source_content_type":"text/x-python","patch_set":1,"id":"a86a1f52_f288cd47","line":1382,"in_reply_to":"b00f0269_083867e9","updated":"2026-09-23 03:41:01.000000000","message":"true, done","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"75676749702fac80f22664902239365f19f1af55","unresolved":true,"context_lines":[{"line_number":1384,"context_line":"                for worker in executor._workers:"},{"line_number":1385,"context_line":"                    # A hanged executor will still hang the test process and"},{"line_number":1386,"context_line":"                    # timeout the job so we need to cleanup the worker."},{"line_number":1387,"context_line":"                    futurist._thread._to_be_cleaned.pop(worker, None)"},{"line_number":1388,"context_line":"            raise RuntimeError("},{"line_number":1389,"context_line":"                f\u0027{self.test_case_id}: the executors still have tasks that \u0027"},{"line_number":1390,"context_line":"                f\u0027did not finish in {self.EXECUTORS_SHUTDOWN_TIMEOUT}s\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"8acf815a_cb349655","line":1387,"updated":"2026-09-21 08:29:10.000000000","message":"1. one more reason we should improve on futurist, this is an open heart surgery in a dirty shed right now ;)\n\n2. btw is this enough? I see another way of a potential hang at https://github.com/openstack/futurist/blob/de8158746c66314b49928fb3d1980438d5466f50/futurist/_thread.py#L70 as it calls stop when the executor is finalized. \n\n3. can we infer anything about the task being executed in that worker to help identifying what task hangs?\nI think we can log a stack trace of the hanging task via\n```\nframe \u003d sys._current_frames().get(worker.ident)\n```","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d273d1ad789c91c7bdd37157d09f235da6e5d718","unresolved":true,"context_lines":[{"line_number":1384,"context_line":"                for worker in executor._workers:"},{"line_number":1385,"context_line":"                    # A hanged executor will still hang the test process and"},{"line_number":1386,"context_line":"                    # timeout the job so we need to cleanup the worker."},{"line_number":1387,"context_line":"                    futurist._thread._to_be_cleaned.pop(worker, None)"},{"line_number":1388,"context_line":"            raise RuntimeError("},{"line_number":1389,"context_line":"                f\u0027{self.test_case_id}: the executors still have tasks that \u0027"},{"line_number":1390,"context_line":"                f\u0027did not finish in {self.EXECUTORS_SHUTDOWN_TIMEOUT}s\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"92ba6590_496da1b5","line":1387,"in_reply_to":"8acf815a_cb349655","updated":"2026-09-23 03:41:01.000000000","message":"\u003e 1. one more reason we should improve on futurist, this is an open heart surgery in a dirty shed right now ;)\n\nyeah, adding a todo for this too.\n\n\u003e \n\u003e 2. btw is this enough? I see another way of a potential hang at https://github.com/openstack/futurist/blob/de8158746c66314b49928fb3d1980438d5466f50/futurist/_thread.py#L70 as it calls stop when the executor is finalized. \n\nthis is not blocking and self.stop() just set the should_stop\u003dtrue which is used to stop accepting new work.\n\n\u003e \n\u003e 3. can we infer anything about the task being executed in that worker to help identifying what task hangs?\n\u003e I think we can log a stack trace of the hanging task via\n\u003e ```\n\u003e frame \u003d sys._current_frames().get(worker.ident)\n\u003e ```\n\nsure, done","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"eb7c17eed93def6a51d681d2812248edf5dad616","unresolved":false,"context_lines":[{"line_number":1384,"context_line":"                for worker in executor._workers:"},{"line_number":1385,"context_line":"                    # A hanged executor will still hang the test process and"},{"line_number":1386,"context_line":"                    # timeout the job so we need to cleanup the worker."},{"line_number":1387,"context_line":"                    futurist._thread._to_be_cleaned.pop(worker, None)"},{"line_number":1388,"context_line":"            raise RuntimeError("},{"line_number":1389,"context_line":"                f\u0027{self.test_case_id}: the executors still have tasks that \u0027"},{"line_number":1390,"context_line":"                f\u0027did not finish in {self.EXECUTORS_SHUTDOWN_TIMEOUT}s\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5cefedaf_d0b599e5","line":1387,"in_reply_to":"92ba6590_496da1b5","updated":"2026-10-01 17:49:54.000000000","message":"Done","commit_id":"1d35e124793acee40903da9730277e3cf1c52bfd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f0870cc8f5bd260043e3262311b809091067f72c","unresolved":true,"context_lines":[{"line_number":1354,"context_line":""},{"line_number":1355,"context_line":"    def _submit(self, executor, submit, fn, *args, **kwargs):"},{"line_number":1356,"context_line":"        # NOTE(gmaan): Once an executor is shutdown and new task is submitted"},{"line_number":1357,"context_line":"        # then error is stored in future object and does not come to the test."},{"line_number":1358,"context_line":"        # In that case, test still pass silently. To capture such test, here"},{"line_number":1359,"context_line":"        # we catch the error and store the tasks submitted after sbutdown so"},{"line_number":1360,"context_line":"        # that later we can fail the test with proper leaked tasks."}],"source_content_type":"text/x-python","patch_set":4,"id":"a28de559_a9c62ee9","line":1357,"updated":"2026-09-24 13:23:57.000000000","message":"really? I think it is a direct raise on the submit() call not an exception set on the returned future.\n\n```\n\u003e\u003e\u003e e \u003d futurist.ThreadPoolExecutor(max_workers\u003d1)\n\u003e\u003e\u003e e.shutdown(wait\u003dTrue)\n\u003e\u003e\u003e def task():\n...     pass\n...     \n\u003e\u003e\u003e e.submit(task)\nTraceback (most recent call last):\n  File \"\u003cpython-input-4\u003e\", line 1, in \u003cmodule\u003e\n    e.submit(task)\n    ~~~~~~~~^^^^^^\n  File \"/home/gibi/upstream/git/openstack/nova/.tox/functional-py313-threading/lib/python3.13/site-packages/futurist/_futures.py\", line 262, in submit\n    raise RuntimeError(\n        \u0027Can not schedule new futures after being shutdown\u0027\n    )\nRuntimeError: Can not schedule new futures after being shutdown\n\u003e\u003e\u003e \n```","commit_id":"d908e48e43ac730a192d28d97ba0cbbe63c6457e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"eb7c17eed93def6a51d681d2812248edf5dad616","unresolved":false,"context_lines":[{"line_number":1354,"context_line":""},{"line_number":1355,"context_line":"    def _submit(self, executor, submit, fn, *args, **kwargs):"},{"line_number":1356,"context_line":"        # NOTE(gmaan): Once an executor is shutdown and new task is submitted"},{"line_number":1357,"context_line":"        # then error is stored in future object and does not come to the test."},{"line_number":1358,"context_line":"        # In that case, test still pass silently. To capture such test, here"},{"line_number":1359,"context_line":"        # we catch the error and store the tasks submitted after sbutdown so"},{"line_number":1360,"context_line":"        # that later we can fail the test with proper leaked tasks."}],"source_content_type":"text/x-python","patch_set":4,"id":"a42833d0_3679f083","line":1357,"in_reply_to":"6803ac50_27781f19","updated":"2026-10-01 17:49:54.000000000","message":"Done","commit_id":"d908e48e43ac730a192d28d97ba0cbbe63c6457e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d77d15816a62ca9d415e351c887973bbfdfa0b16","unresolved":true,"context_lines":[{"line_number":1354,"context_line":""},{"line_number":1355,"context_line":"    def _submit(self, executor, submit, fn, *args, **kwargs):"},{"line_number":1356,"context_line":"        # NOTE(gmaan): Once an executor is shutdown and new task is submitted"},{"line_number":1357,"context_line":"        # then error is stored in future object and does not come to the test."},{"line_number":1358,"context_line":"        # In that case, test still pass silently. To capture such test, here"},{"line_number":1359,"context_line":"        # we catch the error and store the tasks submitted after sbutdown so"},{"line_number":1360,"context_line":"        # that later we can fail the test with proper leaked tasks."}],"source_content_type":"text/x-python","patch_set":4,"id":"6803ac50_27781f19","line":1357,"in_reply_to":"a28de559_a9c62ee9","updated":"2026-09-30 00:53:31.000000000","message":"For normal case yes, it returns the error to that thread/process; but if a task running in the executor submits another task, task2\u0027s error doesn\u0027t propagate to the main process/thread and is stored in the future result. I am not sure we have case of tasks submitting other task in executor but this is one case i thought of that can cause issue.\n\nBut let me figure out the exact root cause and this extra safty might not be needed.","commit_id":"d908e48e43ac730a192d28d97ba0cbbe63c6457e"},{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"146f2b8db472192f399d424bfb012f2555cb1c95","unresolved":true,"context_lines":[{"line_number":1356,"context_line":"        # NOTE(gmaan): Once an executor is shutdown and new task is submitted"},{"line_number":1357,"context_line":"        # then error is stored in future object and does not come to the test."},{"line_number":1358,"context_line":"        # In that case, test still pass silently. To capture such test, here"},{"line_number":1359,"context_line":"        # we catch the error and store the tasks submitted after sbutdown so"},{"line_number":1360,"context_line":"        # that later we can fail the test with proper leaked tasks."},{"line_number":1361,"context_line":"        try:"},{"line_number":1362,"context_line":"            return submit(fn, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a67b2df_cea720e5","line":1359,"updated":"2026-09-24 13:08:30.000000000","message":"nit: sbutdown-\u003eshutdown","commit_id":"d908e48e43ac730a192d28d97ba0cbbe63c6457e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d77d15816a62ca9d415e351c887973bbfdfa0b16","unresolved":false,"context_lines":[{"line_number":1356,"context_line":"        # NOTE(gmaan): Once an executor is shutdown and new task is submitted"},{"line_number":1357,"context_line":"        # then error is stored in future object and does not come to the test."},{"line_number":1358,"context_line":"        # In that case, test still pass silently. To capture such test, here"},{"line_number":1359,"context_line":"        # we catch the error and store the tasks submitted after sbutdown so"},{"line_number":1360,"context_line":"        # that later we can fail the test with proper leaked tasks."},{"line_number":1361,"context_line":"        try:"},{"line_number":1362,"context_line":"            return submit(fn, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":4,"id":"adc340d6_27d6e762","line":1359,"in_reply_to":"5a67b2df_cea720e5","updated":"2026-09-30 00:53:31.000000000","message":"Done","commit_id":"d908e48e43ac730a192d28d97ba0cbbe63c6457e"},{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"146f2b8db472192f399d424bfb012f2555cb1c95","unresolved":true,"context_lines":[{"line_number":1434,"context_line":"                    # TODO(gmaan): need to avoid accessing internal variable"},{"line_number":1435,"context_line":"                    # from futurist, instead futurist should provide some"},{"line_number":1436,"context_line":"                    # public interface."},{"line_number":1437,"context_line":"                    futurist._thread._to_be_cleaned.pop(worker, None)"},{"line_number":1438,"context_line":"            raise RuntimeError("},{"line_number":1439,"context_line":"                f\u0027{self.test_case_id}: the executors still have tasks that \u0027"},{"line_number":1440,"context_line":"                f\u0027did not finish in {self.EXECUTORS_SHUTDOWN_TIMEOUT}s\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"045b4d0a_9e97dac2","line":1437,"updated":"2026-09-24 13:08:30.000000000","message":"_to_be_cleaned is currently a weakref.WeakKeyDictionary[ThreadWorker, bool]\nso .pop(worker, None) is correct. But futurist\u0027s own source\nhas TODO(harlowja) to switch to a WeakRefSet (https://github.com/openstack/futurist/blob/de8158746c66314b49928fb3d1980438d5466f50/futurist/_thread.py#L82) .pop(key, default)\nbreaks (WeakSet.pop() takes no arguments; you\u0027d need .discard(worker)).\n\nCould we track this with a launchpad bug linked in the TODO? That way we\nremember to push for a public API in futurist before the internal structure\nchanges under us?","commit_id":"d908e48e43ac730a192d28d97ba0cbbe63c6457e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d77d15816a62ca9d415e351c887973bbfdfa0b16","unresolved":false,"context_lines":[{"line_number":1434,"context_line":"                    # TODO(gmaan): need to avoid accessing internal variable"},{"line_number":1435,"context_line":"                    # from futurist, instead futurist should provide some"},{"line_number":1436,"context_line":"                    # public interface."},{"line_number":1437,"context_line":"                    futurist._thread._to_be_cleaned.pop(worker, None)"},{"line_number":1438,"context_line":"            raise RuntimeError("},{"line_number":1439,"context_line":"                f\u0027{self.test_case_id}: the executors still have tasks that \u0027"},{"line_number":1440,"context_line":"                f\u0027did not finish in {self.EXECUTORS_SHUTDOWN_TIMEOUT}s\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fabd5fa8_e0049f5b","line":1437,"in_reply_to":"045b4d0a_9e97dac2","updated":"2026-09-30 00:53:31.000000000","message":"sure, good idea to track in LP. done https://bugs.launchpad.net/futurist/+bug/2168924","commit_id":"d908e48e43ac730a192d28d97ba0cbbe63c6457e"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"0c2e97359c0de97329cc977a76b69cc519e86daa","unresolved":false,"context_lines":[{"line_number":1323,"context_line":"                 \u0027pending tasks of executors %s\u0027,"},{"line_number":1324,"context_line":"                 self.test_case_id, self.EXECUTORS_SHUTDOWN_TIMEOUT,"},{"line_number":1325,"context_line":"                 [e.name for e in threading_executors])"},{"line_number":1326,"context_line":"        start \u003d time.monotonic()"},{"line_number":1327,"context_line":"        deadline \u003d start + self.EXECUTORS_SHUTDOWN_TIMEOUT"},{"line_number":1328,"context_line":"        while True:"},{"line_number":1329,"context_line":"            active_executors \u003d _get_executors_with_pending_tasks()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ea5d18b8_c887691a","line":1326,"updated":"2026-09-30 01:52:40.000000000","message":"pep8: F821 undefined name \u0027time\u0027","commit_id":"cd474b374ac31180b754ed41db7da002888b9dfc"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"0c2e97359c0de97329cc977a76b69cc519e86daa","unresolved":false,"context_lines":[{"line_number":1327,"context_line":"        deadline \u003d start + self.EXECUTORS_SHUTDOWN_TIMEOUT"},{"line_number":1328,"context_line":"        while True:"},{"line_number":1329,"context_line":"            active_executors \u003d _get_executors_with_pending_tasks()"},{"line_number":1330,"context_line":"            if not active_executors or time.monotonic() \u003e\u003d deadline:"},{"line_number":1331,"context_line":"                break"},{"line_number":1332,"context_line":"            time.sleep(0.05)"},{"line_number":1333,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7e6a3184_c51e6d16","line":1330,"updated":"2026-09-30 01:52:40.000000000","message":"pep8: F821 undefined name \u0027time\u0027","commit_id":"cd474b374ac31180b754ed41db7da002888b9dfc"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"0c2e97359c0de97329cc977a76b69cc519e86daa","unresolved":false,"context_lines":[{"line_number":1329,"context_line":"            active_executors \u003d _get_executors_with_pending_tasks()"},{"line_number":1330,"context_line":"            if not active_executors or time.monotonic() \u003e\u003d deadline:"},{"line_number":1331,"context_line":"                break"},{"line_number":1332,"context_line":"            time.sleep(0.05)"},{"line_number":1333,"context_line":""},{"line_number":1334,"context_line":"        # If executors still has tasks to finish after timeout"},{"line_number":1335,"context_line":"        # then clean executors manually and fail the tests."}],"source_content_type":"text/x-python","patch_set":5,"id":"ff5e9770_2d3c653a","line":1332,"updated":"2026-09-30 01:52:40.000000000","message":"pep8: F821 undefined name \u0027time\u0027","commit_id":"cd474b374ac31180b754ed41db7da002888b9dfc"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"0c2e97359c0de97329cc977a76b69cc519e86daa","unresolved":false,"context_lines":[{"line_number":1336,"context_line":"        if active_executors:"},{"line_number":1337,"context_line":"            LOG.warning(\u0027%s: executors cleanup timed out after %.2fs, \u0027"},{"line_number":1338,"context_line":"                        \u0027executors %s still have pending tasks\u0027,"},{"line_number":1339,"context_line":"                        self.test_case_id, time.monotonic() - start,"},{"line_number":1340,"context_line":"                        [e.name for e in active_executors])"},{"line_number":1341,"context_line":"            thread_pool_factory.FACTORY._all_executors.clear()"},{"line_number":1342,"context_line":"            for executor in active_executors:"}],"source_content_type":"text/x-python","patch_set":5,"id":"f2f58a34_d74f4f50","line":1339,"updated":"2026-09-30 01:52:40.000000000","message":"pep8: F821 undefined name \u0027time\u0027","commit_id":"cd474b374ac31180b754ed41db7da002888b9dfc"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"0c2e97359c0de97329cc977a76b69cc519e86daa","unresolved":false,"context_lines":[{"line_number":1364,"context_line":"        else:"},{"line_number":1365,"context_line":"            thread_pool_factory.reset_all_executors()"},{"line_number":1366,"context_line":"            LOG.info(\u0027%s: executors cleanup finished in %.2fs\u0027,"},{"line_number":1367,"context_line":"                     self.test_case_id, time.monotonic() - start)"},{"line_number":1368,"context_line":""},{"line_number":1369,"context_line":"    def _cleanup_executors(self):"},{"line_number":1370,"context_line":"        executors \u003d list(thread_pool_factory.FACTORY._all_executors.values())"}],"source_content_type":"text/x-python","patch_set":5,"id":"59b0d358_04c104c4","line":1367,"updated":"2026-09-30 01:52:40.000000000","message":"pep8: F821 undefined name \u0027time\u0027","commit_id":"cd474b374ac31180b754ed41db7da002888b9dfc"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2a41bd683a48da94dd8edbc181ccb1c21a7ba806","unresolved":true,"context_lines":[{"line_number":1271,"context_line":""},{"line_number":1272,"context_line":"        self.addCleanup(self._cleanup_executors)"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"    # This fixture, shutdown all the executors test created with all"},{"line_number":1275,"context_line":"    # in-progress to finish so that new executors are available for the"},{"line_number":1276,"context_line":"    # next test. We want to catch test who are leaving the in-progress"},{"line_number":1277,"context_line":"    # tasks at this cleanup stage, if so then this timeout will help to"},{"line_number":1278,"context_line":"    # fail such tests instead of hanging in executors to finish all the"}],"source_content_type":"text/x-python","patch_set":7,"id":"f85ede9a_724d9004","line":1275,"range":{"start_line":1274,"start_character":6,"end_line":1275,"end_character":27},"updated":"2026-10-05 09:34:31.000000000","message":"this feels like a broken sentence","commit_id":"4a6574fea1111d1e066a9350440895bc3d4cd041"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"7f1d95bd9acc1036ec3ec5fec0d7ef43a6be35cd","unresolved":false,"context_lines":[{"line_number":1271,"context_line":""},{"line_number":1272,"context_line":"        self.addCleanup(self._cleanup_executors)"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"    # This fixture, shutdown all the executors test created with all"},{"line_number":1275,"context_line":"    # in-progress to finish so that new executors are available for the"},{"line_number":1276,"context_line":"    # next test. We want to catch test who are leaving the in-progress"},{"line_number":1277,"context_line":"    # tasks at this cleanup stage, if so then this timeout will help to"},{"line_number":1278,"context_line":"    # fail such tests instead of hanging in executors to finish all the"}],"source_content_type":"text/x-python","patch_set":7,"id":"7de90cfd_5e5f3e9b","line":1275,"range":{"start_line":1274,"start_character":6,"end_line":1275,"end_character":27},"in_reply_to":"f85ede9a_724d9004","updated":"2026-10-06 20:51:27.000000000","message":"Done","commit_id":"4a6574fea1111d1e066a9350440895bc3d4cd041"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2a41bd683a48da94dd8edbc181ccb1c21a7ba806","unresolved":true,"context_lines":[{"line_number":1320,"context_line":"                        self.test_case_id, time.monotonic() - start,"},{"line_number":1321,"context_line":"                        [e.name for e in active_executors])"},{"line_number":1322,"context_line":"            thread_pool_factory.FACTORY._all_executors.clear()"},{"line_number":1323,"context_line":"            for executor in active_executors:"},{"line_number":1324,"context_line":"                for worker in executor._workers:"},{"line_number":1325,"context_line":"                    if worker.is_alive():"},{"line_number":1326,"context_line":"                        stack \u003d \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"4aaabf28_b15268b6","line":1323,"updated":"2026-10-05 09:34:31.000000000","message":"an executor can be active in two cases:\n1. there is a task still running in a worker. That is handled below.\n2. there is no task running in any workers, but there are tasks queued up to the executor that are not cancelled (this is why you needed to modify the unit test case to cancel the future of the second, not started but queued, task).\n\nI would extend the LOG.warning above at L1318 to let the dev know about the two potential cases that causing his or her test to fail. For the case1 we will log a stack trace helping the dev, but the case #2 is also failing the active executor test but we do not leave a hint about what caused the failure.\n\nLets at least say in the warning something along the line: \"executors %s still have tasks running or tasks waiting for a free worker thread\"\n\nIf we can list the function name of the tasks that are waiting for worker that would be an extra good thing her. (The running tasks will have a stack trace below so that is OK)","commit_id":"4a6574fea1111d1e066a9350440895bc3d4cd041"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"7f1d95bd9acc1036ec3ec5fec0d7ef43a6be35cd","unresolved":false,"context_lines":[{"line_number":1320,"context_line":"                        self.test_case_id, time.monotonic() - start,"},{"line_number":1321,"context_line":"                        [e.name for e in active_executors])"},{"line_number":1322,"context_line":"            thread_pool_factory.FACTORY._all_executors.clear()"},{"line_number":1323,"context_line":"            for executor in active_executors:"},{"line_number":1324,"context_line":"                for worker in executor._workers:"},{"line_number":1325,"context_line":"                    if worker.is_alive():"},{"line_number":1326,"context_line":"                        stack \u003d \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"6f691fbf_760ec52c","line":1323,"in_reply_to":"4aaabf28_b15268b6","updated":"2026-10-06 20:51:27.000000000","message":"I did not think about 2nd case which is good to log. done","commit_id":"4a6574fea1111d1e066a9350440895bc3d4cd041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4d0e0e9bfb75e55b5465bf6f246941a028440601","unresolved":true,"context_lines":[{"line_number":1313,"context_line":""},{"line_number":1314,"context_line":"        # If executors still has tasks to finish after timeout"},{"line_number":1315,"context_line":"        # then clean executors manually and fail the tests."},{"line_number":1316,"context_line":"        if active_executors:"},{"line_number":1317,"context_line":"            LOG.warning(\u0027%s: executors cleanup timed out after %.2fs, \u0027"},{"line_number":1318,"context_line":"                        \u0027executors %s still have tasks running or tasks \u0027"},{"line_number":1319,"context_line":"                        \u0027waiting for a free worker thread\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"029818ae_d5cb667f","line":1316,"updated":"2026-10-07 18:51:40.000000000","message":"Seems like we could remove this layer of indenting if we just allow the for loop to not iterate over an empty list. The log below can be covered as the first step of the (first) for loop below.","commit_id":"7f9dbeb5e408489daaf7205a58c09ee9d82d85bb"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"854212e4fcc740746991031c4d940d6d660edd62","unresolved":true,"context_lines":[{"line_number":1313,"context_line":""},{"line_number":1314,"context_line":"        # If executors still has tasks to finish after timeout"},{"line_number":1315,"context_line":"        # then clean executors manually and fail the tests."},{"line_number":1316,"context_line":"        if active_executors:"},{"line_number":1317,"context_line":"            LOG.warning(\u0027%s: executors cleanup timed out after %.2fs, \u0027"},{"line_number":1318,"context_line":"                        \u0027executors %s still have tasks running or tasks \u0027"},{"line_number":1319,"context_line":"                        \u0027waiting for a free worker thread\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9b9be607_04f2010a","line":1316,"in_reply_to":"029818ae_d5cb667f","updated":"2026-10-08 02:09:24.000000000","message":"yeah, i could have refactor it to avoid this extra indentation. will do if i need to respin","commit_id":"7f9dbeb5e408489daaf7205a58c09ee9d82d85bb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4d0e0e9bfb75e55b5465bf6f246941a028440601","unresolved":true,"context_lines":[{"line_number":1349,"context_line":"                    # from futurist, instead futurist should provide some"},{"line_number":1350,"context_line":"                    # public interface."},{"line_number":1351,"context_line":"                    # https://bugs.launchpad.net/futurist/+bug/2168924"},{"line_number":1352,"context_line":"                    futurist._thread._to_be_cleaned.pop(worker, None)"},{"line_number":1353,"context_line":"            raise RuntimeError("},{"line_number":1354,"context_line":"                f\u0027{self.test_case_id}: the executors still have tasks that \u0027"},{"line_number":1355,"context_line":"                f\u0027did not finish in {self.EXECUTORS_SHUTDOWN_TIMEOUT}s\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"46a8dca0_ef462543","line":1352,"updated":"2026-10-07 18:51:40.000000000","message":"This is because they\u0027re not setDaemon right? Seems like we could also make them daemon threads only in the test framework to get around this problem. I don\u0027t think it would be a substantial difference from runtime to matter.","commit_id":"7f9dbeb5e408489daaf7205a58c09ee9d82d85bb"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"854212e4fcc740746991031c4d940d6d660edd62","unresolved":true,"context_lines":[{"line_number":1349,"context_line":"                    # from futurist, instead futurist should provide some"},{"line_number":1350,"context_line":"                    # public interface."},{"line_number":1351,"context_line":"                    # https://bugs.launchpad.net/futurist/+bug/2168924"},{"line_number":1352,"context_line":"                    futurist._thread._to_be_cleaned.pop(worker, None)"},{"line_number":1353,"context_line":"            raise RuntimeError("},{"line_number":1354,"context_line":"                f\u0027{self.test_case_id}: the executors still have tasks that \u0027"},{"line_number":1355,"context_line":"                f\u0027did not finish in {self.EXECUTORS_SHUTDOWN_TIMEOUT}s\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"56ef15b3_eea1aa7e","line":1352,"in_reply_to":"46a8dca0_ef462543","updated":"2026-10-08 02:09:24.000000000","message":"They are daemon threads, but we don\u0027t exit the process during each test right. so they stay alive/running throughout the full tests run which is happening during the single-process lifecycle.","commit_id":"7f9dbeb5e408489daaf7205a58c09ee9d82d85bb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7a16816606d4fedece523eef6a7a6b1ab2a87f68","unresolved":true,"context_lines":[{"line_number":1349,"context_line":"                    # from futurist, instead futurist should provide some"},{"line_number":1350,"context_line":"                    # public interface."},{"line_number":1351,"context_line":"                    # https://bugs.launchpad.net/futurist/+bug/2168924"},{"line_number":1352,"context_line":"                    futurist._thread._to_be_cleaned.pop(worker, None)"},{"line_number":1353,"context_line":"            raise RuntimeError("},{"line_number":1354,"context_line":"                f\u0027{self.test_case_id}: the executors still have tasks that \u0027"},{"line_number":1355,"context_line":"                f\u0027did not finish in {self.EXECUTORS_SHUTDOWN_TIMEOUT}s\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"1aeca4fe_3087f6ff","line":1352,"in_reply_to":"56ef15b3_eea1aa7e","updated":"2026-10-08 18:20:42.000000000","message":"Hah, of course, duh!","commit_id":"7f9dbeb5e408489daaf7205a58c09ee9d82d85bb"}],"nova/tests/unit/test_thread_pool_factory.py":[{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"146f2b8db472192f399d424bfb012f2555cb1c95","unresolved":true,"context_lines":[{"line_number":587,"context_line":"        # Cancel the queued tasks and shutdown the executors to cleanup"},{"line_number":588,"context_line":"        # the queue."},{"line_number":589,"context_line":"        for future in queued:"},{"line_number":590,"context_line":"            self.assertTrue(future.cancel())"},{"line_number":591,"context_line":"        work.set()"},{"line_number":592,"context_line":"        executor.shutdown(wait\u003dTrue)"},{"line_number":593,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"dd35756e_6b444bad","line":590,"updated":"2026-09-24 13:08:30.000000000","message":"maybe I\u0027m overthinking this or missing something, but future.cancel() returns False if the task has already started executing. Using assertTrue would then fail the test due to a teardown race, not a fullness-check regression. In this test with a single-thread executor, the second task should always be cancellable, but IMO it\u0027s safer to just call future.cancel() without the assertion","commit_id":"d908e48e43ac730a192d28d97ba0cbbe63c6457e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"eb7c17eed93def6a51d681d2812248edf5dad616","unresolved":false,"context_lines":[{"line_number":587,"context_line":"        # Cancel the queued tasks and shutdown the executors to cleanup"},{"line_number":588,"context_line":"        # the queue."},{"line_number":589,"context_line":"        for future in queued:"},{"line_number":590,"context_line":"            self.assertTrue(future.cancel())"},{"line_number":591,"context_line":"        work.set()"},{"line_number":592,"context_line":"        executor.shutdown(wait\u003dTrue)"},{"line_number":593,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"a2c6cb34_e4254d21","line":590,"in_reply_to":"dd35756e_6b444bad","updated":"2026-10-01 17:49:54.000000000","message":"you are right, that possible can be racy to cancel the in-progress tasks.","commit_id":"d908e48e43ac730a192d28d97ba0cbbe63c6457e"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2a41bd683a48da94dd8edbc181ccb1c21a7ba806","unresolved":true,"context_lines":[{"line_number":587,"context_line":"        # Cancel the queued tasks and shutdown the executors to cleanup"},{"line_number":588,"context_line":"        # the queue."},{"line_number":589,"context_line":"        for future in queued:"},{"line_number":590,"context_line":"            future.cancel()"},{"line_number":591,"context_line":"        work.set()"},{"line_number":592,"context_line":"        executor.shutdown(wait\u003dTrue)"},{"line_number":593,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"f54a009b_3b4f59cf","line":590,"updated":"2026-10-05 09:34:31.000000000","message":"this should be done not just on the happy path. If the test fails after L575 then this cancel is needed at least on the second task as that potentially not getting a worker thread before the executor shutdown and causing the fixture to see an active executor. \nMove this to a cleanup function and add it to the list via self.addCleanup() after L575","commit_id":"4a6574fea1111d1e066a9350440895bc3d4cd041"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"7f1d95bd9acc1036ec3ec5fec0d7ef43a6be35cd","unresolved":false,"context_lines":[{"line_number":587,"context_line":"        # Cancel the queued tasks and shutdown the executors to cleanup"},{"line_number":588,"context_line":"        # the queue."},{"line_number":589,"context_line":"        for future in queued:"},{"line_number":590,"context_line":"            future.cancel()"},{"line_number":591,"context_line":"        work.set()"},{"line_number":592,"context_line":"        executor.shutdown(wait\u003dTrue)"},{"line_number":593,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fc741137_13d9fa1f","line":590,"in_reply_to":"f54a009b_3b4f59cf","updated":"2026-10-06 20:51:27.000000000","message":"Done","commit_id":"4a6574fea1111d1e066a9350440895bc3d4cd041"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2a41bd683a48da94dd8edbc181ccb1c21a7ba806","unresolved":true,"context_lines":[{"line_number":588,"context_line":"        # the queue."},{"line_number":589,"context_line":"        for future in queued:"},{"line_number":590,"context_line":"            future.cancel()"},{"line_number":591,"context_line":"        work.set()"},{"line_number":592,"context_line":"        executor.shutdown(wait\u003dTrue)"},{"line_number":593,"context_line":""},{"line_number":594,"context_line":"    def test_spawn_on_initializes_profiler(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a495286_31b1572f","line":591,"updated":"2026-10-05 09:34:31.000000000","message":"This is already done via L561 at cleanup and this specific cleanup (work.set) runs before the fixture cleanup as cleanups are executed LIFO.","commit_id":"4a6574fea1111d1e066a9350440895bc3d4cd041"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"7f1d95bd9acc1036ec3ec5fec0d7ef43a6be35cd","unresolved":false,"context_lines":[{"line_number":588,"context_line":"        # the queue."},{"line_number":589,"context_line":"        for future in queued:"},{"line_number":590,"context_line":"            future.cancel()"},{"line_number":591,"context_line":"        work.set()"},{"line_number":592,"context_line":"        executor.shutdown(wait\u003dTrue)"},{"line_number":593,"context_line":""},{"line_number":594,"context_line":"    def test_spawn_on_initializes_profiler(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"053707cd_efdd7669","line":591,"in_reply_to":"5a495286_31b1572f","updated":"2026-10-06 20:51:27.000000000","message":"Done","commit_id":"4a6574fea1111d1e066a9350440895bc3d4cd041"}]}