)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8348ed65384c04e9a90aaa34181bef5877ba152","unresolved":false,"context_lines":[{"line_number":10,"context_line":"nova create a new entry for it. For somehow, if updating comes after"},{"line_number":11,"context_line":"the instance has been deleted (its entry would be soft-deleted),"},{"line_number":12,"context_line":"nova would create a new entry for the deleted instance currently."},{"line_number":13,"context_line":"It causes instance_extra entry duplicate."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"For those deleted instance, we shouldn\u0027t create a new entry,"},{"line_number":16,"context_line":"just let\u0027s skip the \"update\"."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"3fa7e38b_3059f65a","line":13,"updated":"2019-11-06 23:04:33.000000000","message":"Is there no foreign or unique key that prevents this? I guess not:\n\nhttps://github.com/openstack/nova/blob/e79a0effc686f0d1bd2edca24ee50c10c5d6dc0a/nova/db/sqlalchemy/models.py#L379","commit_id":"816c22fd81985aa20c84184e4c222839a46e888d"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":19234,"name":"Alexey Stupnikov","email":"aleksey.stupnikov@gmail.com","username":"astupnikov"},"change_message_id":"a2b452f1fe3ee400899dee8361abfdac75ba5781","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"1978c6a9_449e55c0","updated":"2025-07-03 15:04:33.000000000","message":"Hi Dan and Stephen.\n\nI believe that I have addressed concerns that blocked a previous version of this patch from being merged + wrote a unit test to cover new behavior. Appreciate your look.","commit_id":"1910e23801787670a0ee287856aae7eb8ee558eb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fcf2dcdffa12aa378fbea479d82e18df69b141ac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"82bf341f_6614b7b6","updated":"2025-07-03 17:06:40.000000000","message":"This bug was reported in mitaka and reported as not reproducible in rocky. The age of this thing and the stuff that has changed since then completely resets any amount of inertia this had, IMHO.\n\nPlease update the attached bug with modern steps to reproduce, at the very least. I\u0027m generally quite wary of modifying the context deeply in a DB method like this, even if it is just for read_deleted\u003dTrue, so I\u0027m going to look for a lot of justification.","commit_id":"1910e23801787670a0ee287856aae7eb8ee558eb"},{"author":{"_account_id":19234,"name":"Alexey Stupnikov","email":"aleksey.stupnikov@gmail.com","username":"astupnikov"},"change_message_id":"c3e09523a5d61f41741101a9f7967c2a107daf4e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"d3af857f_0174c5c5","in_reply_to":"82bf341f_6614b7b6","updated":"2025-07-03 19:04:48.000000000","message":"Hi Dan.\n\nThank you for taking a look. I have added details to bug, it was very consistently reproduced in wallaby-based product (customer had like 100 hundreds  of undeleted instance_extra records generated each year for few years in a row). IMO this confirms that problem is still there as  wallaby is much closer to current situation than mitaka.\n\nI agree that overall it is not something very elegant to do.","commit_id":"1910e23801787670a0ee287856aae7eb8ee558eb"}],"nova/db/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8348ed65384c04e9a90aaa34181bef5877ba152","unresolved":false,"context_lines":[{"line_number":964,"context_line":"    \"\"\"Get the instance extra record"},{"line_number":965,"context_line":""},{"line_number":966,"context_line":"    :param instance_uuid: \u003d uuid of the instance tied to the topology record"},{"line_number":967,"context_line":"    :param columns: A list of the columns to load, or None for \u0027all of them\u0027"},{"line_number":968,"context_line":"    \"\"\""},{"line_number":969,"context_line":"    return IMPL.instance_extra_get_by_instance_uuid("},{"line_number":970,"context_line":"        context, instance_uuid, columns\u003dcolumns, read_deleted\u003dread_deleted)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_1085daae","line":967,"updated":"2019-11-06 23:04:33.000000000","message":"Should document the new param.","commit_id":"816c22fd81985aa20c84184e4c222839a46e888d"}],"nova/db/main/api.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9be17cf780d7bff70c019c9165ed74af870a5ef8","unresolved":true,"context_lines":[{"line_number":2567,"context_line":"    rows_updated \u003d model_query("},{"line_number":2568,"context_line":"        context,"},{"line_number":2569,"context_line":"        models.InstanceExtra,"},{"line_number":2570,"context_line":"        read_deleted\u003d\u0027yes\u0027"},{"line_number":2571,"context_line":"        ).filter_by(instance_uuid\u003dinstance_uuid).\\"},{"line_number":2572,"context_line":"        update(updates)"},{"line_number":2573,"context_line":"    # If statement below was introduced during Kilo cycle to address"}],"source_content_type":"text/x-python","patch_set":6,"id":"87a7d11e_add418f7","line":2570,"updated":"2025-07-03 20:13:41.000000000","message":"This is, IMHO, not a great way to go about trying to avoid creating these when it makes no sense to do so. If we make it here after an instance is deleted *and* purged, we\u0027ll go right back to creating a zombie record.\n\nI\u0027m not sure why we don\u0027t have a FK on instance_extra-\u003einstance, but that would really be the right solution here.\n\nEither way, I only see a few places from where this is actually called. It would probably be trivial to refactor those to call create if they really want it created after we try to update in place. From a quick look, I don\u0027t even see where we would want this magical create behavior anymore, and I wonder if it\u0027s like leftover from online data migrations in 2012 or something...","commit_id":"1910e23801787670a0ee287856aae7eb8ee558eb"},{"author":{"_account_id":19234,"name":"Alexey Stupnikov","email":"aleksey.stupnikov@gmail.com","username":"astupnikov"},"change_message_id":"b2307370c4248cd12107921a7c16ddf08116f69e","unresolved":true,"context_lines":[{"line_number":2567,"context_line":"    rows_updated \u003d model_query("},{"line_number":2568,"context_line":"        context,"},{"line_number":2569,"context_line":"        models.InstanceExtra,"},{"line_number":2570,"context_line":"        read_deleted\u003d\u0027yes\u0027"},{"line_number":2571,"context_line":"        ).filter_by(instance_uuid\u003dinstance_uuid).\\"},{"line_number":2572,"context_line":"        update(updates)"},{"line_number":2573,"context_line":"    # If statement below was introduced during Kilo cycle to address"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a6234b1_ab24f298","line":2570,"in_reply_to":"87a7d11e_add418f7","updated":"2025-07-03 20:50:30.000000000","message":"When it comes to approach, then it is coming from previous patchset\u0027s discussion. The goal is to reduce cost of call.\n\nIf statement was added to address a very specific use problem affecting inline data migrations in Kilo. I also failed to find if if statement is used anywhere intentionally as of now.\n\nWhen analyzing this, I have decided to skip dropping if statement, or changing relationships between entities because I wasn\u0027t 100% sure about impact.","commit_id":"1910e23801787670a0ee287856aae7eb8ee558eb"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"a7600858f1bded3f32d107ab1e457d474d7475f2","unresolved":false,"context_lines":[{"line_number":3065,"context_line":"@pick_context_manager_reader"},{"line_number":3066,"context_line":"def instance_extra_get_by_instance_uuid(context, instance_uuid,"},{"line_number":3067,"context_line":"                                        columns\u003dNone, read_deleted\u003dNone):"},{"line_number":3068,"context_line":"    query \u003d model_query(context, models.InstanceExtra, read_deleted\u003dread_deleted).\\"},{"line_number":3069,"context_line":"        filter_by(instance_uuid\u003dinstance_uuid)"},{"line_number":3070,"context_line":"    if columns is None:"},{"line_number":3071,"context_line":"        columns \u003d [\u0027numa_topology\u0027, \u0027pci_requests\u0027, \u0027flavor\u0027, \u0027vcpu_model\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_2b4741bf","line":3068,"range":{"start_line":3068,"start_character":0,"end_line":3068,"end_character":83},"updated":"2019-10-02 23:17:00.000000000","message":"pep8 job fails. This line should be wrapped less than 80 characters.","commit_id":"873ee4f4c7664fad85a69d46612207d522c72755"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"040dbeae1688bc26724d9d26954785383de2fed4","unresolved":false,"context_lines":[{"line_number":3065,"context_line":"@pick_context_manager_reader"},{"line_number":3066,"context_line":"def instance_extra_get_by_instance_uuid(context, instance_uuid,"},{"line_number":3067,"context_line":"                                        columns\u003dNone, read_deleted\u003dNone):"},{"line_number":3068,"context_line":"    query \u003d model_query(context, models.InstanceExtra, read_deleted\u003dread_deleted).\\"},{"line_number":3069,"context_line":"        filter_by(instance_uuid\u003dinstance_uuid)"},{"line_number":3070,"context_line":"    if columns is None:"},{"line_number":3071,"context_line":"        columns \u003d [\u0027numa_topology\u0027, \u0027pci_requests\u0027, \u0027flavor\u0027, \u0027vcpu_model\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_5d2a7b90","line":3068,"range":{"start_line":3068,"start_character":0,"end_line":3068,"end_character":83},"in_reply_to":"3fa7e38b_2b4741bf","updated":"2019-10-03 11:16:50.000000000","message":"Done","commit_id":"873ee4f4c7664fad85a69d46612207d522c72755"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"45210cea469df2331920f1e4d53e0cdb7087db32","unresolved":false,"context_lines":[{"line_number":3049,"context_line":"def instance_extra_update_by_uuid(context, instance_uuid, values):"},{"line_number":3050,"context_line":"    rows_updated \u003d model_query(context, models.InstanceExtra).\\"},{"line_number":3051,"context_line":"        filter_by(instance_uuid\u003dinstance_uuid).\\"},{"line_number":3052,"context_line":"        update(values)"},{"line_number":3053,"context_line":""},{"line_number":3054,"context_line":"    if (not rows_updated and"},{"line_number":3055,"context_line":"        not instance_extra_get_by_instance_uuid(context,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_289bc773","line":3052,"updated":"2019-11-07 16:04:50.000000000","message":"Why not just change this to read/update the deleted row? if we\u0027re going to ignore it here and then just re-create it below (if it exists), why not just update this one? If we found/updated it then cool, and if it didn\u0027t exist, rows_updated will be Falsey and we\u0027d continue with the create below using the same logic.","commit_id":"816c22fd81985aa20c84184e4c222839a46e888d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8348ed65384c04e9a90aaa34181bef5877ba152","unresolved":false,"context_lines":[{"line_number":3052,"context_line":"        update(values)"},{"line_number":3053,"context_line":""},{"line_number":3054,"context_line":"    if (not rows_updated and"},{"line_number":3055,"context_line":"        not instance_extra_get_by_instance_uuid(context,"},{"line_number":3056,"context_line":"        instance_uuid, read_deleted\u003d\u0027yes\u0027)):"},{"line_number":3057,"context_line":"        LOG.debug(\"Created instance_extra for %s\", instance_uuid)"},{"line_number":3058,"context_line":"        create_values \u003d copy.copy(values)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_90ba2a6f","line":3055,"updated":"2019-11-06 23:04:33.000000000","message":"Would be good to have a comment here for this logic.\n\nAlso, this alignment is a bit ugly - can we indent?","commit_id":"816c22fd81985aa20c84184e4c222839a46e888d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8348ed65384c04e9a90aaa34181bef5877ba152","unresolved":false,"context_lines":[{"line_number":3053,"context_line":""},{"line_number":3054,"context_line":"    if (not rows_updated and"},{"line_number":3055,"context_line":"        not instance_extra_get_by_instance_uuid(context,"},{"line_number":3056,"context_line":"        instance_uuid, read_deleted\u003d\u0027yes\u0027)):"},{"line_number":3057,"context_line":"        LOG.debug(\"Created instance_extra for %s\", instance_uuid)"},{"line_number":3058,"context_line":"        create_values \u003d copy.copy(values)"},{"line_number":3059,"context_line":"        create_values[\"instance_uuid\"] \u003d instance_uuid"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_30ab361f","line":3056,"updated":"2019-11-06 23:04:33.000000000","message":"If you want to make this as fast as possible, which you would since it\u0027s just an existence check (correct?), then you should pass columns\u003d[] so instance_extra_get_by_instance_uuid doesn\u0027t by default load those other columns that we don\u0027t care about.","commit_id":"816c22fd81985aa20c84184e4c222839a46e888d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"45210cea469df2331920f1e4d53e0cdb7087db32","unresolved":false,"context_lines":[{"line_number":3053,"context_line":""},{"line_number":3054,"context_line":"    if (not rows_updated and"},{"line_number":3055,"context_line":"        not instance_extra_get_by_instance_uuid(context,"},{"line_number":3056,"context_line":"        instance_uuid, read_deleted\u003d\u0027yes\u0027)):"},{"line_number":3057,"context_line":"        LOG.debug(\"Created instance_extra for %s\", instance_uuid)"},{"line_number":3058,"context_line":"        create_values \u003d copy.copy(values)"},{"line_number":3059,"context_line":"        create_values[\"instance_uuid\"] \u003d instance_uuid"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_088ccb33","line":3056,"in_reply_to":"3fa7e38b_30ab361f","updated":"2019-11-07 16:04:50.000000000","message":"It\u0027d be better to do something even less expensive to check I think,  but I think with my comment above we could avoid the extra DB hit anyway, if I\u0027m understanding the issue correctly.","commit_id":"816c22fd81985aa20c84184e4c222839a46e888d"}],"nova/tests/unit/db/test_db_api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8348ed65384c04e9a90aaa34181bef5877ba152","unresolved":false,"context_lines":[{"line_number":3401,"context_line":"            self.ctxt, self.instance[\u0027uuid\u0027])"},{"line_number":3402,"context_line":"        self.assertEqual(\u0027changed\u0027, inst_extra.numa_topology)"},{"line_number":3403,"context_line":""},{"line_number":3404,"context_line":"    def test_instance_extra_update_by_uuid_not_create_entry_for_deleted(self):"},{"line_number":3405,"context_line":"        @sqlalchemy_api.pick_context_manager_writer"},{"line_number":3406,"context_line":"        def test(context):"},{"line_number":3407,"context_line":"            sqlalchemy_api.model_query(context, models.InstanceExtra).\\"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_f0791ec5","line":3404,"updated":"2019-11-06 23:04:33.000000000","message":"Please add comments along with the calls in this test to make it more clear what is going on.","commit_id":"816c22fd81985aa20c84184e4c222839a46e888d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8348ed65384c04e9a90aaa34181bef5877ba152","unresolved":false,"context_lines":[{"line_number":3403,"context_line":""},{"line_number":3404,"context_line":"    def test_instance_extra_update_by_uuid_not_create_entry_for_deleted(self):"},{"line_number":3405,"context_line":"        @sqlalchemy_api.pick_context_manager_writer"},{"line_number":3406,"context_line":"        def test(context):"},{"line_number":3407,"context_line":"            sqlalchemy_api.model_query(context, models.InstanceExtra).\\"},{"line_number":3408,"context_line":"               filter_by(instance_uuid\u003dself.instance[\u0027uuid\u0027]).\\"},{"line_number":3409,"context_line":"               soft_delete()"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_706e8e01","line":3406,"range":{"start_line":3406,"start_character":12,"end_line":3406,"end_character":16},"updated":"2019-11-06 23:04:33.000000000","message":"I guess this is just copying from above, but it\u0027s not a test, so how about we call this soft_delete_instance() instead?","commit_id":"816c22fd81985aa20c84184e4c222839a46e888d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8348ed65384c04e9a90aaa34181bef5877ba152","unresolved":false,"context_lines":[{"line_number":3408,"context_line":"               filter_by(instance_uuid\u003dself.instance[\u0027uuid\u0027]).\\"},{"line_number":3409,"context_line":"               soft_delete()"},{"line_number":3410,"context_line":""},{"line_number":3411,"context_line":"        test(self.ctxt)"},{"line_number":3412,"context_line":""},{"line_number":3413,"context_line":"        inst_extra \u003d db.instance_extra_get_by_instance_uuid("},{"line_number":3414,"context_line":"            self.ctxt, self.instance[\u0027uuid\u0027], read_deleted\u003d\u0027yes\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_b04de69a","line":3411,"updated":"2019-11-06 23:04:33.000000000","message":"so we soft delete the instance created in setup","commit_id":"816c22fd81985aa20c84184e4c222839a46e888d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8348ed65384c04e9a90aaa34181bef5877ba152","unresolved":false,"context_lines":[{"line_number":3412,"context_line":""},{"line_number":3413,"context_line":"        inst_extra \u003d db.instance_extra_get_by_instance_uuid("},{"line_number":3414,"context_line":"            self.ctxt, self.instance[\u0027uuid\u0027], read_deleted\u003d\u0027yes\u0027)"},{"line_number":3415,"context_line":"        self.assertIsNotNone(inst_extra)"},{"line_number":3416,"context_line":""},{"line_number":3417,"context_line":"        db.instance_extra_update_by_uuid(self.ctxt, self.instance[\u0027uuid\u0027],"},{"line_number":3418,"context_line":"                                         {\u0027numa_topology\u0027: \u0027changed\u0027})"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_5042726a","line":3415,"updated":"2019-11-06 23:04:33.000000000","message":"this shows that we can get instance extra from the deleted instance","commit_id":"816c22fd81985aa20c84184e4c222839a46e888d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8348ed65384c04e9a90aaa34181bef5877ba152","unresolved":false,"context_lines":[{"line_number":3418,"context_line":"                                         {\u0027numa_topology\u0027: \u0027changed\u0027})"},{"line_number":3419,"context_line":"        inst_extra \u003d db.instance_extra_get_by_instance_uuid("},{"line_number":3420,"context_line":"            self.ctxt, self.instance[\u0027uuid\u0027])"},{"line_number":3421,"context_line":"        self.assertIsNone(inst_extra)"},{"line_number":3422,"context_line":""},{"line_number":3423,"context_line":"        inst_extra \u003d db.instance_extra_get_by_instance_uuid("},{"line_number":3424,"context_line":"            self.ctxt, self.instance[\u0027uuid\u0027], read_deleted\u003d\u0027yes\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_105cfa4a","line":3421,"updated":"2019-11-06 23:04:33.000000000","message":"and this shows instance_extra_update_by_uuid didn\u0027t attempt to create instance extra since the instance is soft deleted","commit_id":"816c22fd81985aa20c84184e4c222839a46e888d"}]}