)]}'
{"/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"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a0fb3f7bacf0a405ba065dade260e2afa19daa4f","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":"75a41c30_d130a4f0","line":23,"in_reply_to":"33a1a8c7_2067418e","updated":"2026-08-17 09:06:01.000000000","message":"I reviewed it and I\u0027m OK with this change.\n\nAs this codepath is exercised in almost every functional test and in every tempest job. I feel like we are not worse than before about quality and definitely better about maintainability. So it is net positive. Let\u0027s go for it. I will be around to catch any falling pieces if needed.","commit_id":"7c7c68136de2f63de76579fbc6c37bb3ea68333a"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3ab0f6356455d5165d4cb1876ba98ef1bf0d32c9","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":"6c55e5da_68ff1827","line":23,"in_reply_to":"75a41c30_d130a4f0","updated":"2026-08-18 19:07:22.000000000","message":"let me remove this false line now as this no doubt is a big change :)","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"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ac86c492797d6b15de89abe195bcad9c59b458a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"ad399a20_a5e2a953","updated":"2026-08-18 13:46:50.000000000","message":"As noted, I really would prefer for `nova.thread_pool_factory` to be more of a generic tool and push the config elements and initialization (ahead of time at service start) into the places that use it. Don\u0027t change this now, I\u0027m just saying I don\u0027t like how this is structured. If it\u0027s just me then leave it, and if it\u0027s not, maybe refactor later.\n\nOtherwise I think this is okay.. I\u0027m using the coverage report to help with the tests and I\u0027m seeing that we fail to cover a couple of things it seems - detailed inline.","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3ab0f6356455d5165d4cb1876ba98ef1bf0d32c9","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":11,"id":"d849fb61_bb0a1817","in_reply_to":"ad399a20_a5e2a953","updated":"2026-08-18 19:07:22.000000000","message":"sure, I will be open to refactor those bits later, especially \u0027initialization at service start\u0027 is a good idea. It will keep service specific executors creation and its size calculation to that service start. pre-creation during service start can save execution time compare to the on-demand creation when the operation requested for first time but it will consume the memory in advance. Or maybe that is correct way to tell how much memory this process will eventually take when operations are performed.","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"013e658701ed8c26b2bdc141c5149fca8e452834","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"4b325f6f_b7e9bfc9","updated":"2026-08-19 14:14:44.000000000","message":"Test coverage looks better now, thanks.","commit_id":"0842edaa998d12e77d622dae5aedfb5e0024c16a"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3fbdbb6bfe4b933573842712fa7c90f74fe481de","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"ae10bb4e_a0ba1fc6","updated":"2026-08-20 14:51:13.000000000","message":"recheck another ceph job failure which seems temporary as i can see it passing in latest run https://zuul.opendev.org/t/openstack/builds?job_name\u003dnova-ceph-multistore\u0026skip\u003d0","commit_id":"0842edaa998d12e77d622dae5aedfb5e0024c16a"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"f78fa33defbbbabbb0caee442a63478fcf51b641","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"ff12dfbf_c1a9be8b","updated":"2026-08-20 04:11:11.000000000","message":"recheck ceph job unrelated failure","commit_id":"0842edaa998d12e77d622dae5aedfb5e0024c16a"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"5c8a517cc3d69269be394c839e1a2ade1bbe98b8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"acd7b75f_2b082e63","updated":"2026-08-19 19:15:47.000000000","message":"recheck seems like notificaiton functioanl tests failing in threading mode, not sure if that is related to this. will track that separatly.\n\nTraceback (most recent call last):\n  File \"/home/zuul/src/opendev.org/openstack/nova/nova/test.py\", line 689, in assertJsonEqual\n    inner(expected, observed)\n    ~~~~~^^^^^^^^^^^^^^^^^^^^\n  File \"/home/zuul/src/opendev.org/openstack/nova/nova/test.py\", line 664, in inner\n    inner(expected[key], observed[key], path + \u0027.%s\u0027 % key)\n    ~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/home/zuul/src/opendev.org/openstack/nova/nova/test.py\", line 686, in inner\n    self.assertEqual(expected, observed, \u0027path: %s\u0027 % path)\n    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/home/zuul/src/opendev.org/openstack/nova/.tox/functional-py313-threading/lib/python3.13/site-packages/testtools/testcase.py\", line 513, in assertEqual\n    self.assertThat(observed, matcher, message)\n    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/home/zuul/src/opendev.org/openstack/nova/.tox/functional-py313-threading/lib/python3.13/site-packages/testtools/testcase.py\", line 704, in assertThat\n    raise mismatch_error\ntesttools.matchers._impl.MismatchError: !\u003d:\nreference \u003d \u0027instance.live_migration_force_complete.end\u0027\nactual    \u003d \u0027instance.live_migration_post.start\u0027\n: path: root.event_type","commit_id":"0842edaa998d12e77d622dae5aedfb5e0024c16a"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"ec5732c54f7c1c4356d868d0746c03193aa822b3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"79678a5d_e2556cb7","updated":"2026-08-20 17:10:44.000000000","message":"recheck this time alt-configuration job but failure are schema failure which should not happen? I will debug those in parallel but that is not related to this change so will rehecking it\n\n\nTraceback (most recent call last):\n  File \"/opt/stack/tempest/tempest/lib/common/rest_client.py\", line 1113, in validate_response\n    jsonschema.validate(body, body_schema,\n  File \"/opt/stack/tempest/.tox/tempest/lib/python3.11/site-packages/jsonschema/validators.py\", line 1332, in validate\n    raise error\njsonschema.exceptions.ValidationError: \u0027instance_uuid\u0027 is a required property\n\nFailed validating \u0027required\u0027 in schema[\u0027properties\u0027][\u0027migrations\u0027][\u0027items\u0027]:\n    {\u0027type\u0027: \u0027object\u0027,\n     \u0027properties\u0027: {\u0027id\u0027: {\u0027type\u0027: \u0027integer\u0027},\n                    \u0027status\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027server_uuid\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027source_node\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027source_compute\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027dest_node\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027dest_compute\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027dest_host\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027disk_processed_bytes\u0027: {\u0027type\u0027: [\u0027integer\u0027, \u0027null\u0027]},\n                    \u0027disk_remaining_bytes\u0027: {\u0027type\u0027: [\u0027integer\u0027, \u0027null\u0027]},\n                    \u0027disk_total_bytes\u0027: {\u0027type\u0027: [\u0027integer\u0027, \u0027null\u0027]},\n                    \u0027memory_processed_bytes\u0027: {\u0027type\u0027: [\u0027integer\u0027, \u0027null\u0027]},\n                    \u0027memory_remaining_bytes\u0027: {\u0027type\u0027: [\u0027integer\u0027, \u0027null\u0027]},\n                    \u0027memory_total_bytes\u0027: {\u0027type\u0027: [\u0027integer\u0027, \u0027null\u0027]},\n                    \u0027created_at\u0027: {\u0027type\u0027: \u0027string\u0027,\n                                   \u0027format\u0027: \u0027iso8601-date-time\u0027},\n                    \u0027updated_at\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027],\n                                   \u0027format\u0027: \u0027iso8601-date-time\u0027},\n                    \u0027uuid\u0027: {\u0027type\u0027: \u0027string\u0027, \u0027format\u0027: \u0027uuid\u0027},\n                    \u0027user_id\u0027: {\u0027type\u0027: \u0027string\u0027},\n                    \u0027project_id\u0027: {\u0027type\u0027: \u0027string\u0027}},\n     \u0027additionalProperties\u0027: False,\n     \u0027required\u0027: [\u0027id\u0027,\n                  \u0027status\u0027,\n                  \u0027instance_uuid\u0027,\n                  \u0027source_node\u0027,\n                  \u0027source_compute\u0027,\n                  \u0027dest_node\u0027,\n                  \u0027dest_compute\u0027,\n                  \u0027dest_host\u0027,\n                  \u0027disk_processed_bytes\u0027,\n                  \u0027disk_remaining_bytes\u0027,\n                  \u0027disk_total_bytes\u0027,\n                  \u0027memory_processed_bytes\u0027,\n                  \u0027memory_remaining_bytes\u0027,\n                  \u0027memory_total_bytes\u0027,\n                  \u0027created_at\u0027,\n                  \u0027updated_at\u0027,\n                  \u0027uuid\u0027,\n                  \u0027user_id\u0027,\n                  \u0027project_id\u0027]}\n\nOn instance[\u0027migrations\u0027][0]:\n    {\u0027created_at\u0027: \u00272026-08-20T15:32:44.000000\u0027,\n     \u0027dest_compute\u0027: None,\n     \u0027dest_host\u0027: None,\n     \u0027dest_node\u0027: None,\n     \u0027disk_processed_bytes\u0027: None,\n     \u0027disk_remaining_bytes\u0027: None,\n     \u0027disk_total_bytes\u0027: None,\n     \u0027id\u0027: 3,\n     \u0027memory_processed_bytes\u0027: None,\n     \u0027memory_remaining_bytes\u0027: None,\n     \u0027memory_total_bytes\u0027: None,\n     \u0027server_uuid\u0027: \u0027fa2d6e7d-5bf8-4384-ad2e-69cca7e0d8f0\u0027,\n     \u0027source_compute\u0027: None,\n     \u0027source_node\u0027: None,\n     \u0027status\u0027: \u0027queued\u0027,\n     \u0027updated_at\u0027: \u00272026-08-20T15:32:47.000000\u0027,\n     \u0027uuid\u0027: \u00279f58cbac-139a-4a24-bed9-c5f22f2495f9\u0027,\n     \u0027user_id\u0027: \u0027c6ce45046cc048b1882ea5cbfe2d9e7a\u0027,\n     \u0027project_id\u0027: \u0027fd9be143b049408aba93433853f94d51\u0027}\n\nDuring handling of the above exception, another exception occurred:\n\nTraceback (most recent call last):\n  File \"/opt/stack/tempest/tempest/api/compute/admin/test_live_migration.py\", line 517, in test_live_migration_by_project_manager\n    self._initiate_live_migration())\n    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/opt/stack/tempest/tempest/api/compute/admin/test_live_migration.py\", line 478, in _initiate_live_migration\n    self.mgr_server_client.list_in_progress_live_migration(\n  File \"/opt/stack/tempest/tempest/lib/services/compute/servers_client.py\", line 568, in list_in_progress_live_migration\n    self.validate_response(schema.list_live_migrations, resp, body)\n  File \"/opt/stack/tempest/tempest/lib/common/rest_client.py\", line 1118, in validate_response\n    raise exceptions.InvalidHTTPResponseBody(msg)\ntempest.lib.exceptions.InvalidHTTPResponseBody: HTTP response body is invalid json or xml\nDetails: HTTP response body is invalid (\u0027instance_uuid\u0027 is a required property\n\nFailed validating \u0027required\u0027 in schema[\u0027properties\u0027][\u0027migrations\u0027][\u0027items\u0027]:\n    {\u0027type\u0027: \u0027object\u0027,\n     \u0027properties\u0027: {\u0027id\u0027: {\u0027type\u0027: \u0027integer\u0027},\n                    \u0027status\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027server_uuid\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027source_node\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027source_compute\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027dest_node\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027dest_compute\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027dest_host\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},\n                    \u0027disk_processed_bytes\u0027: {\u0027type\u0027: [\u0027integer\u0027, \u0027null\u0027]},\n                    \u0027disk_remaining_bytes\u0027: {\u0027type\u0027: [\u0027integer\u0027, \u0027null\u0027]},\n                    \u0027disk_total_bytes\u0027: {\u0027type\u0027: [\u0027integer\u0027, \u0027null\u0027]},\n                    \u0027memory_processed_bytes\u0027: {\u0027type\u0027: [\u0027integer\u0027, \u0027null\u0027]},\n                    \u0027memory_remaining_bytes\u0027: {\u0027type\u0027: [\u0027integer\u0027, \u0027null\u0027]},\n                    \u0027memory_total_bytes\u0027: {\u0027type\u0027: [\u0027integer\u0027, \u0027null\u0027]},\n                    \u0027created_at\u0027: {\u0027type\u0027: \u0027string\u0027,\n                                   \u0027format\u0027: \u0027iso8601-date-time\u0027},\n                    \u0027updated_at\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027],\n                                   \u0027format\u0027: \u0027iso8601-date-time\u0027},\n                    \u0027uuid\u0027: {\u0027type\u0027: \u0027string\u0027, \u0027format\u0027: \u0027uuid\u0027},\n                    \u0027user_id\u0027: {\u0027type\u0027: \u0027string\u0027},\n                    \u0027project_id\u0027: {\u0027type\u0027: \u0027string\u0027}},\n     \u0027additionalProperties\u0027: False,\n     \u0027required\u0027: [\u0027id\u0027,\n                  \u0027status\u0027,\n                  \u0027instance_uuid\u0027,\n                  \u0027source_node\u0027,\n                  \u0027source_compute\u0027,\n                  \u0027dest_node\u0027,\n                  \u0027dest_compute\u0027,\n                  \u0027dest_host\u0027,\n                  \u0027disk_processed_bytes\u0027,\n                  \u0027disk_remaining_bytes\u0027,\n                  \u0027disk_total_bytes\u0027,\n                  \u0027memory_processed_bytes\u0027,\n                  \u0027memory_remaining_bytes\u0027,\n                  \u0027memory_total_bytes\u0027,\n                  \u0027created_at\u0027,\n                  \u0027updated_at\u0027,\n                  \u0027uuid\u0027,\n                  \u0027user_id\u0027,\n                  \u0027project_id\u0027]}\n\nOn instance[\u0027migrations\u0027][0]:\n    {\u0027created_at\u0027: \u00272026-08-20T15:32:44.000000\u0027,\n     \u0027dest_compute\u0027: None,\n     \u0027dest_host\u0027: None,\n     \u0027dest_node\u0027: None,\n     \u0027disk_processed_bytes\u0027: None,\n     \u0027disk_remaining_bytes\u0027: None,\n     \u0027disk_total_bytes\u0027: None,\n     \u0027id\u0027: 3,\n     \u0027memory_processed_bytes\u0027: None,\n     \u0027memory_remaining_bytes\u0027: None,\n     \u0027memory_total_bytes\u0027: None,\n     \u0027server_uuid\u0027: \u0027fa2d6e7d-5bf8-4384-ad2e-69cca7e0d8f0\u0027,\n     \u0027source_compute\u0027: None,\n     \u0027source_node\u0027: None,\n     \u0027status\u0027: \u0027queued\u0027,\n     \u0027updated_at\u0027: \u00272026-08-20T15:32:47.000000\u0027,\n     \u0027uuid\u0027: \u00279f58cbac-139a-4a24-bed9-c5f22f2495f9\u0027,\n     \u0027user_id\u0027: \u0027c6ce45046cc048b1882ea5cbfe2d9e7a\u0027,\n     \u0027project_id\u0027: \u0027fd9be143b049408aba93433853f94d51\u0027})","commit_id":"0842edaa998d12e77d622dae5aedfb5e0024c16a"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"7b84c7d3b88c7d7adc0ae84f2cdf9f2dc14db74a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"e415cedc_a7cc55d6","in_reply_to":"79678a5d_e2556cb7","updated":"2026-08-20 18:06:40.000000000","message":"reported the bug, most probably it is tempest https://bugs.launchpad.net/nova/+bug/2164677","commit_id":"0842edaa998d12e77d622dae5aedfb5e0024c16a"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"61a4e91e1c3a2b7dac76eee4694d07589b952206","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"32f047c0_86ab5f83","in_reply_to":"e415cedc_a7cc55d6","updated":"2026-08-20 18:33:38.000000000","message":"fix proposed it was tempest schema bug introduced by me last year :) https://review.opendev.org/c/openstack/tempest/+/1001722","commit_id":"0842edaa998d12e77d622dae5aedfb5e0024c16a"}],"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"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d27f609f7dc4277c8adfc509a66f155d1c82efce","unresolved":true,"context_lines":[{"line_number":353,"context_line":"        thread_pool_factory.shutdown_all_executors()"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        self.assertEqual({}, thread_pool_factory.FACTORY._all_executors)"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"class ResetAllExecutorsTestCase(test.NoDBTestCase):"},{"line_number":359,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"41d2e08d_588ca268","line":356,"updated":"2026-08-18 15:04:47.000000000","message":"Yeah I think a test of `get_executor()` after shutdown is needed/missing here.","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3ab0f6356455d5165d4cb1876ba98ef1bf0d32c9","unresolved":false,"context_lines":[{"line_number":353,"context_line":"        thread_pool_factory.shutdown_all_executors()"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        self.assertEqual({}, thread_pool_factory.FACTORY._all_executors)"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"class ResetAllExecutorsTestCase(test.NoDBTestCase):"},{"line_number":359,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"0f71e450_ca5c896d","line":356,"in_reply_to":"41d2e08d_588ca268","updated":"2026-08-18 19:07:22.000000000","message":"Done","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"}],"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"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ac86c492797d6b15de89abe195bcad9c59b458a","unresolved":true,"context_lines":[{"line_number":114,"context_line":"    CACHE_IMAGES \u003d \"cache_images\""},{"line_number":115,"context_line":"    LONG_TASK \u003d \"long_task\""},{"line_number":116,"context_line":"    SYNC_POWER \u003d \"sync_power_state\""},{"line_number":117,"context_line":"    LIVE_MIGRATION \u003d \"live_migration\""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"class ExecutorsPoolSize:"}],"source_content_type":"text/x-python","patch_set":11,"id":"d7b2cac1_3b458d2d","line":117,"updated":"2026-08-18 13:46:50.000000000","message":"I think part of what I don\u0027t like is that these things live here. In this common module, you\u0027re mixing pieces of compute, conductor and scheduler. Code in this module can\u0027t pre-initialize these things because it doesn\u0027t know where it\u0027s running, which leads to the previous discussion about the complexity around spinning up a new one. The above config getters are for CONF options that may not even be set on scheduler or conductor and thus would return default values if called there, but actual values on compute.\n\nIt seems to me like this would be a lot cleaner if you implemented this factory in a way that was generic. Let the caller spin up a new factory, provided a name and size. Almost all the rest of it could be the same, and executors would be created at service init time, and only the ones that service planned to use.","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"27ca98e001ce472a8b80c023cdb44ecf3c5906f9","unresolved":true,"context_lines":[{"line_number":114,"context_line":"    CACHE_IMAGES \u003d \"cache_images\""},{"line_number":115,"context_line":"    LONG_TASK \u003d \"long_task\""},{"line_number":116,"context_line":"    SYNC_POWER \u003d \"sync_power_state\""},{"line_number":117,"context_line":"    LIVE_MIGRATION \u003d \"live_migration\""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"class ExecutorsPoolSize:"}],"source_content_type":"text/x-python","patch_set":11,"id":"93c35556_39b0cf4e","line":117,"in_reply_to":"0504ed64_392b59fe","updated":"2026-08-19 14:25:41.000000000","message":"OK. That is an interesting direction to move towards. I\u0027m happy to look at such a refactor and if time / prio allows I can try such refactor myself if needed.","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"013e658701ed8c26b2bdc141c5149fca8e452834","unresolved":true,"context_lines":[{"line_number":114,"context_line":"    CACHE_IMAGES \u003d \"cache_images\""},{"line_number":115,"context_line":"    LONG_TASK \u003d \"long_task\""},{"line_number":116,"context_line":"    SYNC_POWER \u003d \"sync_power_state\""},{"line_number":117,"context_line":"    LIVE_MIGRATION \u003d \"live_migration\""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"class ExecutorsPoolSize:"}],"source_content_type":"text/x-python","patch_set":11,"id":"0504ed64_392b59fe","line":117,"in_reply_to":"57765bef_b17ef80a","updated":"2026-08-19 14:14:44.000000000","message":"I think having the factory here, which tracks executors created by name stored here makes sense. I think having `thread_pool_factory.create_executor(TP_LIVE_MIGRATION, CONF.compute.live_migration_limit)` (note the scoping of what is remote and local there) called in compute service startup makes sense. The factory here can track all those things that are created, provide the single shutdown trigger, tracking, etc. I just don\u0027t think it makes sense to put all the definitions of the executors for any/every service here in the implementation of a generic executor tracking system.","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"72524c6d20606cafcf150455551a4e900d4bcd9b","unresolved":true,"context_lines":[{"line_number":114,"context_line":"    CACHE_IMAGES \u003d \"cache_images\""},{"line_number":115,"context_line":"    LONG_TASK \u003d \"long_task\""},{"line_number":116,"context_line":"    SYNC_POWER \u003d \"sync_power_state\""},{"line_number":117,"context_line":"    LIVE_MIGRATION \u003d \"live_migration\""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"class ExecutorsPoolSize:"}],"source_content_type":"text/x-python","patch_set":11,"id":"ce03ea70_fa96fac4","line":117,"in_reply_to":"93c35556_39b0cf4e","updated":"2026-08-19 17:51:42.000000000","message":"yeah, more of using this factory of what it has but move the  creation/initialization/size calculation etc on service start up. basically this facotry serves all thread pool executors needs but its services who needs to ask for creation/initialization of only things they need.\n\n@gibizer@gmail.com I did half of the refactoring in that direction yesterday but did not finish. I will propose something today if that align with what we discussed here","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aa8aae2a37757b05be586b555bf31b3a0e231782","unresolved":true,"context_lines":[{"line_number":114,"context_line":"    CACHE_IMAGES \u003d \"cache_images\""},{"line_number":115,"context_line":"    LONG_TASK \u003d \"long_task\""},{"line_number":116,"context_line":"    SYNC_POWER \u003d \"sync_power_state\""},{"line_number":117,"context_line":"    LIVE_MIGRATION \u003d \"live_migration\""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"class ExecutorsPoolSize:"}],"source_content_type":"text/x-python","patch_set":11,"id":"57765bef_b17ef80a","line":117,"in_reply_to":"c097412b_f1f2e8be","updated":"2026-08-19 07:51:43.000000000","message":"There are couple of forces:\n1. the need to have a single generic call that stops all executors the service created during shutdown (and before fork while we still using fork to create worker processes)\n2. the need to move service specific code close to the rest of the service code\n3. the need to have generic spawn_* functions that\n   * do all the generic wrapping, logging, etc logic for any new task creation\n   * allows a ~single point to hook into when needed (e.g. from test)\n4. do not repeat code that creates executors\n\nThis patch solves the 1 and 4, keeps 3, but goes against 2. If we can move closer to 2 without loosing much of the rest (or if we can remove some forces be deciding not to care about it) then I\u0027m open to refactor, later.\n\n\u003e It seems to me like this would be a lot cleaner if you implemented this factory in a way that was generic. Let the caller spin up a new factory, provided a name and size. Almost all the rest of it could be the same, and executors would be created at service init time, and only the ones that service planned to use.\n\nWould there be a single factory per service instance? (to keep having a common point to call for shutting down all the executors of the service)\n\nWould that factory held by a global here or we would move it somewhere closer to the service instance? If moved into the service instance can we still have global spawn_* helpers easily? (Would we pass around service_ref to get access to the executors to call spawn_* with? or would we start plugging this into the context maybe?)","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"375e9ee0eed01a8ef8e294956e4942cd6e0d9321","unresolved":true,"context_lines":[{"line_number":114,"context_line":"    CACHE_IMAGES \u003d \"cache_images\""},{"line_number":115,"context_line":"    LONG_TASK \u003d \"long_task\""},{"line_number":116,"context_line":"    SYNC_POWER \u003d \"sync_power_state\""},{"line_number":117,"context_line":"    LIVE_MIGRATION \u003d \"live_migration\""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"class ExecutorsPoolSize:"}],"source_content_type":"text/x-python","patch_set":11,"id":"b0561328_eabb9606","line":117,"in_reply_to":"ce03ea70_fa96fac4","updated":"2026-08-19 21:00:26.000000000","message":"I proposed something like this- https://review.opendev.org/c/openstack/nova/+/1001572","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3ab0f6356455d5165d4cb1876ba98ef1bf0d32c9","unresolved":true,"context_lines":[{"line_number":114,"context_line":"    CACHE_IMAGES \u003d \"cache_images\""},{"line_number":115,"context_line":"    LONG_TASK \u003d \"long_task\""},{"line_number":116,"context_line":"    SYNC_POWER \u003d \"sync_power_state\""},{"line_number":117,"context_line":"    LIVE_MIGRATION \u003d \"live_migration\""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"class ExecutorsPoolSize:"}],"source_content_type":"text/x-python","patch_set":11,"id":"c097412b_f1f2e8be","line":117,"in_reply_to":"d7b2cac1_3b458d2d","updated":"2026-08-18 19:07:22.000000000","message":"\u003e I think part of what I don\u0027t like is that these things live here. In this common module, you\u0027re mixing pieces of compute, conductor and scheduler. Code in this module can\u0027t pre-initialize these things because it doesn\u0027t know where it\u0027s running, which leads to the previous discussion about the complexity around spinning up a new one. The above config getters are for CONF options that may not even be set on scheduler or conductor and thus would return default values if called there, but actual values on compute.\n\nConfig getters will come into pic when executor is created which is when operation (need those executors) is performed for first time by service. FOr other services they will not called at all. For example, scheduler service will never get the CONF.max_concurrent_live_migrations because it will never create live migration executors. Basically code exist in single place for all services but services only execute the part what they require.\n\n\u003e \n\u003e It seems to me like this would be a lot cleaner if you implemented this factory in a way that was generic. Let the caller spin up a new factory, provided a name and size. Almost all the rest of it could be the same, and executors would be created at service init time, and only the ones that service planned to use.\n\nWell, I thought of making this factory a complete nova level which will manage \u0027all thread/executors and spawning things\u0027 code in single place irrespective of which service use what. And services use/get the things only what they require. So execution point of view, services does not prepare/execute anything which they do not require but yes from code perspective it is mixed.\n\nI am open to refactor, if we want to move service specific part to service side. Second, I see the value of executor creation (their size calculation etc) during service start (which makes service specific code to servivce side). Its worth to do when service is started instead if init, something during manager.pre_start_hook ?(https://github.com/openstack/nova/blob/master/nova/service.py#L238)","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ac86c492797d6b15de89abe195bcad9c59b458a","unresolved":true,"context_lines":[{"line_number":172,"context_line":"        ExecutorType.LONG_TASK: _long_task_pool_size,"},{"line_number":173,"context_line":"        ExecutorType.SYNC_POWER: _sync_power_pool_size,"},{"line_number":174,"context_line":"        ExecutorType.LIVE_MIGRATION: get_max_concurrent_live_migrations,"},{"line_number":175,"context_line":"    }"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @classmethod"},{"line_number":178,"context_line":"    def get(cls, executor_type):"}],"source_content_type":"text/x-python","patch_set":11,"id":"58fbe217_898dc356","line":175,"updated":"2026-08-18 13:46:50.000000000","message":"I personally don\u0027t think any of this should be in the common module but... why are some of the conf getters separate and some are `staticmethod`s in this class? I\u0027d assume maybe because they\u0027re called from outside this module, but it seems like they could just call `ExecutorPoolSize.get(LIVE_MIGRATION)` as well. Either way, I think that\u0027s probably an indication that it\u0027s weird to have those getters here, but maybe it\u0027s just me.","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3ab0f6356455d5165d4cb1876ba98ef1bf0d32c9","unresolved":true,"context_lines":[{"line_number":172,"context_line":"        ExecutorType.LONG_TASK: _long_task_pool_size,"},{"line_number":173,"context_line":"        ExecutorType.SYNC_POWER: _sync_power_pool_size,"},{"line_number":174,"context_line":"        ExecutorType.LIVE_MIGRATION: get_max_concurrent_live_migrations,"},{"line_number":175,"context_line":"    }"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @classmethod"},{"line_number":178,"context_line":"    def get(cls, executor_type):"}],"source_content_type":"text/x-python","patch_set":11,"id":"283c6c56_8207252a","line":175,"in_reply_to":"58fbe217_898dc356","updated":"2026-08-18 19:07:22.000000000","message":"yeah, some of them are needed by manager but for eventlet case where we use them for the semaphore concurrency size. But yes ExecutorPoolSize.get(LIVE_MIGRATION) should work there.","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ac86c492797d6b15de89abe195bcad9c59b458a","unresolved":true,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"        return executor"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def get_executor(self, executor_type):"},{"line_number":215,"context_line":"        \"\"\"Return the shared, process-wide singleton executor for"},{"line_number":216,"context_line":"        `executor_type`, lazily creating it the first time it is requested,"},{"line_number":217,"context_line":"        sized via ExecutorsPoolSize."}],"source_content_type":"text/x-python","patch_set":11,"id":"5b2ea2ee_8b7bc3e5","line":214,"range":{"start_line":214,"start_character":27,"end_line":214,"end_character":40},"updated":"2026-08-18 13:46:50.000000000","message":"In my vision above, this would just be a name (like it basically is now) that the callers would use and would provide the same singleton behavior. You could even make the Enum thing dynamic to make it easier to use a constant from other code once initialized at service startup.","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3ab0f6356455d5165d4cb1876ba98ef1bf0d32c9","unresolved":true,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"        return executor"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def get_executor(self, executor_type):"},{"line_number":215,"context_line":"        \"\"\"Return the shared, process-wide singleton executor for"},{"line_number":216,"context_line":"        `executor_type`, lazily creating it the first time it is requested,"},{"line_number":217,"context_line":"        sized via ExecutorsPoolSize."}],"source_content_type":"text/x-python","patch_set":11,"id":"f187056f_0bfc9291","line":214,"range":{"start_line":214,"start_character":27,"end_line":214,"end_character":40},"in_reply_to":"5b2ea2ee_8b7bc3e5","updated":"2026-08-18 19:07:22.000000000","message":"yeah, we can do that if we  move the initialization at service start, that will make it easy to know/restrict executors per service.","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ac86c492797d6b15de89abe195bcad9c59b458a","unresolved":true,"context_lines":[{"line_number":219,"context_line":"        executor \u003d self._all_executors.get(executor_type)"},{"line_number":220,"context_line":"        if not executor:"},{"line_number":221,"context_line":"            with self._shutdown_lock:"},{"line_number":222,"context_line":"                if self._shutdown:"},{"line_number":223,"context_line":"                    raise RuntimeError("},{"line_number":224,"context_line":"                        \"Cannot create the %s thread pool executor because \""},{"line_number":225,"context_line":"                        \"shutdown has been started.\" %"}],"source_content_type":"text/x-python","patch_set":11,"id":"bc058cb9_c330fafd","line":222,"updated":"2026-08-18 13:46:50.000000000","message":"Coverage report for unit and functional says we never this case?\n\nhttps://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_c10/openstack/c106ccd9fd5a4b69a7ae62b561abb98e/cover/z_8ee3d46129bdc436_thread_pool_factory_py.html","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3ab0f6356455d5165d4cb1876ba98ef1bf0d32c9","unresolved":false,"context_lines":[{"line_number":219,"context_line":"        executor \u003d self._all_executors.get(executor_type)"},{"line_number":220,"context_line":"        if not executor:"},{"line_number":221,"context_line":"            with self._shutdown_lock:"},{"line_number":222,"context_line":"                if self._shutdown:"},{"line_number":223,"context_line":"                    raise RuntimeError("},{"line_number":224,"context_line":"                        \"Cannot create the %s thread pool executor because \""},{"line_number":225,"context_line":"                        \"shutdown has been started.\" %"}],"source_content_type":"text/x-python","patch_set":11,"id":"75f1c4c1_3d2c0c4e","line":222,"in_reply_to":"bc058cb9_c330fafd","updated":"2026-08-18 19:07:22.000000000","message":"Done","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ac86c492797d6b15de89abe195bcad9c59b458a","unresolved":true,"context_lines":[{"line_number":225,"context_line":"                        \"shutdown has been started.\" %"},{"line_number":226,"context_line":"                        executor_type.value)"},{"line_number":227,"context_line":"                executor \u003d self._all_executors.get(executor_type)"},{"line_number":228,"context_line":"                if not executor:"},{"line_number":229,"context_line":"                    max_workers \u003d ExecutorsPoolSize.get(executor_type)"},{"line_number":230,"context_line":"                    executor \u003d self._new_executor("},{"line_number":231,"context_line":"                        max_workers, executor_type.value)"}],"source_content_type":"text/x-python","patch_set":11,"id":"c23afe12_8e859447","line":228,"updated":"2026-08-18 13:46:50.000000000","message":"It looks to me like in all of the functional and unit tests we never hit the `if not executor` false case. How is that possible?\n\nhttps://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_c10/openstack/c106ccd9fd5a4b69a7ae62b561abb98e/cover/z_8ee3d46129bdc436_thread_pool_factory_py.html","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"013e658701ed8c26b2bdc141c5149fca8e452834","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                        \"shutdown has been started.\" %"},{"line_number":226,"context_line":"                        executor_type.value)"},{"line_number":227,"context_line":"                executor \u003d self._all_executors.get(executor_type)"},{"line_number":228,"context_line":"                if not executor:"},{"line_number":229,"context_line":"                    max_workers \u003d ExecutorsPoolSize.get(executor_type)"},{"line_number":230,"context_line":"                    executor \u003d self._new_executor("},{"line_number":231,"context_line":"                        max_workers, executor_type.value)"}],"source_content_type":"text/x-python","patch_set":11,"id":"35d603fd_c5ab0052","line":228,"in_reply_to":"92afc1ca_933104e3","updated":"2026-08-19 14:14:44.000000000","message":"Done","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"72524c6d20606cafcf150455551a4e900d4bcd9b","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                        \"shutdown has been started.\" %"},{"line_number":226,"context_line":"                        executor_type.value)"},{"line_number":227,"context_line":"                executor \u003d self._all_executors.get(executor_type)"},{"line_number":228,"context_line":"                if not executor:"},{"line_number":229,"context_line":"                    max_workers \u003d ExecutorsPoolSize.get(executor_type)"},{"line_number":230,"context_line":"                    executor \u003d self._new_executor("},{"line_number":231,"context_line":"                        max_workers, executor_type.value)"}],"source_content_type":"text/x-python","patch_set":11,"id":"a29f41be_d506193c","line":228,"in_reply_to":"92afc1ca_933104e3","updated":"2026-08-19 17:51:42.000000000","message":"Done","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3ab0f6356455d5165d4cb1876ba98ef1bf0d32c9","unresolved":true,"context_lines":[{"line_number":225,"context_line":"                        \"shutdown has been started.\" %"},{"line_number":226,"context_line":"                        executor_type.value)"},{"line_number":227,"context_line":"                executor \u003d self._all_executors.get(executor_type)"},{"line_number":228,"context_line":"                if not executor:"},{"line_number":229,"context_line":"                    max_workers \u003d ExecutorsPoolSize.get(executor_type)"},{"line_number":230,"context_line":"                    executor \u003d self._new_executor("},{"line_number":231,"context_line":"                        max_workers, executor_type.value)"}],"source_content_type":"text/x-python","patch_set":11,"id":"92afc1ca_933104e3","line":228,"in_reply_to":"c23afe12_8e859447","updated":"2026-08-18 19:07:22.000000000","message":"because this is 2nd check under the lock when executors is not created yet. L220 make sure if executors exist then it will just return. This extra check on executor is to handle rare case when Thread1 did not find the executor (so it pass condition at L220) and wait for lock to create the new but thread2 already creating that by aquiring the lock. Once thread2 finish, thread1 aquire lock but again can check if executor exist or not before it create new one.\n\nWe can just have executor exist check under lock but that makes all get call series even executors exist and multiple thread can get it simulatiously.\n\nI will see if I can add some coverage for that.","commit_id":"8af9f3bbf9440fdeab2c9b6a31f6e490aeb6087c"}],"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"}]}