)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e80e36b2538df0e29801b9300def612b66fc2d4e","unresolved":true,"context_lines":[{"line_number":35,"context_line":"The resize/migration itself will work as expected, since it will update"},{"line_number":36,"context_line":"the configuration accordingly afterwards. However, if the selected CPU"},{"line_number":37,"context_line":"numbers for the destination differ from the original ones on the source,"},{"line_number":38,"context_line":"the PCPU inventory in the resource_tracker will be temporarily"},{"line_number":39,"context_line":"incorrect. If the revert operation tries to drop the claims before the"},{"line_number":40,"context_line":"inventory is corrected, then it will fail to unpin the CPUs, because"},{"line_number":41,"context_line":"they will not be considered as pinned."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"Adding a simple fix to the specific issue of CPU pinning. It consists of"},{"line_number":44,"context_line":"moving the \u0027instance.apply_migration_context()\u0027 call from"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"200ad043_64985649","line":41,"range":{"start_line":38,"start_character":4,"end_line":41,"end_character":1},"updated":"2021-12-07 08:37:46.000000000","message":"I have a reproduction test for the first half of this issue in https://review.opendev.org/c/openstack/nova/+/820540\n\nIt would be nice to extend that with the revert step to observe the second issue as well.","commit_id":"1c1fde108737c8718974ee24f531414d7e6d26a1"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"122f30f371867edb0992d6260871f4cb65c047aa","unresolved":true,"context_lines":[{"line_number":41,"context_line":"they will not be considered as pinned."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"Adding a simple fix to the specific issue of CPU pinning. It consists of"},{"line_number":44,"context_line":"moving the \u0027instance.apply_migration_context()\u0027 call from"},{"line_number":45,"context_line":"_finish_resize() [4] to the same update where the instance host is saved"},{"line_number":46,"context_line":"as the destination host, on _resize_instance() [2]. This way, if the"},{"line_number":47,"context_line":"periodic task runs before [3], the instance will already reference the"},{"line_number":48,"context_line":"correct resource when the inventory is updated."},{"line_number":49,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"80448e4c_9eecff68","line":46,"range":{"start_line":44,"start_character":0,"end_line":46,"end_character":51},"updated":"2021-12-07 08:24:12.000000000","message":"I\u0027m wonder if this move has any unwanted side effect. I.e. finish_resize sets the instance.flavor \u003d new_flavor during a resize today but you move applying the migration context earlier. Also if there is a failure and the migration / resize is reverted then the migration context needs to be reverted too. Have you checked that by moving the applying earlier still makes the revert consistent?","commit_id":"1c1fde108737c8718974ee24f531414d7e6d26a1"},{"author":{"_account_id":33608,"name":"Gabriel Silva Trevisan","display_name":"Gabriel Silva Trevisan","email":"gabriel.silvatrevisan@windriver.com","username":"gtrevisan"},"change_message_id":"816ff60658f66066b6f6913672386662a9d0541d","unresolved":true,"context_lines":[{"line_number":41,"context_line":"they will not be considered as pinned."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"Adding a simple fix to the specific issue of CPU pinning. It consists of"},{"line_number":44,"context_line":"moving the \u0027instance.apply_migration_context()\u0027 call from"},{"line_number":45,"context_line":"_finish_resize() [4] to the same update where the instance host is saved"},{"line_number":46,"context_line":"as the destination host, on _resize_instance() [2]. This way, if the"},{"line_number":47,"context_line":"periodic task runs before [3], the instance will already reference the"},{"line_number":48,"context_line":"correct resource when the inventory is updated."},{"line_number":49,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"e4310e67_29ccfab9","line":46,"range":{"start_line":44,"start_character":0,"end_line":46,"end_character":51},"in_reply_to":"80448e4c_9eecff68","updated":"2021-12-07 13:02:42.000000000","message":"I share the same concern for the unwanted side-effects, since the apply_migration_context is saved in this case, so it may impact other unseen areas of the code which may use this information.\n\nRegarding the revert, I believe both changes should have the same effect. I\u0027ve checked that the revert was fixed by this change by forcing the task to run at the beginning of finish_resize, before the update to the instance is performed.","commit_id":"1c1fde108737c8718974ee24f531414d7e6d26a1"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a188c2d64e71444564ca4bbc650fcceb8f1d487f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"07066182_f0a1f663","updated":"2021-12-06 20:39:37.000000000","message":"FYI a patch that might be for the same issue:\n\nhttps://review.opendev.org/c/openstack/nova/+/820549","commit_id":"0471e0c4dd4c293a54a79b2a8077c92e12e0756c"},{"author":{"_account_id":33608,"name":"Gabriel Silva Trevisan","display_name":"Gabriel Silva Trevisan","email":"gabriel.silvatrevisan@windriver.com","username":"gtrevisan"},"change_message_id":"ea8b06b8ea8ab7c368862cb7b41b333431f43a93","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e144ec59_69db2896","in_reply_to":"07066182_f0a1f663","updated":"2021-12-06 21:08:59.000000000","message":"Thanks for pointing it out, will check.","commit_id":"0471e0c4dd4c293a54a79b2a8077c92e12e0756c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"7d3b279007a4ee48d4763c510275fe239f9908fc","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9773ac1c_c40a997b","in_reply_to":"4081f702_9ac2912f","updated":"2021-12-07 10:35:37.000000000","message":"Honestly, I prefer https://review.opendev.org/c/openstack/nova/+/820549/1/nova/compute/resource_tracker.py as it uses the RT for tracking the instances every periodic time.\n\nWhat we *could* do is merging first gibi\u0027s change and verifying whether we also need to merge this change, but I don\u0027t think we need it but maybe I\u0027m wrong","commit_id":"0471e0c4dd4c293a54a79b2a8077c92e12e0756c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1b9dc5768661be70c4c756477988fab565165cdf","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"eb88af5f_aa0e6e51","in_reply_to":"9773ac1c_c40a997b","updated":"2021-12-07 10:36:52.000000000","message":"I can try to extend the reproduction test case in my commit chain to cover the revert step to see if that is also solved.","commit_id":"0471e0c4dd4c293a54a79b2a8077c92e12e0756c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"122f30f371867edb0992d6260871f4cb65c047aa","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4081f702_9ac2912f","in_reply_to":"e144ec59_69db2896","updated":"2021-12-07 08:24:12.000000000","message":"Hm, strange coincidence that we both got hit by the same issue around the same time. Anyhow. I agree that we saw the same problem. So the two bugs are duplicates. I will mark the newer one as duplicate in launchpad. \n\nThe two patches represents two possible solutions for the same issue:\n\n* your patch attacks it from the resize codepath perspective. And moves the applying of the migration context from finish_resize to resize_instance. This change is observable by other parallel processes as the instance object changes are saved. And it moves that apply call from the source host to the destination host. \n\n* mine [1] attacks it from the other racing entity the periodic job. During the periodic job it temporarily and locally applies the migration context to an instances that are in the intermittent post-migration status to keep the resource accounting on the destination correct. \n\n@Reviewers: how do you feel which direction we should go?\n\n[1] https://review.opendev.org/c/openstack/nova/+/820549/1","commit_id":"0471e0c4dd4c293a54a79b2a8077c92e12e0756c"},{"author":{"_account_id":33608,"name":"Gabriel Silva Trevisan","display_name":"Gabriel Silva Trevisan","email":"gabriel.silvatrevisan@windriver.com","username":"gtrevisan"},"change_message_id":"816ff60658f66066b6f6913672386662a9d0541d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e5b720fd_aac2a911","in_reply_to":"eb88af5f_aa0e6e51","updated":"2021-12-07 13:02:42.000000000","message":"I agree that both issues seem to be the same too. I believe https://review.opendev.org/c/openstack/nova/+/820549/1/nova/compute/resource_tracker.py should also work, since it fixes the instance topology inside the resource tracker before it updates the resources for the instance. It has the benefit of also making the change locally, so it has less potential for impact in unexpected areas.","commit_id":"0471e0c4dd4c293a54a79b2a8077c92e12e0756c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"7d3b279007a4ee48d4763c510275fe239f9908fc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"95588957_271b22ee","updated":"2021-12-07 10:35:37.000000000","message":"I\u0027d prefer to wait for https://review.opendev.org/c/openstack/nova/+/820549 to be merged first before seeing whether we need to also merge this one.","commit_id":"1c1fde108737c8718974ee24f531414d7e6d26a1"},{"author":{"_account_id":33608,"name":"Gabriel Silva Trevisan","display_name":"Gabriel Silva Trevisan","email":"gabriel.silvatrevisan@windriver.com","username":"gtrevisan"},"change_message_id":"ca32bbf4352cab519ed868f09558e54e5ace29cd","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"0a10cf9b_cf1ab75f","in_reply_to":"95588957_271b22ee","updated":"2021-12-07 13:19:53.000000000","message":"I agree with this approach. Since the change on resource_tracker does not save the instance, it seems to be less potentially impactful, so we may try it first.","commit_id":"1c1fde108737c8718974ee24f531414d7e6d26a1"}]}