)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"42d4dadb77f23144fc0983b75218684bc873c720","unresolved":true,"context_lines":[{"line_number":20,"context_line":"those are singleton per process and can be shutdown all"},{"line_number":21,"context_line":"together."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"This change might looks big in size but most of the test file"},{"line_number":24,"context_line":"changes are because of moving things from utils.py to a new"},{"line_number":25,"context_line":"thread pool factory file. A few key things changed here:"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"0436e090_9ddde959","line":23,"updated":"2026-08-14 17:33:06.000000000","message":"It looks big and _is_ big :)\n\nI think that what you\u0027re going for here makes sense and certainly unifies a lot of this stuff in a single place. However, it feels like it\u0027s still pretty specific to compute manager and the details of the individual executors we have.\n\nI\u0027m a teensy bit concerned about all the churn here, in critical code that we\u0027ve also recently churned up with the eventlet stuff. If gibi is really okay with it all, then fair enough but I think it deserves careful review.","commit_id":"7c7c68136de2f63de76579fbc6c37bb3ea68333a"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d6aeac1298fea5123622b1223e0a1c9c98280614","unresolved":true,"context_lines":[{"line_number":20,"context_line":"those are singleton per process and can be shutdown all"},{"line_number":21,"context_line":"together."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"This change might looks big in size but most of the test file"},{"line_number":24,"context_line":"changes are because of moving things from utils.py to a new"},{"line_number":25,"context_line":"thread pool factory file. A few key things changed here:"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"33a1a8c7_2067418e","line":23,"in_reply_to":"0436e090_9ddde959","updated":"2026-08-14 18:05:34.000000000","message":"yeah, it change the critical code but I feel unifying this code will be helpful to ease the long term maintenance. but yeah, i leave the call to you guys.","commit_id":"7c7c68136de2f63de76579fbc6c37bb3ea68333a"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78fc96cd43c81e961be0fd076a681e2ac4ad104f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"478e4a79_2809e4a4","updated":"2026-08-06 09:30:06.000000000","message":"I\u0027m happy about the refactoring plans","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"c31763807e3c9c0cff0dc04e12e42c4b8535ca80","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"1699a79c_06b8d915","updated":"2026-08-10 16:22:16.000000000","message":"recheck sdk job failure not related","commit_id":"2002a7198bebea694d18d0db255990090a26227c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e644d02698bb895efe662165e90ad3b449c43118","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"5d7f433d_c9086bae","updated":"2026-08-11 16:58:28.000000000","message":"I had limited time so only focused on the core pieces first. I will get back to this later to review the rest but I already left some actionable feedback. Thanks for developing this, I think we are moving to the right direction with this code.","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"7f6e81e51d7d99b73cdba0bfd0af75d5834ca2f6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"35f2db4e_c3d88927","updated":"2026-08-11 14:43:54.000000000","message":"Shoud we also update doc/source/admin/concurrency.rst ?","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14bf20ba6a478d8931da3df92b1928a70b629224","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"aa77fc94_edc436a2","updated":"2026-08-14 09:24:17.000000000","message":"Almost good. I would not block on the indentation nit, but the lost test coverage needs to be fixed.","commit_id":"af7bbc157227ad984c28328bec1853025bef9a20"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3e25ef15e80a0b3d3cd59dd2b8888fe4227255d4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"11207490_030d25ec","updated":"2026-08-13 22:42:01.000000000","message":"recheck bug 2160254","commit_id":"af7bbc157227ad984c28328bec1853025bef9a20"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"31df938c5229c807c726028066a014d8db6e2e1c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"45754027_edde59ca","updated":"2026-08-13 20:01:28.000000000","message":"recheck not sure why shelve test failing in nova-grenade-multinode, seems not related","commit_id":"af7bbc157227ad984c28328bec1853025bef9a20"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4cf7ee01bb21a3337ac5759cfdea37d6d846e078","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"cb806d23_e40aaec8","updated":"2026-08-13 15:04:25.000000000","message":"recheck server stuck in shelved offload state in grenade job","commit_id":"af7bbc157227ad984c28328bec1853025bef9a20"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5c7e718094195317e49c33d5ff3dad3d49702646","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"54873665_d417c1dc","updated":"2026-08-14 14:01:34.000000000","message":"Thanks. I\u0027m OK to land this. I would use the simpler locking code even if it is less performant but I won\u0027t block on it.","commit_id":"7c7c68136de2f63de76579fbc6c37bb3ea68333a"}],"nova/compute/manager.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e644d02698bb895efe662165e90ad3b449c43118","unresolved":true,"context_lines":[{"line_number":1852,"context_line":""},{"line_number":1853,"context_line":"    def _cleanup_live_migrations_in_pool(self):"},{"line_number":1854,"context_line":"        # Shutdown the pool so we don\u0027t get new requests."},{"line_number":1855,"context_line":"        self._live_migration_executor.shutdown(wait\u003dFalse)"},{"line_number":1856,"context_line":"        # For any queued migrations, cancel the migration and update"},{"line_number":1857,"context_line":"        # its status."},{"line_number":1858,"context_line":"        for migration, future in self._waiting_live_migrations.values():"}],"source_content_type":"text/x-python","patch_set":6,"id":"d98c7763_edfd721c","side":"PARENT","line":1855,"updated":"2026-08-11 16:58:28.000000000","message":"What is the equivalent safety net now that this explicit shutdown is removed?","commit_id":"2a745c7e15376b8c4b9581e9f149bb02ef596ebc"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0345f370a6dae9c9510006becebca8bb28cb1f09","unresolved":false,"context_lines":[{"line_number":1852,"context_line":""},{"line_number":1853,"context_line":"    def _cleanup_live_migrations_in_pool(self):"},{"line_number":1854,"context_line":"        # Shutdown the pool so we don\u0027t get new requests."},{"line_number":1855,"context_line":"        self._live_migration_executor.shutdown(wait\u003dFalse)"},{"line_number":1856,"context_line":"        # For any queued migrations, cancel the migration and update"},{"line_number":1857,"context_line":"        # its status."},{"line_number":1858,"context_line":"        for migration, future in self._waiting_live_migrations.values():"}],"source_content_type":"text/x-python","patch_set":6,"id":"392b9ab4_6c72b68b","side":"PARENT","line":1855,"in_reply_to":"d98c7763_edfd721c","updated":"2026-08-11 18:06:27.000000000","message":"RPC server, so RPC server is stopped as soon as shutdown is initiated so no new live migration can come. If anything in-progress then manager wait for that to complete or this will cleanup. I will add comment here.","commit_id":"2a745c7e15376b8c4b9581e9f149bb02ef596ebc"},{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"7f6e81e51d7d99b73cdba0bfd0af75d5834ca2f6","unresolved":true,"context_lines":[{"line_number":1754,"context_line":"        self.driver.cleanup_host(host\u003dself.host)"},{"line_number":1755,"context_line":"        self._cleanup_live_migrations_in_pool()"},{"line_number":1756,"context_line":"        # NOTE: graceful shutdown needs to take care of the executors"},{"line_number":1757,"context_line":"        # self._sync_power_executor.shutdown()"},{"line_number":1758,"context_line":"        # utils.destroy_long_task_executor()"},{"line_number":1759,"context_line":"        # utils.destroy_default_executor()"},{"line_number":1760,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ad3c62f3_d5db50d1","line":1757,"updated":"2026-08-11 14:43:54.000000000","message":"is this comment invalid now?","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"a1b675c5228d1ae25b30ba2571d055afc4a03515","unresolved":false,"context_lines":[{"line_number":1754,"context_line":"        self.driver.cleanup_host(host\u003dself.host)"},{"line_number":1755,"context_line":"        self._cleanup_live_migrations_in_pool()"},{"line_number":1756,"context_line":"        # NOTE: graceful shutdown needs to take care of the executors"},{"line_number":1757,"context_line":"        # self._sync_power_executor.shutdown()"},{"line_number":1758,"context_line":"        # utils.destroy_long_task_executor()"},{"line_number":1759,"context_line":"        # utils.destroy_default_executor()"},{"line_number":1760,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"248b7d15_b4c83d68","line":1757,"in_reply_to":"ad3c62f3_d5db50d1","updated":"2026-08-11 17:53:45.000000000","message":"not in this change as i separated the graceful shutdown part in another change on top of it and keeping it purely a refactoring one\n\n- https://review.opendev.org/c/openstack/nova/+/1000211/3","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e644d02698bb895efe662165e90ad3b449c43118","unresolved":true,"context_lines":[{"line_number":11173,"context_line":"                    syncs.add(uuid)"},{"line_number":11174,"context_line":"                    thread_pool_factory.spawn_on("},{"line_number":11175,"context_line":"                        _sync, db_instance,"},{"line_number":11176,"context_line":"                        executor_type\u003d("},{"line_number":11177,"context_line":"                            thread_pool_factory.ExecutorType.SYNC_POWER))"},{"line_number":11178,"context_line":""},{"line_number":11179,"context_line":"    def _query_driver_power_state_and_sync(self, context, db_instance):"}],"source_content_type":"text/x-python","patch_set":6,"id":"4466f06f_08a60206","line":11176,"range":{"start_line":11176,"start_character":38,"end_line":11176,"end_character":39},"updated":"2026-08-11 16:58:28.000000000","message":"nit: I guess this \u0027(\u0027 pair is not needed","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0345f370a6dae9c9510006becebca8bb28cb1f09","unresolved":false,"context_lines":[{"line_number":11173,"context_line":"                    syncs.add(uuid)"},{"line_number":11174,"context_line":"                    thread_pool_factory.spawn_on("},{"line_number":11175,"context_line":"                        _sync, db_instance,"},{"line_number":11176,"context_line":"                        executor_type\u003d("},{"line_number":11177,"context_line":"                            thread_pool_factory.ExecutorType.SYNC_POWER))"},{"line_number":11178,"context_line":""},{"line_number":11179,"context_line":"    def _query_driver_power_state_and_sync(self, context, db_instance):"}],"source_content_type":"text/x-python","patch_set":6,"id":"da914f8e_c4b1d876","line":11176,"range":{"start_line":11176,"start_character":38,"end_line":11176,"end_character":39},"in_reply_to":"4466f06f_08a60206","updated":"2026-08-11 18:06:27.000000000","message":"Done","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":false,"context_lines":[{"line_number":1761,"context_line":"    def _cleanup_live_migrations_in_pool(self):"},{"line_number":1762,"context_line":"        # Once shutdown is initiated, main RPC server is stopped which"},{"line_number":1763,"context_line":"        # will make sure that no new live migration will be accepted"},{"line_number":1764,"context_line":"        # by the shutting down compute."},{"line_number":1765,"context_line":"        # For any queued migrations, cancel the migration and update"},{"line_number":1766,"context_line":"        # its status."},{"line_number":1767,"context_line":"        for migration, future in self._waiting_live_migrations.values():"}],"source_content_type":"text/x-python","patch_set":8,"id":"b1ea1835_4a00f9e1","line":1764,"updated":"2026-08-12 12:48:52.000000000","message":"OK so rpc_server.wait() happens first, then we call this. And rpc_server.wait() ensures that any PRC handler thread finished before returns. So when we are here there are no rpc handlers running any more. And we know that the only thing that can put a new task into this executor is an RPC handler running the live_migration method","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":1761,"context_line":"    def _cleanup_live_migrations_in_pool(self):"},{"line_number":1762,"context_line":"        # Once shutdown is initiated, main RPC server is stopped which"},{"line_number":1763,"context_line":"        # will make sure that no new live migration will be accepted"},{"line_number":1764,"context_line":"        # by the shutting down compute."},{"line_number":1765,"context_line":"        # For any queued migrations, cancel the migration and update"},{"line_number":1766,"context_line":"        # its status."},{"line_number":1767,"context_line":"        for migration, future in self._waiting_live_migrations.values():"}],"source_content_type":"text/x-python","patch_set":8,"id":"bb3a7097_4b000ba2","line":1764,"in_reply_to":"b1ea1835_4a00f9e1","updated":"2026-08-13 04:02:10.000000000","message":"that is for the a completely new live migration but for already accepted live migration, there is possibility that executor can get a new task because rpcserver_alt is still up for in-progress live migrations. I re-draw the flow and realized that there is a one narrow (but possible) gap here which was handled by the self._live_migration_executor.shutdown(wait\u003dFlase). The complete flow is (writting the complete flow but if its too long to read, you can jump to point 7 for the **GAP** 😉):\n\n1. rpcserver.stop() \n\n   This will stop accepting the new live migration request. so no more update in self._waiting_live_migrations\n\n2. manager.graceful_shutdown() \n   \n   This will wait for in-progress live migration to complete. Already accepted live migration is added in task_tracking list first, spawn, and then added in self._waiting_live_migrations. so manager graceful_shutdown will wait for this task to complete. if live migration is started in executor then  it is poped out from self._waiting_live_migrations (in _do_live_migration).\n\n3. _cleanup_live_migrations_in_pool\n   \n     This cleanup function is called by and at the end of manager.graceful_shutdown(). It will cleanup self._waiting_live_migrations.\n   \n4. rpcserver.wait() \n\n   This make sure if anything still running on main rpc server will be finished. NOTE: in-progress live migration operations are run on rpcserver_alt so this wait() here is noop for live migration context.\n\n5. rpcserver_alt.stop()\n\n    At this stage, manager has completed or timeout the in-progress live migration.\n\n6. rpcserver_alt.wait()\n\n   In case the last rpc call for in-progress live migration is going on then this wait make sure that they get 2nd chance to finish before service/executors are shutdown.\n\n7. shutdown all thread pool executors\n\n   Here we stop all the executors including live migration executor But there is a gap.\n   **GAP**: we shutdown executors with wait\u003dfalse so that any queued task will not be picked up/run any more. but still if executor pick up the task before shutdown executors is called then executor shutdown will wait for that to complete[1] and at this time we have stopped our both rpc servers so it may not be able to complete if that task need to make any RPC call (in live migraiton casem it does multiple rpc call to dest and source.\n   \n   This made me realize that we should shutdown the executors after driver.cleanup_host() which is last thing who will use the executors and before we stop the rpcserver_alt. THis way we will not let any executors to pick up the tasks when manager graceful_shutdown is over (means no new live migration tasks can happen when _cleanup_live_migrations_in_pool is called). Also, it will achieve the main purpose of rpcserver_alt which is \"keep this alt rpc handler active until anything running for compute service\". \n\nBecause I am doing shutdown_all in separate change, let me remove  self._live_migration_executor.shutdown(wait\u003dFalse) also in that change instead of this. - https://review.opendev.org/c/openstack/nova/+/1000211\n\n[1] https://github.com/openstack/futurist/blob/51390129253181855826c75f8e8c90ff13399020/futurist/_thread.py#L121","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14bf20ba6a478d8931da3df92b1928a70b629224","unresolved":false,"context_lines":[{"line_number":1761,"context_line":"    def _cleanup_live_migrations_in_pool(self):"},{"line_number":1762,"context_line":"        # Once shutdown is initiated, main RPC server is stopped which"},{"line_number":1763,"context_line":"        # will make sure that no new live migration will be accepted"},{"line_number":1764,"context_line":"        # by the shutting down compute."},{"line_number":1765,"context_line":"        # For any queued migrations, cancel the migration and update"},{"line_number":1766,"context_line":"        # its status."},{"line_number":1767,"context_line":"        for migration, future in self._waiting_live_migrations.values():"}],"source_content_type":"text/x-python","patch_set":8,"id":"71460e38_478bbaed","line":1764,"in_reply_to":"bb3a7097_4b000ba2","updated":"2026-08-14 09:24:17.000000000","message":"ack","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"}],"nova/service.py":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"ced09c2de526b1f661cb651e02925ba0a6de4ccf","unresolved":true,"context_lines":[{"line_number":416,"context_line":"                LOG.exception(\u0027Error occurred during %s manager graceful \u0027"},{"line_number":417,"context_line":"                              \u0027shutdown\u0027, self.binary)"},{"line_number":418,"context_line":"            finally:"},{"line_number":419,"context_line":"                # Shut down any thread pool executor created (via"},{"line_number":420,"context_line":"                # nova.utils.create_executor()) by this process."},{"line_number":421,"context_line":"                utils.shutdown_all_executors()"},{"line_number":422,"context_line":"                finished.set()"},{"line_number":423,"context_line":""},{"line_number":424,"context_line":"        # NOTE(gmaan): manager\u0027s graceful_shutdown does two things 1. wait for"}],"source_content_type":"text/x-python","patch_set":2,"id":"6bf6b29c_e9688a4c","line":421,"range":{"start_line":419,"start_character":0,"end_line":421,"end_character":46},"updated":"2026-07-30 19:42:08.000000000","message":"note to me, move this at the end of stop() (L469) when rpcserver_alt is also shutdown. Because that time, all the RPC threads are completed and no chance to have any new tasks (even from the in-progress operations via rpcserver_alt) so it is safe and correct time to shutdown all thread executors created by Nova.","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"}],"nova/test.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":true,"context_lines":[{"line_number":304,"context_line":"        self.useFixture(fixtures.MockPatch("},{"line_number":305,"context_line":"            \u0027nova.thread_pool_factory.reset_all_executors\u0027))"},{"line_number":306,"context_line":"        self.addCleanup("},{"line_number":307,"context_line":"            setattr, thread_pool_factory.FACTORY, \u0027_shutdown\u0027, False)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"        # FIXME(danms): Disable this for all tests by default to avoid breaking"},{"line_number":310,"context_line":"        # any that depend on default/previous ordering"}],"source_content_type":"text/x-python","patch_set":8,"id":"2f8cd59d_10b0bf01","line":307,"updated":"2026-08-12 12:48:52.000000000","message":"So I guess our tests cleanup eventually call shutdown_all_executors that setting this flag to True and preventing the next test case in the same test executor process to create new thread pools. But just above you mocked out shutdown_all_executors so I\u0027m not sure how that can happen as that mock will prevent this flag to be set. \n\nMaking a step back what we actually want is that a test is not leaking anything to the next test running in the same executor process. So I think what we want is to actually \n1) fail the test if it leaks something\n2) force the reset_all call at the end of the test so it does not leak a thread in a pool to the next test.\n\nSome of this isolation is implemented by IsolatedExecutorFixture. I\u0027m not sure adding the above mocks does not interfere with that fixture.","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        self.useFixture(fixtures.MockPatch("},{"line_number":305,"context_line":"            \u0027nova.thread_pool_factory.reset_all_executors\u0027))"},{"line_number":306,"context_line":"        self.addCleanup("},{"line_number":307,"context_line":"            setattr, thread_pool_factory.FACTORY, \u0027_shutdown\u0027, False)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"        # FIXME(danms): Disable this for all tests by default to avoid breaking"},{"line_number":310,"context_line":"        # any that depend on default/previous ordering"}],"source_content_type":"text/x-python","patch_set":8,"id":"f875c3c6_074cb900","line":307,"in_reply_to":"2f8cd59d_10b0bf01","updated":"2026-08-13 04:02:10.000000000","message":"yeah, IsolatedExecutorFixture should take care of all of these, I added these for safer side but i agree with you that if any test leaks it then we can fix it.","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"}],"nova/tests/fixtures/nova.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":true,"context_lines":[{"line_number":1314,"context_line":"        assert not thread_pool_factory.FACTORY._all_executors"},{"line_number":1315,"context_line":""},{"line_number":1316,"context_line":"        origi_get_executor \u003d thread_pool_factory.get_executor"},{"line_number":1317,"context_line":"        self.executors \u003d {}"},{"line_number":1318,"context_line":""},{"line_number":1319,"context_line":"        def _get_executor(executor_type, **kwargs):"},{"line_number":1320,"context_line":"            executor \u003d origi_get_executor(executor_type, **kwargs)"}],"source_content_type":"text/x-python","patch_set":8,"id":"16858967_18a62410","line":1317,"updated":"2026-08-12 12:48:52.000000000","message":"Is this the same as thread_pool_factory.FACTORY._all_executors now?","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":1314,"context_line":"        assert not thread_pool_factory.FACTORY._all_executors"},{"line_number":1315,"context_line":""},{"line_number":1316,"context_line":"        origi_get_executor \u003d thread_pool_factory.get_executor"},{"line_number":1317,"context_line":"        self.executors \u003d {}"},{"line_number":1318,"context_line":""},{"line_number":1319,"context_line":"        def _get_executor(executor_type, **kwargs):"},{"line_number":1320,"context_line":"            executor \u003d origi_get_executor(executor_type, **kwargs)"}],"source_content_type":"text/x-python","patch_set":8,"id":"26eacefe_05eba786","line":1317,"in_reply_to":"16858967_18a62410","updated":"2026-08-13 04:02:10.000000000","message":"no, we do not need this extra dict to track executors","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":true,"context_lines":[{"line_number":1336,"context_line":"    def _cleanup_executors(self):"},{"line_number":1337,"context_line":"        for executor in self.executors.values():"},{"line_number":1338,"context_line":"            self.do_cleanup_executor(executor)"},{"line_number":1339,"context_line":"        for executor_type in self.executors:"},{"line_number":1340,"context_line":"            thread_pool_factory.FACTORY._all_executors.pop(executor_type, None)"},{"line_number":1341,"context_line":""},{"line_number":1342,"context_line":"    def do_cleanup_executor(self, executor):"},{"line_number":1343,"context_line":"        # NOTE(gibi): we cannot rely on utils.concurrency_mode_threading"}],"source_content_type":"text/x-python","patch_set":8,"id":"d71b33a2_3e804ab0","line":1340,"range":{"start_line":1339,"start_character":0,"end_line":1340,"end_character":79},"updated":"2026-08-12 12:48:52.000000000","message":"I fancy calling reset_all here to do the extra cleanup","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14bf20ba6a478d8931da3df92b1928a70b629224","unresolved":false,"context_lines":[{"line_number":1336,"context_line":"    def _cleanup_executors(self):"},{"line_number":1337,"context_line":"        for executor in self.executors.values():"},{"line_number":1338,"context_line":"            self.do_cleanup_executor(executor)"},{"line_number":1339,"context_line":"        for executor_type in self.executors:"},{"line_number":1340,"context_line":"            thread_pool_factory.FACTORY._all_executors.pop(executor_type, None)"},{"line_number":1341,"context_line":""},{"line_number":1342,"context_line":"    def do_cleanup_executor(self, executor):"},{"line_number":1343,"context_line":"        # NOTE(gibi): we cannot rely on utils.concurrency_mode_threading"}],"source_content_type":"text/x-python","patch_set":8,"id":"58f4d3fd_1f8e24c8","line":1340,"range":{"start_line":1339,"start_character":0,"end_line":1340,"end_character":79},"in_reply_to":"cb2792a1_6b076c75","updated":"2026-08-14 09:24:17.000000000","message":"thanks","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":1336,"context_line":"    def _cleanup_executors(self):"},{"line_number":1337,"context_line":"        for executor in self.executors.values():"},{"line_number":1338,"context_line":"            self.do_cleanup_executor(executor)"},{"line_number":1339,"context_line":"        for executor_type in self.executors:"},{"line_number":1340,"context_line":"            thread_pool_factory.FACTORY._all_executors.pop(executor_type, None)"},{"line_number":1341,"context_line":""},{"line_number":1342,"context_line":"    def do_cleanup_executor(self, executor):"},{"line_number":1343,"context_line":"        # NOTE(gibi): we cannot rely on utils.concurrency_mode_threading"}],"source_content_type":"text/x-python","patch_set":8,"id":"cb2792a1_6b076c75","line":1340,"range":{"start_line":1339,"start_character":0,"end_line":1340,"end_character":79},"in_reply_to":"d71b33a2_3e804ab0","updated":"2026-08-13 04:02:10.000000000","message":"let me refactor clean up with reset_all, I will keep the greenthread cleanup as it is and rest all should be cleaned up by reset_all","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":true,"context_lines":[{"line_number":1982,"context_line":"        # let\u0027s replace nova.thread_pool_factory.spawn with the wrapped one"},{"line_number":1983,"context_line":"        # that injects our initialization to the child eventlet"},{"line_number":1984,"context_line":"        self.useFixture("},{"line_number":1985,"context_line":"    fixtures.MonkeyPatch("},{"line_number":1986,"context_line":"        \u0027nova.thread_pool_factory.spawn\u0027,"},{"line_number":1987,"context_line":"         wrapped_spawn))"},{"line_number":1988,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"8c6e16e2_926551cd","line":1985,"updated":"2026-08-12 12:48:52.000000000","message":"nit: indentation","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":1982,"context_line":"        # let\u0027s replace nova.thread_pool_factory.spawn with the wrapped one"},{"line_number":1983,"context_line":"        # that injects our initialization to the child eventlet"},{"line_number":1984,"context_line":"        self.useFixture("},{"line_number":1985,"context_line":"    fixtures.MonkeyPatch("},{"line_number":1986,"context_line":"        \u0027nova.thread_pool_factory.spawn\u0027,"},{"line_number":1987,"context_line":"         wrapped_spawn))"},{"line_number":1988,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ccf95f27_162738cd","line":1985,"in_reply_to":"8c6e16e2_926551cd","updated":"2026-08-13 04:02:10.000000000","message":"Done","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14bf20ba6a478d8931da3df92b1928a70b629224","unresolved":true,"context_lines":[{"line_number":1961,"context_line":"        # let\u0027s replace nova.thread_pool_factory.spawn with the wrapped one"},{"line_number":1962,"context_line":"        # that injects our initialization to the child eventlet"},{"line_number":1963,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":1964,"context_line":"        \u0027nova.thread_pool_factory.spawn\u0027,"},{"line_number":1965,"context_line":"         wrapped_spawn))"},{"line_number":1966,"context_line":""},{"line_number":1967,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"d9761222_1c58dad0","line":1964,"updated":"2026-08-14 09:24:17.000000000","message":"nit: sorry this still seems under indented","commit_id":"af7bbc157227ad984c28328bec1853025bef9a20"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b35cff73157656e4c532093be0d00b49861da713","unresolved":false,"context_lines":[{"line_number":1961,"context_line":"        # let\u0027s replace nova.thread_pool_factory.spawn with the wrapped one"},{"line_number":1962,"context_line":"        # that injects our initialization to the child eventlet"},{"line_number":1963,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":1964,"context_line":"        \u0027nova.thread_pool_factory.spawn\u0027,"},{"line_number":1965,"context_line":"         wrapped_spawn))"},{"line_number":1966,"context_line":""},{"line_number":1967,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"4aca9813_de26d58e","line":1964,"in_reply_to":"d9761222_1c58dad0","updated":"2026-08-14 13:38:21.000000000","message":"Done","commit_id":"af7bbc157227ad984c28328bec1853025bef9a20"}],"nova/tests/functional/test_graceful_shutdown.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":true,"context_lines":[{"line_number":879,"context_line":"        # NOTE: super().setUp() below mocks"},{"line_number":880,"context_line":"        # nova.thread_pool_factory.shutdown_all_executors. Capture the real"},{"line_number":881,"context_line":"        # function here first so tests that need the real shutdown to"},{"line_number":882,"context_line":"        # happen (e.g. to avoid leaking executors into later tests) can"},{"line_number":883,"context_line":"        # still call it."},{"line_number":884,"context_line":"        self._real_shutdown_all_executors \u003d ("},{"line_number":885,"context_line":"            thread_pool_factory.shutdown_all_executors)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7797e6b1_7d1f3d9d","line":882,"range":{"start_line":882,"start_character":17,"end_line":882,"end_character":67},"updated":"2026-08-12 12:48:52.000000000","message":"What would happen if we don\u0027t mock shutdown_all_executors globally? Would that work?","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":879,"context_line":"        # NOTE: super().setUp() below mocks"},{"line_number":880,"context_line":"        # nova.thread_pool_factory.shutdown_all_executors. Capture the real"},{"line_number":881,"context_line":"        # function here first so tests that need the real shutdown to"},{"line_number":882,"context_line":"        # happen (e.g. to avoid leaking executors into later tests) can"},{"line_number":883,"context_line":"        # still call it."},{"line_number":884,"context_line":"        self._real_shutdown_all_executors \u003d ("},{"line_number":885,"context_line":"            thread_pool_factory.shutdown_all_executors)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7009ca30_8ce3bbff","line":882,"range":{"start_line":882,"start_character":17,"end_line":882,"end_character":67},"in_reply_to":"7797e6b1_7d1f3d9d","updated":"2026-08-13 04:02:10.000000000","message":"removed the mock at global level and let IsolatedExecutorFixture take care of executors cleanup and isolation.","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":891,"context_line":"        manager \u003d self.conductor_service.manager"},{"line_number":892,"context_line":"        self.assertFalse(manager._shutdown_in_progress.is_set())"},{"line_number":893,"context_line":""},{"line_number":894,"context_line":"        thread_pool_factory.get_executor("},{"line_number":895,"context_line":"    thread_pool_factory.ExecutorType.CACHE_IMAGES)"},{"line_number":896,"context_line":"        self.addCleanup(self._real_shutdown_all_executors)"},{"line_number":897,"context_line":""},{"line_number":898,"context_line":"        start \u003d time.monotonic()"},{"line_number":899,"context_line":"        self.conductor_service.stop()"}],"source_content_type":"text/x-python","patch_set":8,"id":"cbe954ce_f1f37b19","line":896,"range":{"start_line":894,"start_character":0,"end_line":896,"end_character":58},"updated":"2026-08-13 04:02:10.000000000","message":"I do not know why i created executor in this tests, it is not needed.","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"}],"nova/tests/unit/cmd/test_scheduler.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":true,"context_lines":[{"line_number":72,"context_line":"            thread_pool_factory.ExecutorType.SCATTER_GATHER)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        # nova.test.TestCase.setUp() mocks reset_all_executors by"},{"line_number":75,"context_line":"        # default; un-mock it here since this test needs the real reset"},{"line_number":76,"context_line":"        # to happen."},{"line_number":77,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":78,"context_line":"            \u0027nova.thread_pool_factory.reset_all_executors\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"f35eeb4e_1ea4037f","line":75,"updated":"2026-08-12 12:48:52.000000000","message":"Can we not have that mock globally at all?","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":72,"context_line":"            thread_pool_factory.ExecutorType.SCATTER_GATHER)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        # nova.test.TestCase.setUp() mocks reset_all_executors by"},{"line_number":75,"context_line":"        # default; un-mock it here since this test needs the real reset"},{"line_number":76,"context_line":"        # to happen."},{"line_number":77,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":78,"context_line":"            \u0027nova.thread_pool_factory.reset_all_executors\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ac08ef16_4e6c6250","line":75,"in_reply_to":"f35eeb4e_1ea4037f","updated":"2026-08-13 04:02:10.000000000","message":"Done","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":true,"context_lines":[{"line_number":1666,"context_line":"            thread_pool_factory.ExecutorType.LIVE_MIGRATION] \u003d ("},{"line_number":1667,"context_line":"                futurist.SynchronousExecutor())"},{"line_number":1668,"context_line":"        self.addCleanup("},{"line_number":1669,"context_line":"            thread_pool_factory.FACTORY._all_executors.pop,"},{"line_number":1670,"context_line":"            thread_pool_factory.ExecutorType.LIVE_MIGRATION, None)"},{"line_number":1671,"context_line":"        # NOTE(gibi): the _sync_power_states periodic task in the"},{"line_number":1672,"context_line":"        # ComputeManager spawning concurrent tasks and uses a lock to"}],"source_content_type":"text/x-python","patch_set":8,"id":"3e99444c_67b8857a","line":1669,"updated":"2026-08-12 12:48:52.000000000","message":"hm I would rather call just shutdown_all_executors instead of calling into the internals of the factory. Also this call now does not stop the executor just make the factory forget about it. We probably want to stop the executor too to avoid leaking. (I know shutdown_all_executors is mocked globally but I\u0027m arguing elsewhere not to mock it)","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":1666,"context_line":"            thread_pool_factory.ExecutorType.LIVE_MIGRATION] \u003d ("},{"line_number":1667,"context_line":"                futurist.SynchronousExecutor())"},{"line_number":1668,"context_line":"        self.addCleanup("},{"line_number":1669,"context_line":"            thread_pool_factory.FACTORY._all_executors.pop,"},{"line_number":1670,"context_line":"            thread_pool_factory.ExecutorType.LIVE_MIGRATION, None)"},{"line_number":1671,"context_line":"        # NOTE(gibi): the _sync_power_states periodic task in the"},{"line_number":1672,"context_line":"        # ComputeManager spawning concurrent tasks and uses a lock to"}],"source_content_type":"text/x-python","patch_set":8,"id":"4c67f97f_a3c8ef7a","line":1669,"in_reply_to":"3e99444c_67b8857a","updated":"2026-08-13 04:02:10.000000000","message":"ok, as _live_migration_executor is not a instance variable of compute manager and maintained by thread pool factory, I do not think we need to create any here.","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":true,"context_lines":[{"line_number":1674,"context_line":"        # synchronous meaning the tasks runs on the caller thread. This means"},{"line_number":1675,"context_line":"        # the simple lock causes a deadlock in the unit test. Upgrade that lock"},{"line_number":1676,"context_line":"        # to be reentrant so the test can pass with synchronous spawn."},{"line_number":1677,"context_line":"        self.useFixture(fixtures.SpawnIsSynchronousFixture())"},{"line_number":1678,"context_line":"        self.compute._syncs_in_progress_lock \u003d threading.RLock()"},{"line_number":1679,"context_line":""},{"line_number":1680,"context_line":"        self.image_api \u003d image_api.API()"}],"source_content_type":"text/x-python","patch_set":8,"id":"06bb5f5e_bfe257b3","line":1677,"updated":"2026-08-12 12:48:52.000000000","message":"Would it be enough to use this now to force all executors including the LIVE_MIGRATION to be Synchronous?","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":true,"context_lines":[{"line_number":1674,"context_line":"        # synchronous meaning the tasks runs on the caller thread. This means"},{"line_number":1675,"context_line":"        # the simple lock causes a deadlock in the unit test. Upgrade that lock"},{"line_number":1676,"context_line":"        # to be reentrant so the test can pass with synchronous spawn."},{"line_number":1677,"context_line":"        self.useFixture(fixtures.SpawnIsSynchronousFixture())"},{"line_number":1678,"context_line":"        self.compute._syncs_in_progress_lock \u003d threading.RLock()"},{"line_number":1679,"context_line":""},{"line_number":1680,"context_line":"        self.image_api \u003d image_api.API()"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f13f92f_51b7ec10","line":1677,"in_reply_to":"06bb5f5e_bfe257b3","updated":"2026-08-13 04:02:10.000000000","message":"did not get your question completly but if you are asking if live migration also works with this sunc executors then yes because I do nto see any change here. as this fixture mock the spawn_on/spawn then it would not go to pool factory to create any new executors.","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14bf20ba6a478d8931da3df92b1928a70b629224","unresolved":false,"context_lines":[{"line_number":1674,"context_line":"        # synchronous meaning the tasks runs on the caller thread. This means"},{"line_number":1675,"context_line":"        # the simple lock causes a deadlock in the unit test. Upgrade that lock"},{"line_number":1676,"context_line":"        # to be reentrant so the test can pass with synchronous spawn."},{"line_number":1677,"context_line":"        self.useFixture(fixtures.SpawnIsSynchronousFixture())"},{"line_number":1678,"context_line":"        self.compute._syncs_in_progress_lock \u003d threading.RLock()"},{"line_number":1679,"context_line":""},{"line_number":1680,"context_line":"        self.image_api \u003d image_api.API()"}],"source_content_type":"text/x-python","patch_set":8,"id":"b58031d6_3925086a","line":1677,"in_reply_to":"9f13f92f_51b7ec10","updated":"2026-08-14 09:24:17.000000000","message":"yeah that was my question and then it seems the answer is yes, it is enough to have SpawnIsSynchronousFixture added here, and does not need to manually create SynchronousExecutor above. Thanks.","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":true,"context_lines":[{"line_number":970,"context_line":"        self.flags(max_concurrent_builds\u003d0)"},{"line_number":971,"context_line":"        self._test_max_concurrent_builds()"},{"line_number":972,"context_line":""},{"line_number":973,"context_line":"    def test_max_concurrent_builds_semaphore_limited(self):"},{"line_number":974,"context_line":"        self.flags(max_concurrent_builds\u003d123)"},{"line_number":975,"context_line":"        compute \u003d manager.ComputeManager()"},{"line_number":976,"context_line":"        if utils.concurrency_mode_threading():"}],"source_content_type":"text/x-python","patch_set":8,"id":"5bbcf896_a12a32ec","line":973,"updated":"2026-08-12 12:48:52.000000000","message":"The code that decides the size of the executor is moved and also this patch moves *when* the executor is created by the compute manager. So it feels like this test case is not a single case any more. I mean the semaphore is created by the compute manager init that test is valid here. But the compute manager init is not creating the executor any more so asserting that here is not valid here now.\n\n(same is applicable below to the other executor tests here)","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":970,"context_line":"        self.flags(max_concurrent_builds\u003d0)"},{"line_number":971,"context_line":"        self._test_max_concurrent_builds()"},{"line_number":972,"context_line":""},{"line_number":973,"context_line":"    def test_max_concurrent_builds_semaphore_limited(self):"},{"line_number":974,"context_line":"        self.flags(max_concurrent_builds\u003d123)"},{"line_number":975,"context_line":"        compute \u003d manager.ComputeManager()"},{"line_number":976,"context_line":"        if utils.concurrency_mode_threading():"}],"source_content_type":"text/x-python","patch_set":8,"id":"df981360_6b028bcd","line":973,"in_reply_to":"5bbcf896_a12a32ec","updated":"2026-08-13 04:02:10.000000000","message":"Done","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14bf20ba6a478d8931da3df92b1928a70b629224","unresolved":true,"context_lines":[{"line_number":1098,"context_line":"            self.assertEqual(1000, compute._snapshot_semaphore._value)"},{"line_number":1099,"context_line":""},{"line_number":1100,"context_line":"    @mock.patch.object(thread_pool_factory.LOG, \u0027warning\u0027)"},{"line_number":1101,"context_line":"    def test_max_c_builds_and_snapshots_different_limits(self, mock_log):"},{"line_number":1102,"context_line":"        self.flags(max_concurrent_builds\u003d124)"},{"line_number":1103,"context_line":"        self.flags(max_concurrent_snapshots\u003d123)"},{"line_number":1104,"context_line":"        compute \u003d manager.ComputeManager()"}],"source_content_type":"text/x-python","patch_set":9,"id":"e8c7e729_7ecaac60","line":1101,"updated":"2026-08-14 09:24:17.000000000","message":"mock_log is unused now.\n\nBtw. Thanks for removing the coverage from here as this does not belong directly to the compute manager or at least not for the `__init__` of it as the executor creation does not happen at `__init__` but at the first use. But we still need some coverage for the logic somewhere. Maybe a new test case covering it that does not depend on `ComputeManager.__init__`\n\nAnd please check if other similarly moved logic is still covered by tests.","commit_id":"af7bbc157227ad984c28328bec1853025bef9a20"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b35cff73157656e4c532093be0d00b49861da713","unresolved":false,"context_lines":[{"line_number":1098,"context_line":"            self.assertEqual(1000, compute._snapshot_semaphore._value)"},{"line_number":1099,"context_line":""},{"line_number":1100,"context_line":"    @mock.patch.object(thread_pool_factory.LOG, \u0027warning\u0027)"},{"line_number":1101,"context_line":"    def test_max_c_builds_and_snapshots_different_limits(self, mock_log):"},{"line_number":1102,"context_line":"        self.flags(max_concurrent_builds\u003d124)"},{"line_number":1103,"context_line":"        self.flags(max_concurrent_snapshots\u003d123)"},{"line_number":1104,"context_line":"        compute \u003d manager.ComputeManager()"}],"source_content_type":"text/x-python","patch_set":9,"id":"40cc82bc_b9b72729","line":1101,"in_reply_to":"e8c7e729_7ecaac60","updated":"2026-08-14 13:38:21.000000000","message":"good catch. yes coverage is lost. As warning log is moved to thread_pool_factory.py, I will add a new test in test_thread_pool_factory.py\n\nI realized i can add pool size test for all executors even they are just config value","commit_id":"af7bbc157227ad984c28328bec1853025bef9a20"}],"nova/tests/unit/test_thread_pool_factory.py":[{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"7f6e81e51d7d99b73cdba0bfd0af75d5834ca2f6","unresolved":true,"context_lines":[{"line_number":307,"context_line":"        # The stats are printed *before* the work is submitted so we need an"},{"line_number":308,"context_line":"        # extra task submitted to get the stats from the above task."},{"line_number":309,"context_line":"        thread_pool_factory.spawn(self._task_finishes).result()"},{"line_number":310,"context_line":"        print(mock_debug.mock_calls)"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"        args \u003d mock_debug.mock_calls[3][1]"},{"line_number":313,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":6,"id":"c54b874b_8fc3b4ff","line":310,"updated":"2026-08-11 14:43:54.000000000","message":"is this some leftover?","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":307,"context_line":"        # The stats are printed *before* the work is submitted so we need an"},{"line_number":308,"context_line":"        # extra task submitted to get the stats from the above task."},{"line_number":309,"context_line":"        thread_pool_factory.spawn(self._task_finishes).result()"},{"line_number":310,"context_line":"        print(mock_debug.mock_calls)"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"        args \u003d mock_debug.mock_calls[3][1]"},{"line_number":313,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":6,"id":"b67dd2f2_1d776406","line":310,"in_reply_to":"00b22aec_6af34b24","updated":"2026-08-13 04:02:10.000000000","message":"Done","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"dd90ba110515765651d129b59f0091b76d4274bd","unresolved":true,"context_lines":[{"line_number":307,"context_line":"        # The stats are printed *before* the work is submitted so we need an"},{"line_number":308,"context_line":"        # extra task submitted to get the stats from the above task."},{"line_number":309,"context_line":"        thread_pool_factory.spawn(self._task_finishes).result()"},{"line_number":310,"context_line":"        print(mock_debug.mock_calls)"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"        args \u003d mock_debug.mock_calls[3][1]"},{"line_number":313,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":6,"id":"00b22aec_6af34b24","line":310,"in_reply_to":"02cd842a_f259c3d3","updated":"2026-08-12 11:19:08.000000000","message":"I think we can safe to drop that print. I\u0027m 99% sure it is a leftover form some debugging that ended up in the merged code","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"a1b675c5228d1ae25b30ba2571d055afc4a03515","unresolved":false,"context_lines":[{"line_number":307,"context_line":"        # The stats are printed *before* the work is submitted so we need an"},{"line_number":308,"context_line":"        # extra task submitted to get the stats from the above task."},{"line_number":309,"context_line":"        thread_pool_factory.spawn(self._task_finishes).result()"},{"line_number":310,"context_line":"        print(mock_debug.mock_calls)"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"        args \u003d mock_debug.mock_calls[3][1]"},{"line_number":313,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":6,"id":"02cd842a_f259c3d3","line":310,"in_reply_to":"c54b874b_8fc3b4ff","updated":"2026-08-11 17:53:45.000000000","message":"not sure, it is just moved from test_utils.py https://review.opendev.org/c/openstack/nova/+/998571/6/nova/tests/unit/test_utils.py#1608","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14bf20ba6a478d8931da3df92b1928a70b629224","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        # calling shutdown_all_executors() by the tests will permanently"},{"line_number":166,"context_line":"        # mark it as shutdown. That will break every other test in this"},{"line_number":167,"context_line":"        # process to create an executor. Mock the shutdown flag so it is"},{"line_number":168,"context_line":"        # reset once this test is done."},{"line_number":169,"context_line":"        self.useFixture(fixtures.MockPatchObject("},{"line_number":170,"context_line":"            thread_pool_factory.FACTORY, \u0027_shutdown\u0027, False))"},{"line_number":171,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"91d1ce4b_6656b5fc","line":168,"updated":"2026-08-14 09:24:17.000000000","message":"yeah this make sense in this specific test class.","commit_id":"af7bbc157227ad984c28328bec1853025bef9a20"}],"nova/tests/unit/test_utils.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f00d9dec6e94037e68ebb49cebd672d3fad6d290","unresolved":true,"context_lines":[{"line_number":51,"context_line":"# NOTE: nova.test.TestCase.setUp() mocks nova.utils.shutdown_all_executors"},{"line_number":52,"context_line":"# Capture the real function here, at module import time, before test\u0027s setUp()"},{"line_number":53,"context_line":"# mock it and can test shutdown_all_executors."},{"line_number":54,"context_line":"_REAL_SHUTDOWN_ALL_EXECUTORS \u003d utils.shutdown_all_executors"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"class GenericUtilsTestCase(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"d4033305_4c910066","line":54,"updated":"2026-07-29 14:14:14.000000000","message":"this is a global that will get carry across test cases executed by the same process that tend to lead to hard to debug test case interference. Please put this into a test setUp","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"268d7419161b169de4ab789bc92d44ae85abea15","unresolved":false,"context_lines":[{"line_number":51,"context_line":"# NOTE: nova.test.TestCase.setUp() mocks nova.utils.shutdown_all_executors"},{"line_number":52,"context_line":"# Capture the real function here, at module import time, before test\u0027s setUp()"},{"line_number":53,"context_line":"# mock it and can test shutdown_all_executors."},{"line_number":54,"context_line":"_REAL_SHUTDOWN_ALL_EXECUTORS \u003d utils.shutdown_all_executors"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"class GenericUtilsTestCase(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"70d141ea_979364b7","line":54,"in_reply_to":"d4033305_4c910066","updated":"2026-07-30 19:27:56.000000000","message":"yeah, that\u0027s much better. Done","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"}],"nova/thread_pool_factory.py":[{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"7f6e81e51d7d99b73cdba0bfd0af75d5834ca2f6","unresolved":true,"context_lines":[{"line_number":271,"context_line":"        if not executor:"},{"line_number":272,"context_line":"            max_workers \u003d ExecutorsPoolSize.get(executor_type)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"            with self._lock.read_lock():"},{"line_number":275,"context_line":"                executor \u003d self._all_executors.get(executor_type)"},{"line_number":276,"context_line":"                if not executor:"},{"line_number":277,"context_line":"                    executor \u003d self._new_executor("}],"source_content_type":"text/x-python","patch_set":6,"id":"a667177c_0cc36447","line":274,"updated":"2026-08-11 14:43:54.000000000","message":"Since we are performing a read operation here, do you think we might end up with duplicate executor types?","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0345f370a6dae9c9510006becebca8bb28cb1f09","unresolved":false,"context_lines":[{"line_number":271,"context_line":"        if not executor:"},{"line_number":272,"context_line":"            max_workers \u003d ExecutorsPoolSize.get(executor_type)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"            with self._lock.read_lock():"},{"line_number":275,"context_line":"                executor \u003d self._all_executors.get(executor_type)"},{"line_number":276,"context_line":"                if not executor:"},{"line_number":277,"context_line":"                    executor \u003d self._new_executor("}],"source_content_type":"text/x-python","patch_set":6,"id":"8d078420_eb07dc65","line":274,"in_reply_to":"0fefd097_6306681b","updated":"2026-08-11 18:06:27.000000000","message":"Done","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"dd90ba110515765651d129b59f0091b76d4274bd","unresolved":false,"context_lines":[{"line_number":271,"context_line":"        if not executor:"},{"line_number":272,"context_line":"            max_workers \u003d ExecutorsPoolSize.get(executor_type)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"            with self._lock.read_lock():"},{"line_number":275,"context_line":"                executor \u003d self._all_executors.get(executor_type)"},{"line_number":276,"context_line":"                if not executor:"},{"line_number":277,"context_line":"                    executor \u003d self._new_executor("}],"source_content_type":"text/x-python","patch_set":6,"id":"bf0a92a7_8846896d","line":274,"in_reply_to":"8d078420_eb07dc65","updated":"2026-08-12 11:19:08.000000000","message":"I would go with safety over performance here. Especially as if we do it right we only do one creation per type at most. Given we have a limited amount of type we will have a limited amount of synchronized creation and hence a limited performance impact due to synchronization","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e644d02698bb895efe662165e90ad3b449c43118","unresolved":true,"context_lines":[{"line_number":271,"context_line":"        if not executor:"},{"line_number":272,"context_line":"            max_workers \u003d ExecutorsPoolSize.get(executor_type)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"            with self._lock.read_lock():"},{"line_number":275,"context_line":"                executor \u003d self._all_executors.get(executor_type)"},{"line_number":276,"context_line":"                if not executor:"},{"line_number":277,"context_line":"                    executor \u003d self._new_executor("}],"source_content_type":"text/x-python","patch_set":6,"id":"92e1f9bc_8cc6ecb2","line":274,"in_reply_to":"a667177c_0cc36447","updated":"2026-08-11 16:58:28.000000000","message":"Yeah I think the following will create two executors of the same type and record only one of them in the central dict\nAssume you have t1, t2 threads both executing the following code:\n```\nexecutor \u003d get_executor(MY_TYPE)\nexecutor.submit(foo).result()\n```\nI think the following overlap is not prevented:\n1. t1 executes until L276 (with a read lock) and sees executor \u003d\u003d None\n2. t2 executes until L276 (with a read lock, two read locks are allowed) and also sees executor \u003d\u003d None\n3. t2 executes until L294 and creates a new executor E1, records it into the central dict, logs success, and returns E1\n4. t1 excutes untl L294, and creates another new executor E2 with the same type, puts it into the central dict overriding E1 in it, logs success, and returns E2\n\nAt this point both t1 and t2 has an executor instance E2 and E1 with the same type which breaks the invariant, also E1 is not recorded in the central dict any more so shutdown_all will not stop E1.\n\nI think we need a normal lock guarding at least the L275-L277 critical section","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"a1b675c5228d1ae25b30ba2571d055afc4a03515","unresolved":true,"context_lines":[{"line_number":271,"context_line":"        if not executor:"},{"line_number":272,"context_line":"            max_workers \u003d ExecutorsPoolSize.get(executor_type)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"            with self._lock.read_lock():"},{"line_number":275,"context_line":"                executor \u003d self._all_executors.get(executor_type)"},{"line_number":276,"context_line":"                if not executor:"},{"line_number":277,"context_line":"                    executor \u003d self._new_executor("}],"source_content_type":"text/x-python","patch_set":6,"id":"0fefd097_6306681b","line":274,"in_reply_to":"a667177c_0cc36447","updated":"2026-08-11 17:53:45.000000000","message":"yes, that is possible. I checked and the existing get has the same issue. per type executor creation are once per process so it is narrow window to have race, even we try to get the existing executor at L270 and then L275 once we are in this read lock but still it is possible to have two different executors per type.\n\nWe have two option here\n1. use write lock but that will make different type executors creation serial which is not good.\n2. per executor type lock. executor type is something we want to make serial. let me add that here.","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e644d02698bb895efe662165e90ad3b449c43118","unresolved":true,"context_lines":[{"line_number":281,"context_line":"                        \"The %s thread pool %s is initialized\","},{"line_number":282,"context_line":"                        executor_type.value, executor.name)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        if log_stats:"},{"line_number":285,"context_line":"            self._log_executor_stats(executor, executor_type)"},{"line_number":286,"context_line":"            if self._executor_is_full(executor):"},{"line_number":287,"context_line":"                LOG.warning("}],"source_content_type":"text/x-python","patch_set":6,"id":"63a02433_b271776a","line":284,"updated":"2026-08-11 16:58:28.000000000","message":"This feels like a new behavior. Why we need a flag about logging?","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"abbec74e53aeb3ac4617307e623466c6f7aab742","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                        \"The %s thread pool %s is initialized\","},{"line_number":282,"context_line":"                        executor_type.value, executor.name)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        if log_stats:"},{"line_number":285,"context_line":"            self._log_executor_stats(executor, executor_type)"},{"line_number":286,"context_line":"            if self._executor_is_full(executor):"},{"line_number":287,"context_line":"                LOG.warning("}],"source_content_type":"text/x-python","patch_set":6,"id":"8d220ef5_a98885d7","line":284,"in_reply_to":"4c0a7393_9a31aaf8","updated":"2026-08-11 19:37:44.000000000","message":"I moved it back to spawn_on as it was before.","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0345f370a6dae9c9510006becebca8bb28cb1f09","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                        \"The %s thread pool %s is initialized\","},{"line_number":282,"context_line":"                        executor_type.value, executor.name)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        if log_stats:"},{"line_number":285,"context_line":"            self._log_executor_stats(executor, executor_type)"},{"line_number":286,"context_line":"            if self._executor_is_full(executor):"},{"line_number":287,"context_line":"                LOG.warning("}],"source_content_type":"text/x-python","patch_set":6,"id":"4c0a7393_9a31aaf8","line":284,"in_reply_to":"63a02433_b271776a","updated":"2026-08-11 18:06:27.000000000","message":"I think we want to log only from spawn_on case? or it was always logged? because we do spawn things on executors. I can log here always","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e644d02698bb895efe662165e90ad3b449c43118","unresolved":true,"context_lines":[{"line_number":303,"context_line":"        \"\"\""},{"line_number":304,"context_line":"        # NOTE(gmaan): Hold the write lock so that no one can create the"},{"line_number":305,"context_line":"        # new executors when shutdown is in progress."},{"line_number":306,"context_line":"        with self._lock.write_lock():"},{"line_number":307,"context_line":"            executors \u003d list(self._all_executors.values())"},{"line_number":308,"context_line":"            self._all_executors.clear()"},{"line_number":309,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"f729471d_37640cf1","line":306,"updated":"2026-08-11 16:58:28.000000000","message":"this write lock is released at the end of shutdown_all so after that a thread outside of our tracked executors can use the factory to create a new executor. That is not what we want to allow as we are shutting down. I think we should set a flag (under the lock) that prevents any further get_executor() calls.","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"dd90ba110515765651d129b59f0091b76d4274bd","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        \"\"\""},{"line_number":304,"context_line":"        # NOTE(gmaan): Hold the write lock so that no one can create the"},{"line_number":305,"context_line":"        # new executors when shutdown is in progress."},{"line_number":306,"context_line":"        with self._lock.write_lock():"},{"line_number":307,"context_line":"            executors \u003d list(self._all_executors.values())"},{"line_number":308,"context_line":"            self._all_executors.clear()"},{"line_number":309,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9397b065_75ec2988","line":306,"in_reply_to":"5d72c5d5_b366044e","updated":"2026-08-12 11:19:08.000000000","message":"I think this is a problem of fork and we should have a fork level solutions for it. \n1) lets start removing the fork form the system and use spawn. This has a blocker in oslo.service due to some pickling issue preventing us to use spawn. https://bugs.launchpad.net/nova/+bug/2151537\n2) embrace fork and then start doing a proper forking by setting up the parent / child process properly during fork via callbacks in os.register_at_fork\n\nNow I understand if you don\u0027t want to do that work here in the refactor. So I\u0027m fine having a documented race condition or having two separate methods. But we need to start allocating time to work on either 1) or 2). I prefer 1) as fork is overall problematic when mixed with threading.","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"79b3dbc98d3a026f8c961dee2905fe4b4543b2da","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        \"\"\""},{"line_number":304,"context_line":"        # NOTE(gmaan): Hold the write lock so that no one can create the"},{"line_number":305,"context_line":"        # new executors when shutdown is in progress."},{"line_number":306,"context_line":"        with self._lock.write_lock():"},{"line_number":307,"context_line":"            executors \u003d list(self._all_executors.values())"},{"line_number":308,"context_line":"            self._all_executors.clear()"},{"line_number":309,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"5d72c5d5_b366044e","line":306,"in_reply_to":"d3c5e5a2_d3131fee","updated":"2026-08-12 04:06:43.000000000","message":"I added flag but that created the problem because scheduler and conductor service called the shutdown_all() at the service start which set this flag to true. Now fork() copied the same flag with True value to each worker and never allowed to create any executors.\n\nhttps://zuul.opendev.org/t/openstack/build/751d64e73db54f63ae9bcd52ae9872d5/log/controller/logs/screen-n-sch.txt#2815\n\n    nova-scheduler[74046]: ERROR oslo_messaging.rpc.server RuntimeError: Cannot create the cell_worker thread pool executor because shutdown has been started.\n\nI will create a new reset_all mehtod which will just shutdown executors and clear the self._all_executors but will never set any state/flag. And this shutdown_All will be kept only for the graceful shutdown purpose.","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0345f370a6dae9c9510006becebca8bb28cb1f09","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        \"\"\""},{"line_number":304,"context_line":"        # NOTE(gmaan): Hold the write lock so that no one can create the"},{"line_number":305,"context_line":"        # new executors when shutdown is in progress."},{"line_number":306,"context_line":"        with self._lock.write_lock():"},{"line_number":307,"context_line":"            executors \u003d list(self._all_executors.values())"},{"line_number":308,"context_line":"            self._all_executors.clear()"},{"line_number":309,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"d3c5e5a2_d3131fee","line":306,"in_reply_to":"f729471d_37640cf1","updated":"2026-08-11 18:06:27.000000000","message":"shutdown_all is the last things in service stop but oslo.service stop still take time after shutdown_all. let me add a flag or event to prevent it.","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e644d02698bb895efe662165e90ad3b449c43118","unresolved":true,"context_lines":[{"line_number":308,"context_line":"            self._all_executors.clear()"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"            for executor in executors:"},{"line_number":311,"context_line":"                name \u003d getattr(executor, \"name\", \"unknown\")"},{"line_number":312,"context_line":"                LOG.info(\"The thread pool %s is shutting down\", name)"},{"line_number":313,"context_line":"                # NOTE: wait\u003dFalse. The manager\u0027s own"},{"line_number":314,"context_line":"                # graceful_shutdown(timeout) already spent its timeout"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a0ea5df_a80486a7","line":311,"updated":"2026-08-11 16:58:28.000000000","message":"I think by having all executors pre-declared we can make sure all of them has a name so we don\u0027t need the fallback to \"unknown\" as that should not happen and actually should be a software error to fix.","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0345f370a6dae9c9510006becebca8bb28cb1f09","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            self._all_executors.clear()"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"            for executor in executors:"},{"line_number":311,"context_line":"                name \u003d getattr(executor, \"name\", \"unknown\")"},{"line_number":312,"context_line":"                LOG.info(\"The thread pool %s is shutting down\", name)"},{"line_number":313,"context_line":"                # NOTE: wait\u003dFalse. The manager\u0027s own"},{"line_number":314,"context_line":"                # graceful_shutdown(timeout) already spent its timeout"}],"source_content_type":"text/x-python","patch_set":6,"id":"6984718d_944f9ed9","line":311,"in_reply_to":"5a0ea5df_a80486a7","updated":"2026-08-11 18:06:27.000000000","message":"Done","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e644d02698bb895efe662165e90ad3b449c43118","unresolved":true,"context_lines":[{"line_number":349,"context_line":"def spawn_on("},{"line_number":350,"context_line":"    func: ty.Callable[..., ty.Any],"},{"line_number":351,"context_line":"    *args: ty.Any,"},{"line_number":352,"context_line":"    executor_type\u003dExecutorType.DEFAULT,"},{"line_number":353,"context_line":"    **kwargs: ty.Any,"},{"line_number":354,"context_line":") -\u003e futurist.Future:"},{"line_number":355,"context_line":"    \"\"\"Passthrough method to run func on a thread in the named executor."}],"source_content_type":"text/x-python","patch_set":6,"id":"abbb55ff_2b3b0fcb","line":352,"updated":"2026-08-11 16:58:28.000000000","message":"that is strangely placed. I would rather keep *args and **kwargs next to each other at the end of the signature.\n\nAlso I would not default the type argument here. If somebody wants the default exucutor without typing it out then it can simply call spawn() instead of spawn_on() and then let spawn use the default when calling spawn_on","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"dd90ba110515765651d129b59f0091b76d4274bd","unresolved":false,"context_lines":[{"line_number":349,"context_line":"def spawn_on("},{"line_number":350,"context_line":"    func: ty.Callable[..., ty.Any],"},{"line_number":351,"context_line":"    *args: ty.Any,"},{"line_number":352,"context_line":"    executor_type\u003dExecutorType.DEFAULT,"},{"line_number":353,"context_line":"    **kwargs: ty.Any,"},{"line_number":354,"context_line":") -\u003e futurist.Future:"},{"line_number":355,"context_line":"    \"\"\"Passthrough method to run func on a thread in the named executor."}],"source_content_type":"text/x-python","patch_set":6,"id":"058d8515_adb9c817","line":352,"in_reply_to":"9aabf74a_ee5e3475","updated":"2026-08-12 11:19:08.000000000","message":"yeah if we have spawn that does the defaulting then spawn_on can require the extra parameter.","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0345f370a6dae9c9510006becebca8bb28cb1f09","unresolved":false,"context_lines":[{"line_number":349,"context_line":"def spawn_on("},{"line_number":350,"context_line":"    func: ty.Callable[..., ty.Any],"},{"line_number":351,"context_line":"    *args: ty.Any,"},{"line_number":352,"context_line":"    executor_type\u003dExecutorType.DEFAULT,"},{"line_number":353,"context_line":"    **kwargs: ty.Any,"},{"line_number":354,"context_line":") -\u003e futurist.Future:"},{"line_number":355,"context_line":"    \"\"\"Passthrough method to run func on a thread in the named executor."}],"source_content_type":"text/x-python","patch_set":6,"id":"9aabf74a_ee5e3475","line":352,"in_reply_to":"abbb55ff_2b3b0fcb","updated":"2026-08-11 18:06:27.000000000","message":"I had that before but changed it to avoid typing default from users but I agree to be explicit here at least in spawn_on","commit_id":"b1b8b6062a695339e37802a1b6ef2aa5771d2ec4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        `executor_type`, lazily creating it the first time it is requested,"},{"line_number":229,"context_line":"        sized via ExecutorsPoolSize."},{"line_number":230,"context_line":"        \"\"\""},{"line_number":231,"context_line":"        executor \u003d self._all_executors.get(executor_type)"},{"line_number":232,"context_line":"        if not executor:"},{"line_number":233,"context_line":"            max_workers \u003d ExecutorsPoolSize.get(executor_type)"},{"line_number":234,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"13e39ff9_335a0c2a","line":231,"updated":"2026-08-12 12:48:52.000000000","message":"This is OK to be returned even if we are during shutdown as the executor itself has an atomic way to handle shutdown as well.","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":true,"context_lines":[{"line_number":246,"context_line":"                        self._all_executors[executor_type] \u003d executor"},{"line_number":247,"context_line":"                        LOG.info("},{"line_number":248,"context_line":"                            \"The %s thread pool %s is initialized\","},{"line_number":249,"context_line":"                            executor_type.value, executor.name)"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        return executor"},{"line_number":252,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"0f4554fd_8780afaf","line":249,"updated":"2026-08-12 12:48:52.000000000","message":"OK this is probably thread safe now. But I feel we are over-complicating it to avoid some non-existent performance impact of two different type of executors created in parallel.\n\nThis could be as simple as:\n```\ndef __init__():\n  ...\n  self._shutdown_lock \u003d threading.Lock()\n  \ndef get_executor():\n    with self._shutdown_lock:\n        if self._shutdown:\n            raise RuntimeError()\n\n        executor \u003d self._all_executors.get(executor_type)\n        if not executor:\n            executor \u003d self._new_executor(...)\n            \n        return executor\n```\nIt is less performant for sure but I think we don\u0027t hit that in production like ever. But in return the code is a lot simpler this way.","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d6aeac1298fea5123622b1223e0a1c9c98280614","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                        self._all_executors[executor_type] \u003d executor"},{"line_number":247,"context_line":"                        LOG.info("},{"line_number":248,"context_line":"                            \"The %s thread pool %s is initialized\","},{"line_number":249,"context_line":"                            executor_type.value, executor.name)"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        return executor"},{"line_number":252,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"02935e2c_152db3f6","line":249,"in_reply_to":"00910bd0_bdc6dc5f","updated":"2026-08-14 18:05:34.000000000","message":"yeah it will be when executors are created for the first time and when multiple operation like build instance, live migration etc are running at same time which is also make it less common.\n\nI think I am fine to make it simple as two votes are for simple lock and not to worry about performance in this case (even I do not have any proof/performance data that this parallelism will give us any performance impact.)\n\ndone","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":true,"context_lines":[{"line_number":246,"context_line":"                        self._all_executors[executor_type] \u003d executor"},{"line_number":247,"context_line":"                        LOG.info("},{"line_number":248,"context_line":"                            \"The %s thread pool %s is initialized\","},{"line_number":249,"context_line":"                            executor_type.value, executor.name)"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        return executor"},{"line_number":252,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"a0384d75_c727f3c4","line":249,"in_reply_to":"0f4554fd_8780afaf","updated":"2026-08-13 04:02:10.000000000","message":"I agree this is little complex but i think that is needed now. Previously all the executors were created serially during compute manager __init__ so we are safe but now with this pool facotry, those are created when actual operations are called so there are chances that multiple type of executors creation can happen in parallel. build instance, snapshot, live migration can happen at same time (and power syncup also).\n\nI am not sure how much time new executor creation takes and making executors creation mutual exclusive will not have any performance impact. But there is logic change now so i thought of making them in parallel.","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"42d4dadb77f23144fc0983b75218684bc873c720","unresolved":true,"context_lines":[{"line_number":246,"context_line":"                        self._all_executors[executor_type] \u003d executor"},{"line_number":247,"context_line":"                        LOG.info("},{"line_number":248,"context_line":"                            \"The %s thread pool %s is initialized\","},{"line_number":249,"context_line":"                            executor_type.value, executor.name)"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        return executor"},{"line_number":252,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"00910bd0_bdc6dc5f","line":249,"in_reply_to":"a0384d75_c727f3c4","updated":"2026-08-14 17:33:06.000000000","message":"I have to say, I really don\u0027t like this complexity either. I don\u0027t like that we\u0027re taking two locks to do it (even though one is a read_lock) and I don\u0027t think we need to be super concerned about the performance. For the most part, holding the lock to get the executor will be extremely quick, right? The only time it won\u0027t is if/when we are initializing the executor for the first time. Even so, with eventlet we wouldn\u0027t have near the scheduling yields that we have (potentially) here so I doubt it\u0027s worse than the previous performance where multiple things got spawned at the same time. We could also make it iterate through all the executor types and pre-spawn then at startup time (if it were going to be a problem).\n\nI\u0027d prefer to go with a much simpler single-lock, fewer-conditions, less-nesting approach now and solve the performance problem (if it exists) later. That\u0027s just MHO.","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":true,"context_lines":[{"line_number":288,"context_line":""},{"line_number":289,"context_line":"        This shuts down every executor and clears self._all_executors but"},{"line_number":290,"context_line":"        does not set any flag/state on this factory. That is the only"},{"line_number":291,"context_line":"        difference from shutdown_all(). It is used before the main process"},{"line_number":292,"context_line":"        uses os.fork() to create multiple workers. Any executor created in"},{"line_number":293,"context_line":"        the parent or this factory state must not be inherited by the"},{"line_number":294,"context_line":"        forked worker. Use this when you need to clear the executors but"}],"source_content_type":"text/x-python","patch_set":8,"id":"05c487fa_fa06e7ff","line":291,"range":{"start_line":291,"start_character":8,"end_line":291,"end_character":40},"updated":"2026-08-12 12:48:52.000000000","message":"there is anther difference. This calls shutdown with wait\u003dTrue while shutdown_all uses wait\u003dFalse","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":288,"context_line":""},{"line_number":289,"context_line":"        This shuts down every executor and clears self._all_executors but"},{"line_number":290,"context_line":"        does not set any flag/state on this factory. That is the only"},{"line_number":291,"context_line":"        difference from shutdown_all(). It is used before the main process"},{"line_number":292,"context_line":"        uses os.fork() to create multiple workers. Any executor created in"},{"line_number":293,"context_line":"        the parent or this factory state must not be inherited by the"},{"line_number":294,"context_line":"        forked worker. Use this when you need to clear the executors but"}],"source_content_type":"text/x-python","patch_set":8,"id":"2d81e872_393fece0","line":291,"range":{"start_line":291,"start_character":8,"end_line":291,"end_character":40},"in_reply_to":"05c487fa_fa06e7ff","updated":"2026-08-13 04:02:10.000000000","message":"Done","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":true,"context_lines":[{"line_number":291,"context_line":"        difference from shutdown_all(). It is used before the main process"},{"line_number":292,"context_line":"        uses os.fork() to create multiple workers. Any executor created in"},{"line_number":293,"context_line":"        the parent or this factory state must not be inherited by the"},{"line_number":294,"context_line":"        forked worker. Use this when you need to clear the executors but"},{"line_number":295,"context_line":"        do not want to mark this thread pool factory as shutdown."},{"line_number":296,"context_line":"        \"\"\""},{"line_number":297,"context_line":"        self._teardown_all(mark_shutdown\u003dFalse, wait\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":8,"id":"befc16fd_8f2a473b","line":294,"updated":"2026-08-12 12:48:52.000000000","message":"I would add a TODO here to get rid of all this by moving away from fork and using spawn","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":291,"context_line":"        difference from shutdown_all(). It is used before the main process"},{"line_number":292,"context_line":"        uses os.fork() to create multiple workers. Any executor created in"},{"line_number":293,"context_line":"        the parent or this factory state must not be inherited by the"},{"line_number":294,"context_line":"        forked worker. Use this when you need to clear the executors but"},{"line_number":295,"context_line":"        do not want to mark this thread pool factory as shutdown."},{"line_number":296,"context_line":"        \"\"\""},{"line_number":297,"context_line":"        self._teardown_all(mark_shutdown\u003dFalse, wait\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":8,"id":"0dd56732_eac6f5ce","line":294,"in_reply_to":"befc16fd_8f2a473b","updated":"2026-08-13 04:02:10.000000000","message":"Done","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a852b447f3ebc3e3ddfedb3b59141efa6fc4c92c","unresolved":true,"context_lines":[{"line_number":403,"context_line":"            \"will be queued. If this happens repeatedly then the \""},{"line_number":404,"context_line":"            \"size of the pool is too small for the load or there \""},{"line_number":405,"context_line":"            \"are stuck threads filling the pool.\","},{"line_number":406,"context_line":"            executor_type.value)"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"    _context \u003d common_context.get_current()"},{"line_number":409,"context_line":"    profiler_info \u003d _serialize_profile_info()"}],"source_content_type":"text/x-python","patch_set":8,"id":"85819488_e153834b","line":406,"updated":"2026-08-12 12:48:52.000000000","message":"In the baseline this logged the function submitted as well, please restore it as that is useful infromation","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"258da2b0dde12e3c36c686d988cb207aef065b93","unresolved":false,"context_lines":[{"line_number":403,"context_line":"            \"will be queued. If this happens repeatedly then the \""},{"line_number":404,"context_line":"            \"size of the pool is too small for the load or there \""},{"line_number":405,"context_line":"            \"are stuck threads filling the pool.\","},{"line_number":406,"context_line":"            executor_type.value)"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"    _context \u003d common_context.get_current()"},{"line_number":409,"context_line":"    profiler_info \u003d _serialize_profile_info()"}],"source_content_type":"text/x-python","patch_set":8,"id":"4f13921d_a6a33e55","line":406,"in_reply_to":"85819488_e153834b","updated":"2026-08-13 04:02:10.000000000","message":"ah my bad, done","commit_id":"c1bfbd327bbc4bec89e030aa3404b54ddd04f235"}],"nova/utils.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f00d9dec6e94037e68ebb49cebd672d3fad6d290","unresolved":true,"context_lines":[{"line_number":91,"context_line":"# NOTE(gmaan): Every executor created via create_executor() is tracked here"},{"line_number":92,"context_line":"# so that shutdown_all_executors() can shut down all of them."},{"line_number":93,"context_line":"_ALL_EXECUTORS: list[Executor] \u003d []"},{"line_number":94,"context_line":"_ALL_EXECUTORS_LOCK \u003d threading.Lock()"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def cooperative_yield():"}],"source_content_type":"text/x-python","patch_set":2,"id":"23f68a57_21843d10","line":94,"updated":"2026-07-29 14:14:14.000000000","message":"I have an itch due to adding more and more globals to handle these executors. Also we have a bunch of duplicated code to create and destroy these executors, i.e. you had to copy your list handling for each executor. That feels like something we can unify / generalize.","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"268d7419161b169de4ab789bc92d44ae85abea15","unresolved":true,"context_lines":[{"line_number":91,"context_line":"# NOTE(gmaan): Every executor created via create_executor() is tracked here"},{"line_number":92,"context_line":"# so that shutdown_all_executors() can shut down all of them."},{"line_number":93,"context_line":"_ALL_EXECUTORS: list[Executor] \u003d []"},{"line_number":94,"context_line":"_ALL_EXECUTORS_LOCK \u003d threading.Lock()"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def cooperative_yield():"}],"source_content_type":"text/x-python","patch_set":2,"id":"cb8885e7_467181b1","line":94,"in_reply_to":"23f68a57_21843d10","updated":"2026-07-30 19:27:56.000000000","message":"yeah, I think that will be better for long term. I am even thinking to move all executors related things to new file or just a separate class managing them in utils only. so a few options and before I start modifying it, we can discuss which one is better:\n\n**option 1:**\n  Add a separate ExecutorRegistry in utils.py who will take care of create and shutdown executors with tracking of all created exectuors. existing get methods stay same and use this ExecutorRegistry for creation or shutdown of executors. \nThis is minimul changes to make executors tracking better but does not provide a unify way of maintaining executors.\n \n**option 2:**\n  A new executor class something like ThresadPoolExecutorManager in utils.py who will manage the complete lifecycle of all executors: create, shutdown, get for all type of executors. This will provide a better unify way to handle/maintain executors lifecycle. We can move spawn and spawn_on in this but if we do that then I prefer to do option3 itself.\n\n**option 3:**\n  option 2 + move the new ThresadPoolExecutorManager to a separate file thread_manager.py or some better name. And this will not just move the ThresadPoolExecutorManager to new file but also move spawn and spawn_on and related functions from utils to this new file. This will make utils.py less complex (currently it is lengthy file and mixed up with many things) and easy for long term maintenance.\n\noption3 might looks like a big change because we need to modify the usage of moved functions but easy change to do/review. This is my preference though.","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"49641e0463d8cd98d407a04d4253e73f687f782f","unresolved":false,"context_lines":[{"line_number":91,"context_line":"# NOTE(gmaan): Every executor created via create_executor() is tracked here"},{"line_number":92,"context_line":"# so that shutdown_all_executors() can shut down all of them."},{"line_number":93,"context_line":"_ALL_EXECUTORS: list[Executor] \u003d []"},{"line_number":94,"context_line":"_ALL_EXECUTORS_LOCK \u003d threading.Lock()"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def cooperative_yield():"}],"source_content_type":"text/x-python","patch_set":2,"id":"ebddade1_f7f44f58","line":94,"in_reply_to":"7ad6279a_810c1a0d","updated":"2026-08-08 19:09:36.000000000","message":"Done","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78fc96cd43c81e961be0fd076a681e2ac4ad104f","unresolved":true,"context_lines":[{"line_number":91,"context_line":"# NOTE(gmaan): Every executor created via create_executor() is tracked here"},{"line_number":92,"context_line":"# so that shutdown_all_executors() can shut down all of them."},{"line_number":93,"context_line":"_ALL_EXECUTORS: list[Executor] \u003d []"},{"line_number":94,"context_line":"_ALL_EXECUTORS_LOCK \u003d threading.Lock()"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def cooperative_yield():"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ad6279a_810c1a0d","line":94,"in_reply_to":"cb8885e7_467181b1","updated":"2026-08-06 09:30:06.000000000","message":"We discussed this yesterday on the eventlet sync call and agreed that option 3 looks good.","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f00d9dec6e94037e68ebb49cebd672d3fad6d290","unresolved":true,"context_lines":[{"line_number":109,"context_line":"    return eventlet"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"def destroy_default_executor():"},{"line_number":113,"context_line":"    \"\"\"Closes the executor and resets the global to None to allow forked worker"},{"line_number":114,"context_line":"    processes to properly init it."},{"line_number":115,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"092c174b_7f211469","line":112,"updated":"2026-07-29 14:14:14.000000000","message":"I think these individual destroy calls per executor can be dropped now and all places that uses them (basically the one before we forking the workers) can use the new shutdown_all_executor call.","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"268d7419161b169de4ab789bc92d44ae85abea15","unresolved":true,"context_lines":[{"line_number":109,"context_line":"    return eventlet"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"def destroy_default_executor():"},{"line_number":113,"context_line":"    \"\"\"Closes the executor and resets the global to None to allow forked worker"},{"line_number":114,"context_line":"    processes to properly init it."},{"line_number":115,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"2e9858a3_32a9af24","line":112,"in_reply_to":"092c174b_7f211469","updated":"2026-07-30 19:27:56.000000000","message":"yeah, we do not need these separate destroy and in start of service and stop (graceful shutdown), cleaning up all is what current use case is. we can provide a generic method to shutdown a single executors but I will say implement that when we need it.","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"49641e0463d8cd98d407a04d4253e73f687f782f","unresolved":false,"context_lines":[{"line_number":109,"context_line":"    return eventlet"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"def destroy_default_executor():"},{"line_number":113,"context_line":"    \"\"\"Closes the executor and resets the global to None to allow forked worker"},{"line_number":114,"context_line":"    processes to properly init it."},{"line_number":115,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"fd090aae_b5f1e3f1","line":112,"in_reply_to":"2e9858a3_32a9af24","updated":"2026-08-08 19:09:36.000000000","message":"Done","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f00d9dec6e94037e68ebb49cebd672d3fad6d290","unresolved":true,"context_lines":[{"line_number":1724,"context_line":""},{"line_number":1725,"context_line":"    with _ALL_EXECUTORS_LOCK:"},{"line_number":1726,"context_line":"        executors \u003d list(_ALL_EXECUTORS)"},{"line_number":1727,"context_line":"        _ALL_EXECUTORS.clear()"},{"line_number":1728,"context_line":""},{"line_number":1729,"context_line":"    for executor in executors:"},{"line_number":1730,"context_line":"        name \u003d getattr(executor, \"name\", \"unknown\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"8b5b4337_e4e2cc0e","line":1727,"updated":"2026-07-29 14:14:14.000000000","message":"hm. We need to drop the references but this will not be thread safe as is. Nothing prevents another thread to just create a new executor after we cleaned the list as we are releasing the lock. I guess we need a global (bah) flag telling the world we are in a shutdown phase and prevent new executors being created while we are shutting down.","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"49641e0463d8cd98d407a04d4253e73f687f782f","unresolved":false,"context_lines":[{"line_number":1724,"context_line":""},{"line_number":1725,"context_line":"    with _ALL_EXECUTORS_LOCK:"},{"line_number":1726,"context_line":"        executors \u003d list(_ALL_EXECUTORS)"},{"line_number":1727,"context_line":"        _ALL_EXECUTORS.clear()"},{"line_number":1728,"context_line":""},{"line_number":1729,"context_line":"    for executor in executors:"},{"line_number":1730,"context_line":"        name \u003d getattr(executor, \"name\", \"unknown\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"d602c7f2_4d4151c0","line":1727,"in_reply_to":"5c948188_1a00f40e","updated":"2026-08-08 19:09:36.000000000","message":"Done","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"268d7419161b169de4ab789bc92d44ae85abea15","unresolved":true,"context_lines":[{"line_number":1724,"context_line":""},{"line_number":1725,"context_line":"    with _ALL_EXECUTORS_LOCK:"},{"line_number":1726,"context_line":"        executors \u003d list(_ALL_EXECUTORS)"},{"line_number":1727,"context_line":"        _ALL_EXECUTORS.clear()"},{"line_number":1728,"context_line":""},{"line_number":1729,"context_line":"    for executor in executors:"},{"line_number":1730,"context_line":"        name \u003d getattr(executor, \"name\", \"unknown\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"e2400545_08f0f876","line":1727,"in_reply_to":"8b5b4337_e4e2cc0e","updated":"2026-07-30 19:27:56.000000000","message":"I am thinking if anyone need to create executor if in shutdown phase? in graceful shutdown, no because this can be called after manager finsih with their in-progress tasks or timeout and let shutdown to finsih.\n\nwhen service start and fork happen, that time also it is safe as no one should have a active thread pool executor.\n\nI think there is no other use of this to be called (at least for now). let me make shutdown and create mutual exclusive,","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78fc96cd43c81e961be0fd076a681e2ac4ad104f","unresolved":true,"context_lines":[{"line_number":1724,"context_line":""},{"line_number":1725,"context_line":"    with _ALL_EXECUTORS_LOCK:"},{"line_number":1726,"context_line":"        executors \u003d list(_ALL_EXECUTORS)"},{"line_number":1727,"context_line":"        _ALL_EXECUTORS.clear()"},{"line_number":1728,"context_line":""},{"line_number":1729,"context_line":"    for executor in executors:"},{"line_number":1730,"context_line":"        name \u003d getattr(executor, \"name\", \"unknown\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"5c948188_1a00f40e","line":1727,"in_reply_to":"e2400545_08f0f876","updated":"2026-08-06 09:30:06.000000000","message":"Lets see how this will look like when we move it to a single manager / factory. But I would err on the side of being more protective than necessary and have a flag under a lock that prevents new stuff added during / after shutdown.","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f00d9dec6e94037e68ebb49cebd672d3fad6d290","unresolved":true,"context_lines":[{"line_number":1732,"context_line":"        # NOTE: wait\u003dFalse. The manager\u0027s own graceful_shutdown(timeout)"},{"line_number":1733,"context_line":"        # already spent its timeout budget waiting for in-progress tasks"},{"line_number":1734,"context_line":"        # to finish, so we must not block here again with no timeout."},{"line_number":1735,"context_line":"        executor.shutdown(wait\u003dFalse)"},{"line_number":1736,"context_line":"        LOG.info(\"The thread pool %s is closed\", name)"},{"line_number":1737,"context_line":""},{"line_number":1738,"context_line":"    # Reset the shared singletons so a fresh executor is transparently"}],"source_content_type":"text/x-python","patch_set":2,"id":"3d8dffc9_ec8e3da6","line":1735,"updated":"2026-07-29 14:14:14.000000000","message":"does the executor implementations we use allow the python interpreter to exit if there is still a task running in them?\n\nIn general those tasks might be important to wait for to prevent half done things during shutdown.","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"268d7419161b169de4ab789bc92d44ae85abea15","unresolved":true,"context_lines":[{"line_number":1732,"context_line":"        # NOTE: wait\u003dFalse. The manager\u0027s own graceful_shutdown(timeout)"},{"line_number":1733,"context_line":"        # already spent its timeout budget waiting for in-progress tasks"},{"line_number":1734,"context_line":"        # to finish, so we must not block here again with no timeout."},{"line_number":1735,"context_line":"        executor.shutdown(wait\u003dFalse)"},{"line_number":1736,"context_line":"        LOG.info(\"The thread pool %s is closed\", name)"},{"line_number":1737,"context_line":""},{"line_number":1738,"context_line":"    # Reset the shared singletons so a fresh executor is transparently"}],"source_content_type":"text/x-python","patch_set":2,"id":"cac5d367_e550e8ca","line":1735,"in_reply_to":"3d8dffc9_ec8e3da6","updated":"2026-07-30 19:27:56.000000000","message":"yes, it does. futurist thread pool executors create daemon thread by default and they will let main process to exit even things are running. \n\nfrom graceful shutdown perspective, we will give chance to thread to finish the already picked up/running tasks from queue but if that takes the longer time than shutdown timeout then those will be interrupted and main process will exit immediately. \n- https://github.com/openstack/futurist/blob/135c68d858f96f628f73499473ccaaad7f83b15c/futurist/_thread.py#L67","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78fc96cd43c81e961be0fd076a681e2ac4ad104f","unresolved":false,"context_lines":[{"line_number":1732,"context_line":"        # NOTE: wait\u003dFalse. The manager\u0027s own graceful_shutdown(timeout)"},{"line_number":1733,"context_line":"        # already spent its timeout budget waiting for in-progress tasks"},{"line_number":1734,"context_line":"        # to finish, so we must not block here again with no timeout."},{"line_number":1735,"context_line":"        executor.shutdown(wait\u003dFalse)"},{"line_number":1736,"context_line":"        LOG.info(\"The thread pool %s is closed\", name)"},{"line_number":1737,"context_line":""},{"line_number":1738,"context_line":"    # Reset the shared singletons so a fresh executor is transparently"}],"source_content_type":"text/x-python","patch_set":2,"id":"2493cefd_ffe777d4","line":1735,"in_reply_to":"cac5d367_e550e8ca","updated":"2026-08-06 09:30:06.000000000","message":"OK thanks.","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f00d9dec6e94037e68ebb49cebd672d3fad6d290","unresolved":true,"context_lines":[{"line_number":1737,"context_line":""},{"line_number":1738,"context_line":"    # Reset the shared singletons so a fresh executor is transparently"},{"line_number":1739,"context_line":"    # created if one of them is requested again (e.g. by a periodic task"},{"line_number":1740,"context_line":"    # racing with shutdown)."},{"line_number":1741,"context_line":"    DEFAULT_EXECUTOR \u003d None"},{"line_number":1742,"context_line":"    SCATTER_GATHER_EXECUTOR \u003d None"},{"line_number":1743,"context_line":"    CACHE_IMAGES_EXECUTOR \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"a05d58aa_4a9d28dd","line":1740,"updated":"2026-07-29 14:14:14.000000000","message":"I agree we need to reset these due to forking happens form the main process to the worker processes and such fork cannot carry over an initialized executor. \n\nBut I have bad feelings about such race during shutdown.","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"49641e0463d8cd98d407a04d4253e73f687f782f","unresolved":false,"context_lines":[{"line_number":1737,"context_line":""},{"line_number":1738,"context_line":"    # Reset the shared singletons so a fresh executor is transparently"},{"line_number":1739,"context_line":"    # created if one of them is requested again (e.g. by a periodic task"},{"line_number":1740,"context_line":"    # racing with shutdown)."},{"line_number":1741,"context_line":"    DEFAULT_EXECUTOR \u003d None"},{"line_number":1742,"context_line":"    SCATTER_GATHER_EXECUTOR \u003d None"},{"line_number":1743,"context_line":"    CACHE_IMAGES_EXECUTOR \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"53c2ab44_d58dff59","line":1740,"in_reply_to":"83986121_47550ba4","updated":"2026-08-08 19:09:36.000000000","message":"Done","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"268d7419161b169de4ab789bc92d44ae85abea15","unresolved":true,"context_lines":[{"line_number":1737,"context_line":""},{"line_number":1738,"context_line":"    # Reset the shared singletons so a fresh executor is transparently"},{"line_number":1739,"context_line":"    # created if one of them is requested again (e.g. by a periodic task"},{"line_number":1740,"context_line":"    # racing with shutdown)."},{"line_number":1741,"context_line":"    DEFAULT_EXECUTOR \u003d None"},{"line_number":1742,"context_line":"    SCATTER_GATHER_EXECUTOR \u003d None"},{"line_number":1743,"context_line":"    CACHE_IMAGES_EXECUTOR \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"83986121_47550ba4","line":1740,"in_reply_to":"a05d58aa_4a9d28dd","updated":"2026-07-30 19:27:56.000000000","message":"yeah race is possible, I will handle it by making create and shutdown mutual exclusive so that we do not allow any new executors to be created and set these global during shutdown.","commit_id":"064d1f5dfac8ec2b5906115262c92c182748fa0f"}]}