)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8fe272ba16a042b5b0c7d2da476e9e054aa9bdd0","unresolved":true,"context_lines":[{"line_number":9,"context_line":"graceful shutdown functional tests are unstable and some tests"},{"line_number":10,"context_line":"hit the service down in between of the tests. One of the"},{"line_number":11,"context_line":"possible reason is service status heartbeat where service can"},{"line_number":12,"context_line":"update their status as down too early and in between of other tests."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"  File \"/home/zuul/src/opendev.org/openstack/nova/nova/conductor/tasks/base.py\", line 23, in wrap"},{"line_number":15,"context_line":"    return original(self)"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5b595dfd_500def21","line":12,"range":{"start_line":12,"start_character":38,"end_line":12,"end_character":68},"updated":"2026-08-25 08:54:11.000000000","message":"that is strange. I think we stop every service at the end of the test case and start a new service for the new test case. Are there services leaking across test cases? If so we need to fix that as well","commit_id":"26b4247dfd9c920d700f3b89570c58bba86b1f03"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"75b6017045c7ee19bfbb3afe9eac5f7d07571156","unresolved":false,"context_lines":[{"line_number":9,"context_line":"graceful shutdown functional tests are unstable and some tests"},{"line_number":10,"context_line":"hit the service down in between of the tests. One of the"},{"line_number":11,"context_line":"possible reason is service status heartbeat where service can"},{"line_number":12,"context_line":"update their status as down too early and in between of other tests."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"  File \"/home/zuul/src/opendev.org/openstack/nova/nova/conductor/tasks/base.py\", line 23, in wrap"},{"line_number":15,"context_line":"    return original(self)"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"0ebfc874_33b0dfd1","line":12,"range":{"start_line":12,"start_character":38,"end_line":12,"end_character":68},"in_reply_to":"5b595dfd_500def21","updated":"2026-08-25 19:00:56.000000000","message":"yes, test do start service in setUp and even I added explicit wait for service status to be up - https://review.opendev.org/c/openstack/nova/+/1002223/2/nova/tests/functional/test_graceful_shutdown.py#74\n\nThat make sure service is ready when test starts but still test see service down during its execution time (when nobody did service.stop). When test started service was up, test created server so service was up but when test was waiting for live migration to complete, conductor _check_host_is_up strated seeing the compute as down. This proof that service is going down in between of tests. As no body is stopping service in between, one possible reason can be DB reporting the status wrongly. With so many tests running in heavy parallel load and in threading mode, there is chance that service status reporting gets its time to execute late (say late than service_down_time of 6 sec) and DB start showing service as down after 6 sec.\n\nIncreasing the service_down_time, we will be narrowing down that window. let me update the test notes about it.","commit_id":"26b4247dfd9c920d700f3b89570c58bba86b1f03"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e4761f33564354687cccce3d885249393f1b4329","unresolved":true,"context_lines":[{"line_number":13,"context_line":"time (when nobody did service.stop). When test started service was up,"},{"line_number":14,"context_line":"test created server so service was up but when test was waiting for"},{"line_number":15,"context_line":"live migration to complete, conductor _check_host_is_up strated seeing"},{"line_number":16,"context_line":"the compute as down. This proof that service is going down in between"},{"line_number":17,"context_line":"of tests. As no body is stopping service in between, one possible reason"},{"line_number":18,"context_line":"can be DB reporting the status wrongly. With so many tests running in"},{"line_number":19,"context_line":"heavy parallel load and in threading mode, there is chance that service"},{"line_number":20,"context_line":"status reporting gets its time to execute late and DB start showing service"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"f0cfe6e7_e622969b","line":17,"range":{"start_line":16,"start_character":21,"end_line":17,"end_character":9},"updated":"2026-08-26 11:41:13.000000000","message":"do you mean during the test case not in between two test cases?","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4377b8824574320ac377134c42b0a9b8d58873b0","unresolved":true,"context_lines":[{"line_number":13,"context_line":"time (when nobody did service.stop). When test started service was up,"},{"line_number":14,"context_line":"test created server so service was up but when test was waiting for"},{"line_number":15,"context_line":"live migration to complete, conductor _check_host_is_up strated seeing"},{"line_number":16,"context_line":"the compute as down. This proof that service is going down in between"},{"line_number":17,"context_line":"of tests. As no body is stopping service in between, one possible reason"},{"line_number":18,"context_line":"can be DB reporting the status wrongly. With so many tests running in"},{"line_number":19,"context_line":"heavy parallel load and in threading mode, there is chance that service"},{"line_number":20,"context_line":"status reporting gets its time to execute late and DB start showing service"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"2da05cd7_359948fc","line":17,"range":{"start_line":16,"start_character":21,"end_line":17,"end_character":9},"in_reply_to":"f0cfe6e7_e622969b","updated":"2026-08-26 21:21:28.000000000","message":"yes, \u0027during the test\u0027 will fix it","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8fe272ba16a042b5b0c7d2da476e9e054aa9bdd0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"24892614_f4925d82","updated":"2026-08-25 08:54:11.000000000","message":"the functional test now fails in CI in this patch\n```\nnova.tests.functional.test_graceful_shutdown.TestComputeGracefulShutdown.test_revert_resize_dest_compute_graceful_shutdown\n--------------------------------------------------------------------------------------------------------------------------\n\nCaptured traceback:\n~~~~~~~~~~~~~~~~~~~\n    Traceback (most recent call last):\n\n      File \"/home/zuul/src/opendev.org/openstack/nova/nova/tests/functional/test_graceful_shutdown.py\", line 562, in test_revert_resize_dest_compute_graceful_shutdown\n    self.wait_for_service_stop(stop_thread, \u0027dest\u0027)\n    ~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^\n\n      File \"/home/zuul/src/opendev.org/openstack/nova/nova/tests/functional/test_graceful_shutdown.py\", line 109, in wait_for_service_stop\n    self._wait_for_service_parameter(\n    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^\n        host, binary, {\u0027state\u0027: \u0027down\u0027}, max_retries\u003d20)\n        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n      File \"/home/zuul/src/opendev.org/openstack/nova/nova/tests/functional/integrated_helpers.py\", line 121, in _wait_for_service_parameter\n    self.fail(\n    ~~~~~~~~~^\n        f\u0027Wait for service parameter change failed, \u0027\n        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n        f\u0027expected_params\u003d{expected_params}, service\u003d{service}\u0027)\n        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n      File \"/usr/lib/python3.13/unittest/case.py\", line 732, in fail\n    raise self.failureException(msg)\n\n    AssertionError: Wait for service parameter change failed, expected_params\u003d{\u0027state\u0027: \u0027down\u0027}, service\u003d{\u0027binary\u0027: \u0027nova-compute\u0027, \u0027host\u0027: \u0027dest\u0027, \u0027id\u0027: \u00279ff1cc5f-36a6-4d4f-9bcd-b8aef93a6ac8\u0027, \u0027zone\u0027: \u0027nova\u0027, \u0027status\u0027: \u0027enabled\u0027, \u0027state\u0027: \u0027up\u0027, \u0027updated_at\u0027: None, \u0027disabled_reason\u0027: None, \u0027forced_down\u0027: False}\n    ```","commit_id":"26b4247dfd9c920d700f3b89570c58bba86b1f03"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b9bfce30c905199ce49fcad58ffdd745f21fbe75","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"556affa4_e39ef37c","in_reply_to":"24892614_f4925d82","updated":"2026-08-25 08:54:55.000000000","message":"I guess the config change affect some tests that needs to wait for the down state but now it times out","commit_id":"26b4247dfd9c920d700f3b89570c58bba86b1f03"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"734b3369acf0545e6830887c1218c41cf99dfdac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1139238f_fb464ce7","in_reply_to":"38a40077_0a72893a","updated":"2026-08-25 19:08:22.000000000","message":"| also it might make sense to put this on master directly as their parents might not land due to flaky CI making this hard to land.\n\nits both way, keeping it on top of other fixes also narrow down if same failure happen in this. let\u0027s keep it in the series. can change if we see if anything stuck due to deps.","commit_id":"26b4247dfd9c920d700f3b89570c58bba86b1f03"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"365eca2d3a7b4d34d25af475f1b20f16fc64c59c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e0ffdc98_056e043e","in_reply_to":"556affa4_e39ef37c","updated":"2026-08-25 08:57:15.000000000","message":"also it might make sense to put this on master directly as their parents might not land due to flaky CI making this hard to land.","commit_id":"26b4247dfd9c920d700f3b89570c58bba86b1f03"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"a8221b58ed36999fedbe55093bba53e9de74a56a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"38a40077_0a72893a","in_reply_to":"e0ffdc98_056e043e","updated":"2026-08-25 19:02:24.000000000","message":"there is all different timeout/retry here which effect the how test get the service status. let me allign those with single value so that test execution, polling status, DB service stats updates will be in sync.","commit_id":"26b4247dfd9c920d700f3b89570c58bba86b1f03"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"57a38efa9eb6a785eadc5707fdac9692fff7990a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"5237b3b3_f4e665ad","updated":"2026-08-27 20:28:54.000000000","message":"Updates look good to me, gibi was previously +2, so approving.","commit_id":"ac8864d949968b4e37d764eaed21c497bd1d2f11"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"67b6dabcd2c92ab8390438704a9ebe862d381c69","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"eb947e46_61ad1a0c","updated":"2026-08-28 04:42:37.000000000","message":"recheck tox env installation error","commit_id":"ac8864d949968b4e37d764eaed21c497bd1d2f11"}],"nova/tests/functional/test_graceful_shutdown.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"789955de18bdcb523d1d63741108b340aeba19c3","unresolved":true,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def setUp(self):"},{"line_number":56,"context_line":"        # report_interval/service_down_time control how fast the compute"},{"line_number":57,"context_line":"        # service update status to \"down\". graceful shutdown tests are heavly"},{"line_number":58,"context_line":"        # relies on the service start/stop so we need higher service_down_time"},{"line_number":59,"context_line":"        # here so that service status down does not happen during mid of"},{"line_number":60,"context_line":"        # other tests."},{"line_number":61,"context_line":"        self.flags(report_interval\u003d1, service_down_time\u003d20)"}],"source_content_type":"text/x-python","patch_set":2,"id":"aba09f0a_bd4ee3f2","line":58,"range":{"start_line":57,"start_character":43,"end_line":58,"end_character":16},"updated":"2026-08-25 15:28:26.000000000","message":"graceful shutdown tests heavily rely","commit_id":"26b4247dfd9c920d700f3b89570c58bba86b1f03"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"75b6017045c7ee19bfbb3afe9eac5f7d07571156","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def setUp(self):"},{"line_number":56,"context_line":"        # report_interval/service_down_time control how fast the compute"},{"line_number":57,"context_line":"        # service update status to \"down\". graceful shutdown tests are heavly"},{"line_number":58,"context_line":"        # relies on the service start/stop so we need higher service_down_time"},{"line_number":59,"context_line":"        # here so that service status down does not happen during mid of"},{"line_number":60,"context_line":"        # other tests."},{"line_number":61,"context_line":"        self.flags(report_interval\u003d1, service_down_time\u003d20)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fd3dab2e_9a1099f2","line":58,"range":{"start_line":57,"start_character":43,"end_line":58,"end_character":16},"in_reply_to":"aba09f0a_bd4ee3f2","updated":"2026-08-25 19:00:56.000000000","message":"Done","commit_id":"26b4247dfd9c920d700f3b89570c58bba86b1f03"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e4761f33564354687cccce3d885249393f1b4329","unresolved":true,"context_lines":[{"line_number":81,"context_line":"        # .5 sec, so wait for CONF.service_down_time + 10 sec worth of"},{"line_number":82,"context_line":"        # retries whenever we poll for a service state (\u0027up\u0027 or \u0027down\u0027)"},{"line_number":83,"context_line":"        # change in this test class."},{"line_number":84,"context_line":"        self.max_retries \u003d int(CONF.service_down_time / 0.5 + 10)"},{"line_number":85,"context_line":"        self._start_compute(\u0027src\u0027)"},{"line_number":86,"context_line":"        self._start_compute(\u0027dest\u0027)"},{"line_number":87,"context_line":"        # The servicegroup DB driver waits INITIAL_REPORTING_DELAY (5s)"}],"source_content_type":"text/x-python","patch_set":3,"id":"6fc66f72_658611bc","line":84,"updated":"2026-08-26 11:41:13.000000000","message":"as far as I see this is 65 / 0.5 + 10 \u003d 140 retries. We poll every 0.5 sec in _wait_for_service_parameter that means this test can potentially wait for 70 seconds on a single _wait_for_service_parameter call. Is this really necessary in all cases?\n\n\nBTW why do we need such a huge service_downtime? Do we stop sending the hearthbeat during graceful shutdown? If not then the service should not go down during the graceful period. I think we can and should keep sending the hearthbeat during graceful shutdown to avoid in progress live migration to fail due to a host reported down.","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4377b8824574320ac377134c42b0a9b8d58873b0","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        # .5 sec, so wait for CONF.service_down_time + 10 sec worth of"},{"line_number":82,"context_line":"        # retries whenever we poll for a service state (\u0027up\u0027 or \u0027down\u0027)"},{"line_number":83,"context_line":"        # change in this test class."},{"line_number":84,"context_line":"        self.max_retries \u003d int(CONF.service_down_time / 0.5 + 10)"},{"line_number":85,"context_line":"        self._start_compute(\u0027src\u0027)"},{"line_number":86,"context_line":"        self._start_compute(\u0027dest\u0027)"},{"line_number":87,"context_line":"        # The servicegroup DB driver waits INITIAL_REPORTING_DELAY (5s)"}],"source_content_type":"text/x-python","patch_set":3,"id":"c275a184_d374578c","line":84,"in_reply_to":"6fc66f72_658611bc","updated":"2026-08-26 21:21:28.000000000","message":"\u003e as far as I see this is 65 / 0.5 + 10 \u003d 140 retries. We poll every 0.5 sec in _wait_for_service_parameter that means this test can potentially wait for 70 seconds on a single _wait_for_service_parameter call. Is this really necessary in all cases?\n\nyes, that\u0027s right. It needs to be more than \u0027service_down_time\u0027 otherwise  _wait_for_service_parameter() waiting for service \u0027down\u0027 will timeout before Db update the status which happen after _wait_for_service_parameter. That is why i changed it to calculate from service_down_time value.\n\n\u003e \n\u003e \n\u003e BTW why do we need such a huge service_downtime? Do we stop sending the hearthbeat during graceful shutdown? If not then the service should not go down during the graceful period. I think we can and should keep sending the heartbeat during graceful shutdown to avoid in progress live migration to fail due to a host reported down.\n\nwe do not stop sending the heartbeat during shutdown. service heartbeat thread is not touched and it should keep reporting the service until the main process is stopped (means shutdown is finished).\n\nOn higher value, I think yes. I am taking it safe path. Basically how long a test can run (in worst case self.OPERATION_TIMEOUT), service should stay up. The worst case I am considering is that due to high load of parallelism if the heartbeat thread did get not time to execute for long time say till self.OPERATION_TIMEOUT then test will see the service as unavailable. That is why to consider this worst case I am setting service_down_time as \"self.OPERATION_TIMEOUT +5\".\n\nThough this is very rare that heartbeat thread will be not get its turn for that long, I can go back and try with service_down_time as 30 sec (heartbeat arrives in 30 sec) and if we still see this issue then we can change.","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"de971823772d3a0c997b0cfec4ffb25538c190c4","unresolved":true,"context_lines":[{"line_number":87,"context_line":"        # The servicegroup DB driver waits INITIAL_REPORTING_DELAY (5s)"},{"line_number":88,"context_line":"        # before sending the first service state report. Without waiting"},{"line_number":89,"context_line":"        # here, a slow test host can see either service as \"down\" (falling"},{"line_number":90,"context_line":"        # back to its stale created_at timestamp) before its first heartbeat"},{"line_number":91,"context_line":"        # lands, causing 409 ServiceUnavailable errors."},{"line_number":92,"context_line":"        for host in (\u0027src\u0027, \u0027dest\u0027):"},{"line_number":93,"context_line":"            self._wait_for_service_parameter("}],"source_content_type":"text/x-python","patch_set":3,"id":"9997bc1c_41882d81","line":90,"updated":"2026-08-26 12:04:12.000000000","message":"this cannot be true, that would mean that each of our functional test that boots a VM should wait 5 sec before it can boot the VM but our functional tests are not like that, they totally capable of running it in less than 5 secs.","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f7247ed236d9b84e2994867a25bfb9d1f510f1c4","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        # The servicegroup DB driver waits INITIAL_REPORTING_DELAY (5s)"},{"line_number":88,"context_line":"        # before sending the first service state report. Without waiting"},{"line_number":89,"context_line":"        # here, a slow test host can see either service as \"down\" (falling"},{"line_number":90,"context_line":"        # back to its stale created_at timestamp) before its first heartbeat"},{"line_number":91,"context_line":"        # lands, causing 409 ServiceUnavailable errors."},{"line_number":92,"context_line":"        for host in (\u0027src\u0027, \u0027dest\u0027):"},{"line_number":93,"context_line":"            self._wait_for_service_parameter("}],"source_content_type":"text/x-python","patch_set":3,"id":"8bea429c_983a2a31","line":90,"in_reply_to":"4f7d5d10_0275a6d0","updated":"2026-08-27 10:46:41.000000000","message":"yeah that is a valid point about a possible misconfiguration. We might need to state in our config doc that service_down_time \u003c 5 is not really a stable configuration due to our hardcoded 5.","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"76790e39be39c4c448a207762745772a8c752351","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        # The servicegroup DB driver waits INITIAL_REPORTING_DELAY (5s)"},{"line_number":88,"context_line":"        # before sending the first service state report. Without waiting"},{"line_number":89,"context_line":"        # here, a slow test host can see either service as \"down\" (falling"},{"line_number":90,"context_line":"        # back to its stale created_at timestamp) before its first heartbeat"},{"line_number":91,"context_line":"        # lands, causing 409 ServiceUnavailable errors."},{"line_number":92,"context_line":"        for host in (\u0027src\u0027, \u0027dest\u0027):"},{"line_number":93,"context_line":"            self._wait_for_service_parameter("}],"source_content_type":"text/x-python","patch_set":3,"id":"a3a1467f_73036e06","line":90,"in_reply_to":"8bea429c_983a2a31","updated":"2026-08-27 12:55:49.000000000","message":"reported bug for this, let\u0027s fix it separately https://bugs.launchpad.net/nova/+bug/2165323","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4377b8824574320ac377134c42b0a9b8d58873b0","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        # The servicegroup DB driver waits INITIAL_REPORTING_DELAY (5s)"},{"line_number":88,"context_line":"        # before sending the first service state report. Without waiting"},{"line_number":89,"context_line":"        # here, a slow test host can see either service as \"down\" (falling"},{"line_number":90,"context_line":"        # back to its stale created_at timestamp) before its first heartbeat"},{"line_number":91,"context_line":"        # lands, causing 409 ServiceUnavailable errors."},{"line_number":92,"context_line":"        for host in (\u0027src\u0027, \u0027dest\u0027):"},{"line_number":93,"context_line":"            self._wait_for_service_parameter("}],"source_content_type":"text/x-python","patch_set":3,"id":"4f7d5d10_0275a6d0","line":90,"in_reply_to":"9997bc1c_41882d81","updated":"2026-08-26 21:21:28.000000000","message":"you are right, i considered only first heartbeat case but after rechecking the code, servicegroup consider the \u0027created_at\u0027 time for first INITIAL_REPORTING_DELAY (5 sec) before first heartbeat is set. So basically service will always be up until first heartbeat is seen or  \nservice_down_time is elapsed.\n\nOn side note:\n\nI am thinking that we have a production gap here (rare one though), what if service_down_time is configured \u003c INITIAL_REPORTING_DELAY then service will be seen as down even it did not get its first heartbeat. for example:\n\n- service_down_time is configured as 3 sec\n- INITIAL_REPORTING_DELAY is hardcoded as 5 sec\n- service is created, created_at is considered for is_up for first 5 sec\n- after 3 sec (service_down_time) service will be seen as down\n- after 5 sec heartbeat started and it set service as up\n\nI feel we should compare the INITIAL_REPORTING_DELAY and service_down_time and if service_down_time \u003c INITIAL_REPORTING_DELAY then do not mark service down until first heartbeat is sent?","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e4761f33564354687cccce3d885249393f1b4329","unresolved":true,"context_lines":[{"line_number":92,"context_line":"        for host in (\u0027src\u0027, \u0027dest\u0027):"},{"line_number":93,"context_line":"            self._wait_for_service_parameter("},{"line_number":94,"context_line":"                host, \u0027nova-compute\u0027, {\u0027state\u0027: \u0027up\u0027},"},{"line_number":95,"context_line":"                max_retries\u003dself.max_retries)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _setup_graceful_shutdown_mock(self, compute, operation_complete_event):"},{"line_number":98,"context_line":"        # Manager graceful_shutdown() wait for operation_complete_event to"}],"source_content_type":"text/x-python","patch_set":3,"id":"c5a84643_8a1c0fa7","line":95,"updated":"2026-08-26 11:41:13.000000000","message":"if we need this due to the initial delay, then we can sleep for the initial delay and then call the _wait_ to poll for the status with the normal 10 retries with 0.5sec sleeps.","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"81fa8ee904dd8984cf530d1979577796d038a14f","unresolved":true,"context_lines":[{"line_number":92,"context_line":"        for host in (\u0027src\u0027, \u0027dest\u0027):"},{"line_number":93,"context_line":"            self._wait_for_service_parameter("},{"line_number":94,"context_line":"                host, \u0027nova-compute\u0027, {\u0027state\u0027: \u0027up\u0027},"},{"line_number":95,"context_line":"                max_retries\u003dself.max_retries)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _setup_graceful_shutdown_mock(self, compute, operation_complete_event):"},{"line_number":98,"context_line":"        # Manager graceful_shutdown() wait for operation_complete_event to"}],"source_content_type":"text/x-python","patch_set":3,"id":"579a5536_a7362e0a","line":95,"in_reply_to":"1b88493c_4d6fbbec","updated":"2026-08-27 17:07:09.000000000","message":"That is kind of surprising that any tests fail without this non-waiting wait 🤔 \n\nDid you see it in a local test run or how did you check it?","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7706b3e80cfe8a3a60e300bd3d4c7cce446a87d2","unresolved":true,"context_lines":[{"line_number":92,"context_line":"        for host in (\u0027src\u0027, \u0027dest\u0027):"},{"line_number":93,"context_line":"            self._wait_for_service_parameter("},{"line_number":94,"context_line":"                host, \u0027nova-compute\u0027, {\u0027state\u0027: \u0027up\u0027},"},{"line_number":95,"context_line":"                max_retries\u003dself.max_retries)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _setup_graceful_shutdown_mock(self, compute, operation_complete_event):"},{"line_number":98,"context_line":"        # Manager graceful_shutdown() wait for operation_complete_event to"}],"source_content_type":"text/x-python","patch_set":3,"id":"e0aaa17b_72ae9406","line":95,"in_reply_to":"35480b69_ee666b1f","updated":"2026-08-27 19:18:50.000000000","message":"Ahh, got it, thank you. Sorry, I thought when you said \"removing wait after start service here fail the cold migration tests\" specifically _here_ you saw tests fail after removing the wait from setUp() and I was so confused 😆 I understand now, thanks.","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"161301c51d6cbf7edd6949c567a1ffdd3b57567b","unresolved":true,"context_lines":[{"line_number":92,"context_line":"        for host in (\u0027src\u0027, \u0027dest\u0027):"},{"line_number":93,"context_line":"            self._wait_for_service_parameter("},{"line_number":94,"context_line":"                host, \u0027nova-compute\u0027, {\u0027state\u0027: \u0027up\u0027},"},{"line_number":95,"context_line":"                max_retries\u003dself.max_retries)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _setup_graceful_shutdown_mock(self, compute, operation_complete_event):"},{"line_number":98,"context_line":"        # Manager graceful_shutdown() wait for operation_complete_event to"}],"source_content_type":"text/x-python","patch_set":3,"id":"35480b69_ee666b1f","line":95,"in_reply_to":"579a5536_a7362e0a","updated":"2026-08-27 19:09:12.000000000","message":"Here we do not need but we need in _restart_compute() where it restart the existing service instead of freshly created (even that call same self._start_compute() but it start the existing service. I should have commented it in _restart_compute function comment but let me explain here as discussion is here - https://review.opendev.org/c/openstack/nova/+/1002223/comment/5c405b21_31b1317d/\n\nnewly created service start vs restart:\n---------------------------------------\n\n**newly created service start**: In this case, when we start service start, servicegroup consider \u0027created_at\u0027 instead of \u0027last_seen_up\u0027[1] and service is always up until first heartbeat arrives (\u0027created_at\u0027 time elapsed is \u003c service_down_Time).\n\n**restart existing service**: When we stop the service, DB entry of that service is not deleted but service status is \u0027down\u0027. Once we start it again then it remains \u0027down\u0027 until first heartbeat arrives because this time it does not consider \u0027created_at\u0027 (because service is not newly created) instead it consider \u0027last_seen_up\u0027 to check the elapsed time. That is why service remains \u0027down\u0027 after start until first heartbeat arrives so we need to wait there.\n\nIf we remove wait then it fail test_cold_migration_source_compute_graceful_shutdown 100% locally. becuase this test restart the service in between and then confirResie is performed and fail because of service down.\n\n[1] https://github.com/openstack/nova/blob/e8e78114191b120f0412d26c903f9e716c08d87a/nova/servicegroup/drivers/db.py#L61-L62\n\nIf you would like, I can remove the wait here as setUp aleays create the new service and keep wait only in _restart_service.","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4377b8824574320ac377134c42b0a9b8d58873b0","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        for host in (\u0027src\u0027, \u0027dest\u0027):"},{"line_number":93,"context_line":"            self._wait_for_service_parameter("},{"line_number":94,"context_line":"                host, \u0027nova-compute\u0027, {\u0027state\u0027: \u0027up\u0027},"},{"line_number":95,"context_line":"                max_retries\u003dself.max_retries)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _setup_graceful_shutdown_mock(self, compute, operation_complete_event):"},{"line_number":98,"context_line":"        # Manager graceful_shutdown() wait for operation_complete_event to"}],"source_content_type":"text/x-python","patch_set":3,"id":"e18666d8_8fb83edf","line":95,"in_reply_to":"c5a84643_8a1c0fa7","updated":"2026-08-26 21:21:28.000000000","message":"no, this is not needed as I missed to check the created_at time is considered for initial delay.","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"8cd53bdaf3c5474117266c445950b72216875b22","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        for host in (\u0027src\u0027, \u0027dest\u0027):"},{"line_number":93,"context_line":"            self._wait_for_service_parameter("},{"line_number":94,"context_line":"                host, \u0027nova-compute\u0027, {\u0027state\u0027: \u0027up\u0027},"},{"line_number":95,"context_line":"                max_retries\u003dself.max_retries)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _setup_graceful_shutdown_mock(self, compute, operation_complete_event):"},{"line_number":98,"context_line":"        # Manager graceful_shutdown() wait for operation_complete_event to"}],"source_content_type":"text/x-python","patch_set":3,"id":"23d2beb4_c29cc9f8","line":95,"in_reply_to":"e0aaa17b_72ae9406","updated":"2026-08-27 19:26:09.000000000","message":"Done","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"c0a9809002564840989fed8a45c057f89e4bf0f2","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        for host in (\u0027src\u0027, \u0027dest\u0027):"},{"line_number":93,"context_line":"            self._wait_for_service_parameter("},{"line_number":94,"context_line":"                host, \u0027nova-compute\u0027, {\u0027state\u0027: \u0027up\u0027},"},{"line_number":95,"context_line":"                max_retries\u003dself.max_retries)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _setup_graceful_shutdown_mock(self, compute, operation_complete_event):"},{"line_number":98,"context_line":"        # Manager graceful_shutdown() wait for operation_complete_event to"}],"source_content_type":"text/x-python","patch_set":3,"id":"1b88493c_4d6fbbec","line":95,"in_reply_to":"e18666d8_8fb83edf","updated":"2026-08-27 00:36:30.000000000","message":"removing wait after start service here fail the cold migration tests, let me keep them as it is and only change service_down_time which will fix the race.","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e4761f33564354687cccce3d885249393f1b4329","unresolved":true,"context_lines":[{"line_number":129,"context_line":"        \"\"\"Join stop_thread then poll until the service is down in the DB.\"\"\""},{"line_number":130,"context_line":"        self._join_stop_thread(stop_thread, timeout\u003dtimeout)"},{"line_number":131,"context_line":"        self._wait_for_service_parameter("},{"line_number":132,"context_line":"            host, binary, {\u0027state\u0027: \u0027down\u0027}, max_retries\u003dself.max_retries)"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    def _restart_compute(self, hostname):"},{"line_number":135,"context_line":"        self.computes.pop(hostname, None)"}],"source_content_type":"text/x-python","patch_set":3,"id":"03c6cf3d_92aae194","line":132,"updated":"2026-08-26 11:41:13.000000000","message":"so here initiated a stop and want to see the service going down. Here I can imagine the need for a max_retry of 140.","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4377b8824574320ac377134c42b0a9b8d58873b0","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        \"\"\"Join stop_thread then poll until the service is down in the DB.\"\"\""},{"line_number":130,"context_line":"        self._join_stop_thread(stop_thread, timeout\u003dtimeout)"},{"line_number":131,"context_line":"        self._wait_for_service_parameter("},{"line_number":132,"context_line":"            host, binary, {\u0027state\u0027: \u0027down\u0027}, max_retries\u003dself.max_retries)"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    def _restart_compute(self, hostname):"},{"line_number":135,"context_line":"        self.computes.pop(hostname, None)"}],"source_content_type":"text/x-python","patch_set":3,"id":"f933c780_531f83f2","line":132,"in_reply_to":"03c6cf3d_92aae194","updated":"2026-08-26 21:21:28.000000000","message":"Acknowledged","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e4761f33564354687cccce3d885249393f1b4329","unresolved":true,"context_lines":[{"line_number":136,"context_line":"        self._start_compute(hostname)"},{"line_number":137,"context_line":"        self._wait_for_service_parameter("},{"line_number":138,"context_line":"            hostname, \u0027nova-compute\u0027, {\u0027state\u0027: \u0027up\u0027},"},{"line_number":139,"context_line":"            max_retries\u003dself.max_retries)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def _complete_live_migration(self, context, instance, dest, post_method,"},{"line_number":142,"context_line":"                                  recover_method, block_migration\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5c405b21_31b1317d","line":139,"updated":"2026-08-26 11:41:13.000000000","message":"ditto, sleep for initial delay then use a normal 10 retry with 0.5 sec sleep","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4377b8824574320ac377134c42b0a9b8d58873b0","unresolved":false,"context_lines":[{"line_number":136,"context_line":"        self._start_compute(hostname)"},{"line_number":137,"context_line":"        self._wait_for_service_parameter("},{"line_number":138,"context_line":"            hostname, \u0027nova-compute\u0027, {\u0027state\u0027: \u0027up\u0027},"},{"line_number":139,"context_line":"            max_retries\u003dself.max_retries)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def _complete_live_migration(self, context, instance, dest, post_method,"},{"line_number":142,"context_line":"                                  recover_method, block_migration\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":3,"id":"78eb2176_486bc701","line":139,"in_reply_to":"5c405b21_31b1317d","updated":"2026-08-26 21:21:28.000000000","message":"as replied above, we do not need this wait as service will be always up after start until first heartbeat is seen.","commit_id":"b27cc85dad630634b6bcac2f31bbeae73583e1c3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"161301c51d6cbf7edd6949c567a1ffdd3b57567b","unresolved":true,"context_lines":[{"line_number":365,"context_line":"        # compute service is restarted to check if after shutdown, service"},{"line_number":366,"context_line":"        # comes up normally and test server is deleted after confirm rezie."},{"line_number":367,"context_line":"        self._restart_compute(\u0027src\u0027)"},{"line_number":368,"context_line":"        self.api.post_server_action(server[\u0027id\u0027], {\u0027confirmResize\u0027: None})"},{"line_number":369,"context_line":"        server \u003d self._wait_for_state_change(server, \u0027ACTIVE\u0027)"},{"line_number":370,"context_line":"        self._delete_server(server)"},{"line_number":371,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"a047a288_48b7225d","line":368,"range":{"start_line":368,"start_character":0,"end_line":368,"end_character":74},"updated":"2026-08-27 19:09:12.000000000","message":"here, tt restart the service and immediately perform confirmResize which find the service as \u0027down\u0027","commit_id":"335beb4cc682e11f2572b4f1956be9d42213418b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7706b3e80cfe8a3a60e300bd3d4c7cce446a87d2","unresolved":true,"context_lines":[{"line_number":365,"context_line":"        # compute service is restarted to check if after shutdown, service"},{"line_number":366,"context_line":"        # comes up normally and test server is deleted after confirm rezie."},{"line_number":367,"context_line":"        self._restart_compute(\u0027src\u0027)"},{"line_number":368,"context_line":"        self.api.post_server_action(server[\u0027id\u0027], {\u0027confirmResize\u0027: None})"},{"line_number":369,"context_line":"        server \u003d self._wait_for_state_change(server, \u0027ACTIVE\u0027)"},{"line_number":370,"context_line":"        self._delete_server(server)"},{"line_number":371,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"a5b06621_ab0d8dc7","line":368,"range":{"start_line":368,"start_character":0,"end_line":368,"end_character":74},"in_reply_to":"a047a288_48b7225d","updated":"2026-08-27 19:18:50.000000000","message":"Got it, yes it makes sense that it\u0027s needed for the service restart case after it has been running before. I was only confused by the fact we were commenting in the setUp() method earlier 🙂","commit_id":"335beb4cc682e11f2572b4f1956be9d42213418b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"8cd53bdaf3c5474117266c445950b72216875b22","unresolved":false,"context_lines":[{"line_number":365,"context_line":"        # compute service is restarted to check if after shutdown, service"},{"line_number":366,"context_line":"        # comes up normally and test server is deleted after confirm rezie."},{"line_number":367,"context_line":"        self._restart_compute(\u0027src\u0027)"},{"line_number":368,"context_line":"        self.api.post_server_action(server[\u0027id\u0027], {\u0027confirmResize\u0027: None})"},{"line_number":369,"context_line":"        server \u003d self._wait_for_state_change(server, \u0027ACTIVE\u0027)"},{"line_number":370,"context_line":"        self._delete_server(server)"},{"line_number":371,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ec211b45_3aa9cdd4","line":368,"range":{"start_line":368,"start_character":0,"end_line":368,"end_character":74},"in_reply_to":"a5b06621_ab0d8dc7","updated":"2026-08-27 19:26:09.000000000","message":"Done","commit_id":"335beb4cc682e11f2572b4f1956be9d42213418b"}]}