)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e2709845cfb4a58d95ff2f0626bfd2b4b9dfff67","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"45c7d281_548aad46","updated":"2025-01-07 12:44:16.000000000","message":"+1 the fix looks reasonable but i agree with gibi we likely do need a regression test for this or at least a unit test to assert that it is updated to 0","commit_id":"4d22871a4151bbd96ec991ebe7a3a1696b1da9e3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"369e1f73ea19efc944485d911eb9afa8439a4dd5","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b24850ea_0368d743","updated":"2024-10-28 15:28:59.000000000","message":"Thanks for proposing a fix! Could you please provide test coverage? A unit test should be easy to do. But would be also nice to add a functional reproducer that shows that if the compute is restarted after a compute service deletion and therefore un-delete the compute node then the nova-manage cell_v2 discover_host command will not re-discover the node.","commit_id":"4d22871a4151bbd96ec991ebe7a3a1696b1da9e3"},{"author":{"_account_id":34378,"name":"Julien LE JEUNE","email":"julien.le-jeune@mailops.fr","username":"jlejeune"},"change_message_id":"8416f5e579600b1187604f75b36ced0afba4a5b1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4afb4148_32047b46","in_reply_to":"0a51af89_fa5f3905","updated":"2025-10-09 09:43:11.000000000","message":"with a unit test","commit_id":"4d22871a4151bbd96ec991ebe7a3a1696b1da9e3"},{"author":{"_account_id":34378,"name":"Julien LE JEUNE","email":"julien.le-jeune@mailops.fr","username":"jlejeune"},"change_message_id":"a8a8efdb9817f9dee3f5a00cd9e6abb050f70895","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0a51af89_fa5f3905","in_reply_to":"b24850ea_0368d743","updated":"2025-10-08 11:47:17.000000000","message":"here is the regression test: https://review.opendev.org/c/openstack/nova/+/963368","commit_id":"4d22871a4151bbd96ec991ebe7a3a1696b1da9e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"98e17309b33683b1e4cb1185bb1add45ab0df119","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"7f22a505_f46510a9","updated":"2025-10-09 12:02:46.000000000","message":"i think the overall concept is correct but it could be implemented more effeicnelty\nsee the comment inline.","commit_id":"ee6a61ded80f12948b2ddcbea07332cd444a2fe5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"91712068694d34bf11967bd937f99c3977f754f5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"e6382676_5b39ff26","updated":"2025-10-10 08:21:03.000000000","message":"I have a question","commit_id":"c04c94ecfb1b5bfb85bddf36612665402ed2bde8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d65ce5efafff844713b5964d50fbabf526e18733","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"716794fb_dce37e82","updated":"2025-10-13 08:23:48.000000000","message":"Looks good to em","commit_id":"c04c94ecfb1b5bfb85bddf36612665402ed2bde8"},{"author":{"_account_id":34378,"name":"Julien LE JEUNE","email":"julien.le-jeune@mailops.fr","username":"jlejeune"},"change_message_id":"caa8bf40eea70e8bc24311c48e8d86959ac67697","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"534aa797_23b6e7aa","updated":"2025-10-09 18:16:42.000000000","message":"recheck nova-multi-cell","commit_id":"c04c94ecfb1b5bfb85bddf36612665402ed2bde8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"92c8b62f09bbe16fa52450836e9c2ce7af6ebbea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"9b70a9ab_193b6256","updated":"2025-10-10 09:49:47.000000000","message":"the refactor is asked for to inline the soft delete with the update of the mapped\u003d0 has been doen so this looks ok to me now","commit_id":"c04c94ecfb1b5bfb85bddf36612665402ed2bde8"}],"nova/db/main/api.py":[{"author":{"_account_id":26250,"name":"Johannes Kulik","email":"johannes.kulik@sap.com","username":"jkulik"},"change_message_id":"c282a6320533402f37cb26058c466759086b6b13","unresolved":true,"context_lines":[{"line_number":392,"context_line":"        # See https://bugs.launchpad.net/nova/+bug/2085135."},{"line_number":393,"context_line":"        compute_ref \u003d query.first()"},{"line_number":394,"context_line":"        if compute_ref:"},{"line_number":395,"context_line":"            compute_ref.update({\u0027mapped\u0027: 0})"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"        query.soft_delete(synchronize_session\u003dFalse)"},{"line_number":398,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"a6c96b8d_d61e5b9c","line":395,"updated":"2025-07-08 11:33:40.000000000","message":"If I read the DB schema correctly, there could be multiple ComputeNode entries having the same `host` attribute, as long as they have different `hypervisor_hostname` attributes. Wouldn\u0027t we have to update all of them instead of only the first?","commit_id":"4d22871a4151bbd96ec991ebe7a3a1696b1da9e3"},{"author":{"_account_id":34378,"name":"Julien LE JEUNE","email":"julien.le-jeune@mailops.fr","username":"jlejeune"},"change_message_id":"8416f5e579600b1187604f75b36ced0afba4a5b1","unresolved":false,"context_lines":[{"line_number":392,"context_line":"        # See https://bugs.launchpad.net/nova/+bug/2085135."},{"line_number":393,"context_line":"        compute_ref \u003d query.first()"},{"line_number":394,"context_line":"        if compute_ref:"},{"line_number":395,"context_line":"            compute_ref.update({\u0027mapped\u0027: 0})"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"        query.soft_delete(synchronize_session\u003dFalse)"},{"line_number":398,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"185c610c_434abf04","line":395,"in_reply_to":"67217665_91f8dcff","updated":"2025-10-09 09:43:11.000000000","message":"I have updated my change to reset all of relevant compute nodes","commit_id":"4d22871a4151bbd96ec991ebe7a3a1696b1da9e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6ad3506fd0544c330342db290ac1500f1961c300","unresolved":true,"context_lines":[{"line_number":392,"context_line":"        # See https://bugs.launchpad.net/nova/+bug/2085135."},{"line_number":393,"context_line":"        compute_ref \u003d query.first()"},{"line_number":394,"context_line":"        if compute_ref:"},{"line_number":395,"context_line":"            compute_ref.update({\u0027mapped\u0027: 0})"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"        query.soft_delete(synchronize_session\u003dFalse)"},{"line_number":398,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"67217665_91f8dcff","line":395,"in_reply_to":"a6c96b8d_d61e5b9c","updated":"2025-07-08 12:22:54.000000000","message":"you are effecitvly asking about ironic\n\nfor ironic we have 1 comptue service that manage many computes specificly for each ironic baremental node we model that as a seperate compute node managed by a shared compute service.\n\n\ni was going to say your wrong but no i cant fault your logic, if we delete the compute service we would need to unmap all compute nodes.\n\nthat gets complicated if your using the legacy peers config opiton.\n\nin caracal we replaced the use of peers with a new consept called ironic sharding\n\nbasiclly before that the ironic compute service would use a hash map and move float the compute nodes between the active compute services.\n\nthat was qutie bugy so we repalced it with a static mapping.\n\nin the new way of using Ironci you would in fact want to loop over the compute nodes and mark them all as unmapped I think, until a new compute service is created for that shard.","commit_id":"4d22871a4151bbd96ec991ebe7a3a1696b1da9e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"98e17309b33683b1e4cb1185bb1add45ab0df119","unresolved":true,"context_lines":[{"line_number":383,"context_line":"    if service.binary \u003d\u003d \u0027nova-compute\u0027:"},{"line_number":384,"context_line":"        # TODO(sbauza): Remove the service_id filter in a later release"},{"line_number":385,"context_line":"        # once we are sure that all compute nodes report the host field"},{"line_number":386,"context_line":"        query \u003d model_query(context, models.ComputeNode).\\"},{"line_number":387,"context_line":"            filter(sql.or_("},{"line_number":388,"context_line":"            models.ComputeNode.service_id \u003d\u003d service_id,"},{"line_number":389,"context_line":"            models.ComputeNode.host \u003d\u003d service[\u0027host\u0027]))"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        # NOTE(jlejeune): Make sure that the mapped field of the"},{"line_number":392,"context_line":"        # relevant compute nodes is set to 0"},{"line_number":393,"context_line":"        # See https://bugs.launchpad.net/nova/+bug/2085135."},{"line_number":394,"context_line":"        compute_nodes \u003d query.all()"},{"line_number":395,"context_line":"        for compute_node in compute_nodes:"},{"line_number":396,"context_line":"            compute_node.update({\u0027mapped\u0027: 0})"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"        query.soft_delete(synchronize_session\u003dFalse)"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":""},{"line_number":401,"context_line":"@pick_context_manager_reader"},{"line_number":402,"context_line":"def service_get(context, service_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ecd2ae3_ab26b195","line":399,"range":{"start_line":386,"start_character":6,"end_line":399,"end_character":1},"updated":"2025-10-09 12:02:46.000000000","message":"oh ok so you going to use the same qury filter to first get the comptue nodes and then later soft delete them after there are updated.\n\nthe priro code dit one sql statment regardless of the number of compute nodes os its O(1) time complexity\n\nyour new algorratiom does one select + n update for mapped + 1 udpate to soft delete the computes.\n\nif fell like we could merge the soft delete and the the update into one update as soft delete is setting deleted \u003d id in the db vai an update.\n\nthe problem is soft_delete is defiend in oslo db\n\nhttps://github.com/openstack/oslo.db/blob/eef6dff8e7938bc651ec7e017ed24e8097e65400/oslo_db/sqlalchemy/orm.py#L27C1-L33C1\n\n\nso if we wanted to inline this we woudl need to doe the following\n```\nfor compute_node in compute_nodes:\n  entity \u003d compute_node.column_descriptions[0][\u0027entity\u0027]\n  compute_node.update({\u0027deleted\u0027: entity.id,\n              \u0027updated_at\u0027: entity.updated_at,\n              \u0027deleted_at\u0027: timeutils.utcnow(),\n              \u0027mapped\u0027: 0},\n               synchronize_session\u003dfalse)\n```\n\ni wonder if we could do something slightly different and still do this in one call\n\n```suggestion\n        model_query(context, models.ComputeNode).\\\n            filter(sql.or_(\n                models.ComputeNode.service_id \u003d\u003d service_id,\n                models.ComputeNode.host \u003d\u003d service[\u0027host\u0027])).\\\n            update({\u0027deleted\u0027: models.ComputeNode.service_id,\n              \u0027updated_at\u0027: models.ComputeNode.updated_at,\n              \u0027deleted_at\u0027: timeutils.utcnow(),\n              \u0027mapped\u0027: 0},\n               synchronize_session\u003dfalse)\n```\n\ni have not tired that but i feel like something like that shoudl restore the O(1) performace of the orgianl approch while also ensuring the mapped filed is set.","commit_id":"a121ad81be4472a181f3c7bb51761bb6727a1f10"},{"author":{"_account_id":34378,"name":"Julien LE JEUNE","email":"julien.le-jeune@mailops.fr","username":"jlejeune"},"change_message_id":"d72d0372aa786f65ec377485e2b94e98eed97bbe","unresolved":false,"context_lines":[{"line_number":383,"context_line":"    if service.binary \u003d\u003d \u0027nova-compute\u0027:"},{"line_number":384,"context_line":"        # TODO(sbauza): Remove the service_id filter in a later release"},{"line_number":385,"context_line":"        # once we are sure that all compute nodes report the host field"},{"line_number":386,"context_line":"        query \u003d model_query(context, models.ComputeNode).\\"},{"line_number":387,"context_line":"            filter(sql.or_("},{"line_number":388,"context_line":"            models.ComputeNode.service_id \u003d\u003d service_id,"},{"line_number":389,"context_line":"            models.ComputeNode.host \u003d\u003d service[\u0027host\u0027]))"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        # NOTE(jlejeune): Make sure that the mapped field of the"},{"line_number":392,"context_line":"        # relevant compute nodes is set to 0"},{"line_number":393,"context_line":"        # See https://bugs.launchpad.net/nova/+bug/2085135."},{"line_number":394,"context_line":"        compute_nodes \u003d query.all()"},{"line_number":395,"context_line":"        for compute_node in compute_nodes:"},{"line_number":396,"context_line":"            compute_node.update({\u0027mapped\u0027: 0})"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"        query.soft_delete(synchronize_session\u003dFalse)"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":""},{"line_number":401,"context_line":"@pick_context_manager_reader"},{"line_number":402,"context_line":"def service_get(context, service_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"a0cfa0af_f6836f98","line":399,"range":{"start_line":386,"start_character":6,"end_line":399,"end_character":1},"in_reply_to":"1ecd2ae3_ab26b195","updated":"2025-10-09 13:20:00.000000000","message":"you are right, I\u0027ve updated it and tested it, it works with that","commit_id":"a121ad81be4472a181f3c7bb51761bb6727a1f10"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"91712068694d34bf11967bd937f99c3977f754f5","unresolved":true,"context_lines":[{"line_number":387,"context_line":"            filter(sql.or_("},{"line_number":388,"context_line":"                models.ComputeNode.service_id \u003d\u003d service_id,"},{"line_number":389,"context_line":"                models.ComputeNode.host \u003d\u003d service[\u0027host\u0027])).\\"},{"line_number":390,"context_line":"            soft_delete(synchronize_session\u003dFalse)"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"@pick_context_manager_reader"}],"source_content_type":"text/x-python","patch_set":9,"id":"16389570_90bca899","side":"PARENT","line":390,"updated":"2025-10-10 08:21:03.000000000","message":"I\u0027m struggling to find the definition of this piece. Could you help me out where it is defined? I guess you found it and inlined to add mapped: 0. I just want to double check the inlining","commit_id":"4073297ae3bec253ce16dae3e91392b0fed10340"},{"author":{"_account_id":34378,"name":"Julien LE JEUNE","email":"julien.le-jeune@mailops.fr","username":"jlejeune"},"change_message_id":"6cf344c6e1c1ee874b10f72090c44f38ed8a3b4f","unresolved":false,"context_lines":[{"line_number":387,"context_line":"            filter(sql.or_("},{"line_number":388,"context_line":"                models.ComputeNode.service_id \u003d\u003d service_id,"},{"line_number":389,"context_line":"                models.ComputeNode.host \u003d\u003d service[\u0027host\u0027])).\\"},{"line_number":390,"context_line":"            soft_delete(synchronize_session\u003dFalse)"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"@pick_context_manager_reader"}],"source_content_type":"text/x-python","patch_set":9,"id":"3c96e94c_a5b297b4","side":"PARENT","line":390,"in_reply_to":"16389570_90bca899","updated":"2025-10-10 09:25:26.000000000","message":"you can find the definition of soft_delete() method here: https://github.com/openstack/oslo.db/blob/master/oslo_db/sqlalchemy/orm.py#L27-L32","commit_id":"4073297ae3bec253ce16dae3e91392b0fed10340"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"92c8b62f09bbe16fa52450836e9c2ce7af6ebbea","unresolved":true,"context_lines":[{"line_number":387,"context_line":"            filter(sql.or_("},{"line_number":388,"context_line":"                models.ComputeNode.service_id \u003d\u003d service_id,"},{"line_number":389,"context_line":"                models.ComputeNode.host \u003d\u003d service[\u0027host\u0027])).\\"},{"line_number":390,"context_line":"            soft_delete(synchronize_session\u003dFalse)"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"@pick_context_manager_reader"}],"source_content_type":"text/x-python","patch_set":9,"id":"eff0e713_cd48172a","side":"PARENT","line":390,"in_reply_to":"3c96e94c_a5b297b4","updated":"2025-10-10 09:49:47.000000000","message":"ya it took me 30-60 minutes to find that orginally too.\n\ni assumed it came from sqlachamy so i went way off in the wrong direction first.","commit_id":"4073297ae3bec253ce16dae3e91392b0fed10340"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d65ce5efafff844713b5964d50fbabf526e18733","unresolved":false,"context_lines":[{"line_number":387,"context_line":"            filter(sql.or_("},{"line_number":388,"context_line":"                models.ComputeNode.service_id \u003d\u003d service_id,"},{"line_number":389,"context_line":"                models.ComputeNode.host \u003d\u003d service[\u0027host\u0027])).\\"},{"line_number":390,"context_line":"            soft_delete(synchronize_session\u003dFalse)"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"@pick_context_manager_reader"}],"source_content_type":"text/x-python","patch_set":9,"id":"7d5bbfac_a8a86ae9","side":"PARENT","line":390,"in_reply_to":"eff0e713_cd48172a","updated":"2025-10-13 08:23:48.000000000","message":"Thanks for the pointer. Inlining looks good. \n\nWould be nice to have a way to say\n```\n             filter(sql.or_(\n                 models.ComputeNode.service_id \u003d\u003d service_id,\n                 models.ComputeNode.host \u003d\u003d service[\u0027host\u0027])).\\\n             soft_delete(synchronize_session\u003dFalse).\\\n             update({\u0027mapped\u0027: 0}, synchronize_session\u003dFalse)\n```\n\nBut it is not how soft_delete is defined. So what you did is the best we can do.","commit_id":"4073297ae3bec253ce16dae3e91392b0fed10340"}]}