)]}'
{"nova/objects/instance_mapping.py":[{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"f411d41c5c30bb704dc5c4008fc2efd026d187d5","unresolved":false,"context_lines":[{"line_number":209,"context_line":"            # user_id populated"},{"line_number":210,"context_line":"            context.session.query(api_models.InstanceMapping)"},{"line_number":211,"context_line":"            .options(joinedload(\u0027cell_mapping\u0027))"},{"line_number":212,"context_line":"            .filter_by(user_id\u003dNone)"},{"line_number":213,"context_line":"            .filter_by(cell_id\u003dcell.id)"},{"line_number":214,"context_line":"            .limit(max_count).all())"},{"line_number":215,"context_line":"        ims_by_inst \u003d {im.instance_uuid: im for im in ims}"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_34b7f1f7","line":212,"range":{"start_line":212,"start_character":12,"end_line":212,"end_character":36},"updated":"2019-02-14 19:42:12.000000000","message":"If the population is based on the value in db being NULL, and since in the previous patch of adding the field to the InstanceMapping object, the default value is nullable, this would run on an infinite loop : same mistake we had while populating queued_for_delete. We might want to add the user_id while creating the InstanceMapping object for the new instances in the previous patch like for queued_for_delete (https://review.openstack.org/#/c/633350/3/nova/objects/instance_mapping.py@112).","commit_id":"bbab98066d9783cbfe00ccbc3d41d77bc5c24232"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00c246d75a78f76bac8f311c08f9e195f029c03a","unresolved":false,"context_lines":[{"line_number":209,"context_line":"            # user_id populated"},{"line_number":210,"context_line":"            context.session.query(api_models.InstanceMapping)"},{"line_number":211,"context_line":"            .options(joinedload(\u0027cell_mapping\u0027))"},{"line_number":212,"context_line":"            .filter_by(user_id\u003dNone)"},{"line_number":213,"context_line":"            .filter_by(cell_id\u003dcell.id)"},{"line_number":214,"context_line":"            .limit(max_count).all())"},{"line_number":215,"context_line":"        ims_by_inst \u003d {im.instance_uuid: im for im in ims}"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_efc8b455","line":212,"range":{"start_line":212,"start_character":12,"end_line":212,"end_character":36},"in_reply_to":"9fdfeff1_34b7f1f7","updated":"2019-02-14 22:01:04.000000000","message":"Ah, thank you for catching this! I did spend a lot of time trying to make user_id non-nullable in InstanceMapping but realized that\u0027s impossible because InstanceMapping could exist that have not yet been scheduled to a cell (and I need cell instance to get the user_id).\n\nI didn\u0027t think of setting user_id in InstanceMapping.create because project_id is passed before creating [1] and I\u0027ll do similar for user_id. BUT, as you\u0027ve pointed out, that works OK for project_id because it\u0027s non-nullable, but for user_id, it could be a problem if any caller does not pass user_id when creating. There should definitely be a fallback to use self._context.user_id as a safety, to avoid the possibility of a never-ending online migration process.\n\n[1] https://github.com/openstack/nova/blob/a4e6340bdcc2fda8fed41abc64ea554d9a8ab713/nova/compute/api.py#L947-L949","commit_id":"bbab98066d9783cbfe00ccbc3d41d77bc5c24232"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5f3bf0baaec0701f27ba6eae1f3325e5c68f3367","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        ims_by_inst \u003d {im.instance_uuid: im for im in ims}"},{"line_number":223,"context_line":"        with nova_context.target_cell(context, cell) as cctxt:"},{"line_number":224,"context_line":"            filters \u003d {\u0027uuid\u0027: list(ims_by_inst.keys())}"},{"line_number":225,"context_line":"            instances \u003d objects.InstanceList.get_by_filters("},{"line_number":226,"context_line":"                cctxt, filters, expected_attrs\u003d[])"},{"line_number":227,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":228,"context_line":"        # and update it"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fdfeff1_2d00ed01","line":225,"range":{"start_line":225,"start_character":45,"end_line":225,"end_character":59},"updated":"2019-02-15 22:20:43.000000000","message":"This will return deleted instances by default. Do you care about those?","commit_id":"a59ec1c06ef51c2e0038354b585940643dc9d125"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"91e58b3ad9b31725f66712216af056cacf30d30c","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        ims_by_inst \u003d {im.instance_uuid: im for im in ims}"},{"line_number":223,"context_line":"        with nova_context.target_cell(context, cell) as cctxt:"},{"line_number":224,"context_line":"            filters \u003d {\u0027uuid\u0027: list(ims_by_inst.keys())}"},{"line_number":225,"context_line":"            instances \u003d objects.InstanceList.get_by_filters("},{"line_number":226,"context_line":"                cctxt, filters, expected_attrs\u003d[])"},{"line_number":227,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":228,"context_line":"        # and update it"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fdfeff1_3f3d6eba","line":225,"range":{"start_line":225,"start_character":45,"end_line":225,"end_character":59},"in_reply_to":"9fdfeff1_08195720","updated":"2019-02-16 00:42:59.000000000","message":"Do we count quota on SOFT_DELETED instances though because I think those are queued_for_delete\u003dTrue until they get reclaimed (hard deleted).\n\nhttps://github.com/openstack/nova/blob/880327cc31fea7328d23355730d5458f3b74662b/nova/compute/api.py#L2231","commit_id":"a59ec1c06ef51c2e0038354b585940643dc9d125"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"22c334e2e260ac790d9dd31c6a05021d5cde8075","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        ims_by_inst \u003d {im.instance_uuid: im for im in ims}"},{"line_number":223,"context_line":"        with nova_context.target_cell(context, cell) as cctxt:"},{"line_number":224,"context_line":"            filters \u003d {\u0027uuid\u0027: list(ims_by_inst.keys())}"},{"line_number":225,"context_line":"            instances \u003d objects.InstanceList.get_by_filters("},{"line_number":226,"context_line":"                cctxt, filters, expected_attrs\u003d[])"},{"line_number":227,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":228,"context_line":"        # and update it"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fdfeff1_08195720","line":225,"range":{"start_line":225,"start_character":45,"end_line":225,"end_character":59},"in_reply_to":"9fdfeff1_2d00ed01","updated":"2019-02-15 22:36:46.000000000","message":"No, I think I don\u0027t. I might have messed this up because I copied the populate_queued_for_delete approach via copy-paste and tweaked it. I think we probably don\u0027t want to bother migrating for deleted instances because those don\u0027t matter for quota usage counting. Similarly, I was thinking we probably don\u0027t need to migrate queued_for_delete\u003dTrue records either. The spec says we should, but thinking about it again, I can\u0027t think of why we would need to. When we count quota usage, we\u0027ll only want ones with queued_for_delete\u003dFalse.","commit_id":"a59ec1c06ef51c2e0038354b585940643dc9d125"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6641c3ce04fa30912ec0079fac9ab27431483758","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        ims_by_inst \u003d {im.instance_uuid: im for im in ims}"},{"line_number":223,"context_line":"        with nova_context.target_cell(context, cell) as cctxt:"},{"line_number":224,"context_line":"            filters \u003d {\u0027uuid\u0027: list(ims_by_inst.keys())}"},{"line_number":225,"context_line":"            instances \u003d objects.InstanceList.get_by_filters("},{"line_number":226,"context_line":"                cctxt, filters, expected_attrs\u003d[])"},{"line_number":227,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":228,"context_line":"        # and update it"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fdfeff1_1542cb3e","line":225,"range":{"start_line":225,"start_character":45,"end_line":225,"end_character":59},"in_reply_to":"9fdfeff1_3f3d6eba","updated":"2019-02-16 06:00:54.000000000","message":"We don\u0027t count SOFT_DELETED instances when counting quota usage [1][2]. Which makes sense, I think, because from a user perspective, they\u0027ve deleted the instance, so it wouldn\u0027t seem correct to count the instance against their quota limit.\n\nSo, that works out, for counting instance mappings with queued_for_delete\u003dFalse for quota usage.\n\n[1] https://github.com/openstack/nova/blob/a2d3ca91c3ae6ed435ad6c6525f77bdccb49f9ed/nova/quota.py#L1181\n[2] https://github.com/openstack/nova/blob/163b074982d4af9295b57383c9f817a7d24b193a/nova/objects/instance.py#L1540","commit_id":"a59ec1c06ef51c2e0038354b585940643dc9d125"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7f89fac5f7eacd98e56548d33f0d6c51e8faf9f0","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            context.session.add(im)"},{"line_number":234,"context_line":"            processed +\u003d 1"},{"line_number":235,"context_line":"            # DEBUGAROO"},{"line_number":236,"context_line":"            print(\u0027setting user_id\u003d%s for instance mapping\u003d%s with cell\u003d%s \u0027"},{"line_number":237,"context_line":"                  \u0027for instance\u003d%s\u0027 % (instance.user_id, im.id, cell.uuid,"},{"line_number":238,"context_line":"                                       instance.uuid))"},{"line_number":239,"context_line":"        max_count -\u003d len(ims)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_4ae4040f","line":236,"updated":"2019-02-19 14:36:37.000000000","message":"Looks like you\u0027re picking up a marker instance record from an earlier online data migration:\n\nhttp://logs.openstack.org/51/633351/6/check/neutron-grenade/0d926dc/logs/grenade.sh.txt.gz#_2019-02-19_03_00_17_540\n\n2019-02-19 03:00:17.540 | InstanceList(objects\u003d[Instance(00000000-0000-0000-0000-000000000000)])\n2019-02-19 03:00:17.540 | setting user_id\u003dNone for instance mapping\u003d25 with cell\u003dc497f098-f0cd-44f2-a5a3-7c5eb3562142 for instance\u003d00000000-0000-0000-0000-000000000000\n2019-02-19 03:00:17.542 | 1 rows matched query populate_user_id, 1 migrated\n\nSo I guess you need to skip any instances that don\u0027t have a user_id set (or any with the stub uuid).","commit_id":"65e7b38d31af94c1e9c12a9cda005e8e9cc5213d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8be8cad4ecc0cb7e24aabd9eadf61f9ac434da34","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            context.session.add(im)"},{"line_number":234,"context_line":"            processed +\u003d 1"},{"line_number":235,"context_line":"            # DEBUGAROO"},{"line_number":236,"context_line":"            print(\u0027setting user_id\u003d%s for instance mapping\u003d%s with cell\u003d%s \u0027"},{"line_number":237,"context_line":"                  \u0027for instance\u003d%s\u0027 % (instance.user_id, im.id, cell.uuid,"},{"line_number":238,"context_line":"                                       instance.uuid))"},{"line_number":239,"context_line":"        max_count -\u003d len(ims)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_4b5e4113","line":236,"in_reply_to":"9fdfeff1_1edf1ad7","updated":"2019-02-19 18:50:49.000000000","message":"Thank you both for the help and explanation on this -- you have saved the day!","commit_id":"65e7b38d31af94c1e9c12a9cda005e8e9cc5213d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"867e6b1e521900fb3a0902340baf8f91dd04851f","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            context.session.add(im)"},{"line_number":234,"context_line":"            processed +\u003d 1"},{"line_number":235,"context_line":"            # DEBUGAROO"},{"line_number":236,"context_line":"            print(\u0027setting user_id\u003d%s for instance mapping\u003d%s with cell\u003d%s \u0027"},{"line_number":237,"context_line":"                  \u0027for instance\u003d%s\u0027 % (instance.user_id, im.id, cell.uuid,"},{"line_number":238,"context_line":"                                       instance.uuid))"},{"line_number":239,"context_line":"        max_count -\u003d len(ims)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_aab490d2","line":236,"in_reply_to":"9fdfeff1_4ae4040f","updated":"2019-02-19 14:44:52.000000000","message":"What I\u0027m not finding is the data migration that added an instance mapping with that same instance_uuid. But we would have to have one to get the instance record based on that UUID right?","commit_id":"65e7b38d31af94c1e9c12a9cda005e8e9cc5213d"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"5ab069254d87e104420d82c95be7f51901ea89cc","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            context.session.add(im)"},{"line_number":234,"context_line":"            processed +\u003d 1"},{"line_number":235,"context_line":"            # DEBUGAROO"},{"line_number":236,"context_line":"            print(\u0027setting user_id\u003d%s for instance mapping\u003d%s with cell\u003d%s \u0027"},{"line_number":237,"context_line":"                  \u0027for instance\u003d%s\u0027 % (instance.user_id, im.id, cell.uuid,"},{"line_number":238,"context_line":"                                       instance.uuid))"},{"line_number":239,"context_line":"        max_count -\u003d len(ims)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_1edf1ad7","line":236,"in_reply_to":"9fdfeff1_6339f33d","updated":"2019-02-19 16:40:54.000000000","message":"ok so since the fake instance created during the \"fill_virtual_interface_list\" is destroyed, the queued_for_delete will at least complete but looks like for this migration if the user does the following:\n\n1) run the \"fill_virtual_interface_list\" migration.\n2) run \"nova-manage cell_v2 map_instances\".\n\nwe will get an infinite loop, unless that fake instance gets archived first. I am not sure if that is what is happening in the grenade, but its a possibility nonetheless.","commit_id":"65e7b38d31af94c1e9c12a9cda005e8e9cc5213d"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1e479d3f9d919a6b1c2de9821511dbad086f2252","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            context.session.add(im)"},{"line_number":234,"context_line":"            processed +\u003d 1"},{"line_number":235,"context_line":"            # DEBUGAROO"},{"line_number":236,"context_line":"            print(\u0027setting user_id\u003d%s for instance mapping\u003d%s with cell\u003d%s \u0027"},{"line_number":237,"context_line":"                  \u0027for instance\u003d%s\u0027 % (instance.user_id, im.id, cell.uuid,"},{"line_number":238,"context_line":"                                       instance.uuid))"},{"line_number":239,"context_line":"        max_count -\u003d len(ims)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_6339f33d","line":236,"in_reply_to":"9fdfeff1_aab490d2","updated":"2019-02-19 16:07:51.000000000","message":"I am not sure of the order of happenings in the grenade job, but is there a chance that the \"nova-manage cell_v2 map_instances\" gets run in between? Because we would have a marker there with a NULL user_id.\n\nAlso irrespective of that, I think its a tricky situation since on running map_instances after this \"fill_virtual_interface_list\" migration (which leaves behind a persistent marker in each cell) it creates an instance_mapping for that deleted marker instance, \n +---------------------+---------------------+----+--------------------------------------+---------+--------------------------------------+-------------------+\n | created_at          | updated_at          | id | instance_uuid                        | cell_id | project_id                           | queued_for_delete |\n +---------------------+---------------------+----+--------------------------------------+---------+--------------------------------------+-------------------+\n | 2019-02-19 15:50:14 | NULL                | 25 | 00000000-0000-0000-0000-000000000000 |       2 | 00000000-0000-0000-0000-000000000000 |              NULL |\n | 2019-02-19 15:50:14 | NULL                | 26 | 00000000 0000 0000 0000 000000000000 |    NULL | INSTANCE_MIGRATION_MARKER            |              NULL |\n +---------------------+---------------------+----+--------------------------------------+---------+--------------------------------------+-------------------+\n\n\nEven if the map_instances marker would go away, there is a chance of these instance_mappings having NULL queued_for_delete and user_id fields. Not sure if it should be considered.","commit_id":"65e7b38d31af94c1e9c12a9cda005e8e9cc5213d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6ac1f536d2786b636391d1f98899dcc388883bd0","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"@db_api.api_context_manager.reader"},{"line_number":211,"context_line":"def user_id_populated(context, project_id\u003dNone):"},{"line_number":212,"context_line":"    \"\"\"Determine whether user_id is set for non-deleted instances."},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    This will be used to determine whether we need to fall back on"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_ec7f67dc","line":211,"updated":"2019-02-27 21:00:17.000000000","message":"This seems out of place in this change with the online data migration. Would have probably been better to split this out to either the change that needs to use it or right before the change that needs to use it, but separate from the data migration change since nothing in here is using it.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"39c8d2ab248c5f3f3ed23c5ff247e3c0cca61a27","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"@db_api.api_context_manager.reader"},{"line_number":211,"context_line":"def user_id_populated(context, project_id\u003dNone):"},{"line_number":212,"context_line":"    \"\"\"Determine whether user_id is set for non-deleted instances."},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    This will be used to determine whether we need to fall back on"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_76a9017a","line":211,"in_reply_to":"9fdfeff1_ec7f67dc","updated":"2019-02-27 23:11:25.000000000","message":"That\u0027s fair enough. I went back and forth thinking where to add it. I\u0027ll move it to the patch where it\u0027s used.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6ac1f536d2786b636391d1f98899dcc388883bd0","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        filter_by(queued_for_delete\u003dFalse)"},{"line_number":233,"context_line":"    if project_id:"},{"line_number":234,"context_line":"        query \u003d query.filter_by(project_id\u003dproject_id)"},{"line_number":235,"context_line":"    result \u003d query.first()"},{"line_number":236,"context_line":"    return int(result[0] or 0) \u003d\u003d 0"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_6c4cd7c7","line":235,"range":{"start_line":235,"start_character":19,"end_line":235,"end_character":24},"updated":"2019-02-27 21:00:17.000000000","message":"Can\u0027t you use scalar() here to clean this up a bit? I\u0027ve used that in nova-status upgrade checks:\n\nhttps://github.com/openstack/nova/blob/eb93d0cffd11fcfca97b3d4679a0043142a5d998/nova/cmd/status.py#L85","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"39c8d2ab248c5f3f3ed23c5ff247e3c0cca61a27","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        filter_by(queued_for_delete\u003dFalse)"},{"line_number":233,"context_line":"    if project_id:"},{"line_number":234,"context_line":"        query \u003d query.filter_by(project_id\u003dproject_id)"},{"line_number":235,"context_line":"    result \u003d query.first()"},{"line_number":236,"context_line":"    return int(result[0] or 0) \u003d\u003d 0"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_3615b99c","line":235,"range":{"start_line":235,"start_character":19,"end_line":235,"end_character":24},"in_reply_to":"9fdfeff1_6c4cd7c7","updated":"2019-02-27 23:11:25.000000000","message":"Hm, based on your example, it should work. I did this based on older code doing func.sum and func.count and then coincidentally, recently [1], I learned that the int cast was probably done because sqlalchemy will return a Decimal object that needs to be cast to int. Maybe that\u0027s only for func.sum though, and not func.count. I\u0027ll try scalar().\n\n[1] https://review.openstack.org/639216","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"28a08097f1f2ba9af727e95f6e9ca8b3e9af04b5","unresolved":false,"context_lines":[{"line_number":238,"context_line":""},{"line_number":239,"context_line":"@db_api.api_context_manager.writer"},{"line_number":240,"context_line":"def populate_user_id(context, max_count):"},{"line_number":241,"context_line":"    cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":242,"context_line":"    processed \u003d 0"},{"line_number":243,"context_line":"    for cell in cells:"},{"line_number":244,"context_line":"        ims \u003d ("}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_60edbb23","line":241,"updated":"2019-02-27 21:17:44.000000000","message":"Why not start by iterating instance mappings that need to be migrated? Otherwise if I\u0027m CERN with 70+ cells and everything in cells 1-50 are migrated, I have to do the query for the first 50 cells to start getting anything to process, right?","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"39c8d2ab248c5f3f3ed23c5ff247e3c0cca61a27","unresolved":false,"context_lines":[{"line_number":238,"context_line":""},{"line_number":239,"context_line":"@db_api.api_context_manager.writer"},{"line_number":240,"context_line":"def populate_user_id(context, max_count):"},{"line_number":241,"context_line":"    cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":242,"context_line":"    processed \u003d 0"},{"line_number":243,"context_line":"    for cell in cells:"},{"line_number":244,"context_line":"        ims \u003d ("}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_76e1019b","line":241,"in_reply_to":"9fdfeff1_60edbb23","updated":"2019-02-27 23:11:25.000000000","message":"As discussed on IRC today, I will change this.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6ac1f536d2786b636391d1f98899dcc388883bd0","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            # If we don\u0027t migrate soft-deleted instances now, we wouldn\u0027t"},{"line_number":250,"context_line":"            # be able to retire this migration code later."},{"line_number":251,"context_line":"            context.session.query(api_models.InstanceMapping)"},{"line_number":252,"context_line":"            .options(joinedload(\u0027cell_mapping\u0027))"},{"line_number":253,"context_line":"            .filter_by(user_id\u003dNone)"},{"line_number":254,"context_line":"            .filter_by(cell_id\u003dcell.id)"},{"line_number":255,"context_line":"            .limit(max_count).all())"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_ac7e3f16","line":252,"range":{"start_line":252,"start_character":12,"end_line":252,"end_character":48},"updated":"2019-02-27 21:00:17.000000000","message":"Why do we need this? I don\u0027t see im.cell_mapping used below. I know this data migration was based on populate_queued_for_delete but I don\u0027t think it was needed there either.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"7a8f4912f410cdd65537b5daf1ad97b976d2e9f6","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            # If we don\u0027t migrate soft-deleted instances now, we wouldn\u0027t"},{"line_number":250,"context_line":"            # be able to retire this migration code later."},{"line_number":251,"context_line":"            context.session.query(api_models.InstanceMapping)"},{"line_number":252,"context_line":"            .options(joinedload(\u0027cell_mapping\u0027))"},{"line_number":253,"context_line":"            .filter_by(user_id\u003dNone)"},{"line_number":254,"context_line":"            .filter_by(cell_id\u003dcell.id)"},{"line_number":255,"context_line":"            .limit(max_count).all())"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_10ae302f","line":252,"range":{"start_line":252,"start_character":12,"end_line":252,"end_character":48},"in_reply_to":"9fdfeff1_92723a12","updated":"2019-03-04 09:56:12.000000000","message":"\u003e \u003e \u003e Why do we need this? I don\u0027t see im.cell_mapping used below. I\n \u003e \u003e know\n \u003e \u003e \u003e this data migration was based on populate_queued_for_delete but\n \u003e I\n \u003e \u003e \u003e don\u0027t think it was needed there either.\n \u003e \u003e\n \u003e \u003e it was needed there because we filter based on cell.id that has\n \u003e to\n \u003e \u003e be joined loaded, it wouldn\u0027t work otherwise.\n \u003e \n \u003e The cell_id is on the instance_mappings record so that should be\n \u003e fine. Anyway, I removed it and it didn\u0027t cause any problems:\n \u003e \n \u003e https://review.openstack.org/#/c/639840/\n\nyikes yea I thought we used the uuid :/ got confused. thanks for the perf patch.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"4dd4076739cfd3e653e303f9c53f00420a0e240c","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            # If we don\u0027t migrate soft-deleted instances now, we wouldn\u0027t"},{"line_number":250,"context_line":"            # be able to retire this migration code later."},{"line_number":251,"context_line":"            context.session.query(api_models.InstanceMapping)"},{"line_number":252,"context_line":"            .options(joinedload(\u0027cell_mapping\u0027))"},{"line_number":253,"context_line":"            .filter_by(user_id\u003dNone)"},{"line_number":254,"context_line":"            .filter_by(cell_id\u003dcell.id)"},{"line_number":255,"context_line":"            .limit(max_count).all())"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_d806cf86","line":252,"range":{"start_line":252,"start_character":12,"end_line":252,"end_character":48},"in_reply_to":"9fdfeff1_ac7e3f16","updated":"2019-02-28 10:27:40.000000000","message":"\u003e Why do we need this? I don\u0027t see im.cell_mapping used below. I know\n \u003e this data migration was based on populate_queued_for_delete but I\n \u003e don\u0027t think it was needed there either.\n\nit was needed there because we filter based on cell.id that has to be joined loaded, it wouldn\u0027t work otherwise.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"39c8d2ab248c5f3f3ed23c5ff247e3c0cca61a27","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            # If we don\u0027t migrate soft-deleted instances now, we wouldn\u0027t"},{"line_number":250,"context_line":"            # be able to retire this migration code later."},{"line_number":251,"context_line":"            context.session.query(api_models.InstanceMapping)"},{"line_number":252,"context_line":"            .options(joinedload(\u0027cell_mapping\u0027))"},{"line_number":253,"context_line":"            .filter_by(user_id\u003dNone)"},{"line_number":254,"context_line":"            .filter_by(cell_id\u003dcell.id)"},{"line_number":255,"context_line":"            .limit(max_count).all())"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_76fa6143","line":252,"range":{"start_line":252,"start_character":12,"end_line":252,"end_character":48},"in_reply_to":"9fdfeff1_ac7e3f16","updated":"2019-02-27 23:11:25.000000000","message":"Yeah, I messed up, I just copied it.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1033d17765613abe0b550bb70cba56520aee9359","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            # If we don\u0027t migrate soft-deleted instances now, we wouldn\u0027t"},{"line_number":250,"context_line":"            # be able to retire this migration code later."},{"line_number":251,"context_line":"            context.session.query(api_models.InstanceMapping)"},{"line_number":252,"context_line":"            .options(joinedload(\u0027cell_mapping\u0027))"},{"line_number":253,"context_line":"            .filter_by(user_id\u003dNone)"},{"line_number":254,"context_line":"            .filter_by(cell_id\u003dcell.id)"},{"line_number":255,"context_line":"            .limit(max_count).all())"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_92723a12","line":252,"range":{"start_line":252,"start_character":12,"end_line":252,"end_character":48},"in_reply_to":"9fdfeff1_d806cf86","updated":"2019-03-01 20:54:03.000000000","message":"\u003e \u003e Why do we need this? I don\u0027t see im.cell_mapping used below. I\n \u003e know\n \u003e \u003e this data migration was based on populate_queued_for_delete but I\n \u003e \u003e don\u0027t think it was needed there either.\n \u003e \n \u003e it was needed there because we filter based on cell.id that has to\n \u003e be joined loaded, it wouldn\u0027t work otherwise.\n\nThe cell_id is on the instance_mappings record so that should be fine. Anyway, I removed it and it didn\u0027t cause any problems:\n\nhttps://review.openstack.org/#/c/639840/","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cef468fb8f79d0de966e96ad103351ecf76dcaa0","unresolved":false,"context_lines":[{"line_number":253,"context_line":"            .filter_by(user_id\u003dNone)"},{"line_number":254,"context_line":"            .filter_by(cell_id\u003dcell.id)"},{"line_number":255,"context_line":"            .limit(max_count).all())"},{"line_number":256,"context_line":"        ims_by_inst \u003d {im.instance_uuid: im for im in ims}"},{"line_number":257,"context_line":"        with nova_context.target_cell(context, cell) as cctxt:"},{"line_number":258,"context_line":"            # We need to migrate soft-deleted instances because they could be"},{"line_number":259,"context_line":"            # restored at any time in the future, preventing us from being able"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_a05ba31f","line":256,"range":{"start_line":256,"start_character":8,"end_line":256,"end_character":58},"updated":"2019-02-27 21:14:12.000000000","message":"If this is empty we can just continue, right? We don\u0027t need to make a query to the cell database for a bunch of instances we\u0027re not going to use.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"39c8d2ab248c5f3f3ed23c5ff247e3c0cca61a27","unresolved":false,"context_lines":[{"line_number":253,"context_line":"            .filter_by(user_id\u003dNone)"},{"line_number":254,"context_line":"            .filter_by(cell_id\u003dcell.id)"},{"line_number":255,"context_line":"            .limit(max_count).all())"},{"line_number":256,"context_line":"        ims_by_inst \u003d {im.instance_uuid: im for im in ims}"},{"line_number":257,"context_line":"        with nova_context.target_cell(context, cell) as cctxt:"},{"line_number":258,"context_line":"            # We need to migrate soft-deleted instances because they could be"},{"line_number":259,"context_line":"            # restored at any time in the future, preventing us from being able"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_16eaf572","line":256,"range":{"start_line":256,"start_character":8,"end_line":256,"end_character":58},"in_reply_to":"9fdfeff1_a05ba31f","updated":"2019-02-27 23:11:25.000000000","message":"True.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6ac1f536d2786b636391d1f98899dcc388883bd0","unresolved":false,"context_lines":[{"line_number":255,"context_line":"            .limit(max_count).all())"},{"line_number":256,"context_line":"        ims_by_inst \u003d {im.instance_uuid: im for im in ims}"},{"line_number":257,"context_line":"        with nova_context.target_cell(context, cell) as cctxt:"},{"line_number":258,"context_line":"            # We need to migrate soft-deleted instances because they could be"},{"line_number":259,"context_line":"            # restored at any time in the future, preventing us from being able"},{"line_number":260,"context_line":"            # to remove any other interim online data migration code we have,"},{"line_number":261,"context_line":"            # if we don\u0027t migrate them here."}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_aca39f9d","line":258,"range":{"start_line":258,"start_character":33,"end_line":258,"end_character":45},"updated":"2019-02-27 21:00:17.000000000","message":"By soft-deleted here I assume you mean with status SOFT_DELETED, not instances records whose deleted value is !\u003d 0.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"39c8d2ab248c5f3f3ed23c5ff247e3c0cca61a27","unresolved":false,"context_lines":[{"line_number":255,"context_line":"            .limit(max_count).all())"},{"line_number":256,"context_line":"        ims_by_inst \u003d {im.instance_uuid: im for im in ims}"},{"line_number":257,"context_line":"        with nova_context.target_cell(context, cell) as cctxt:"},{"line_number":258,"context_line":"            # We need to migrate soft-deleted instances because they could be"},{"line_number":259,"context_line":"            # restored at any time in the future, preventing us from being able"},{"line_number":260,"context_line":"            # to remove any other interim online data migration code we have,"},{"line_number":261,"context_line":"            # if we don\u0027t migrate them here."}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_f6e651b0","line":258,"range":{"start_line":258,"start_character":33,"end_line":258,"end_character":45},"in_reply_to":"9fdfeff1_aca39f9d","updated":"2019-02-27 23:11:25.000000000","message":"Correct.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6ac1f536d2786b636391d1f98899dcc388883bd0","unresolved":false,"context_lines":[{"line_number":259,"context_line":"            # restored at any time in the future, preventing us from being able"},{"line_number":260,"context_line":"            # to remove any other interim online data migration code we have,"},{"line_number":261,"context_line":"            # if we don\u0027t migrate them here."},{"line_number":262,"context_line":"            # NOTE: it\u0027s not possible to query only for soft-deleted instances."},{"line_number":263,"context_line":"            # We must query for both deleted and soft-deleted instances."},{"line_number":264,"context_line":"            filters \u003d {\u0027uuid\u0027: list(ims_by_inst.keys())}"},{"line_number":265,"context_line":"            instances \u003d objects.InstanceList.get_by_filters("},{"line_number":266,"context_line":"                cctxt, filters, expected_attrs\u003d[])"},{"line_number":267,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":268,"context_line":"        # and update it"},{"line_number":269,"context_line":"        for instance in instances:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_ac3f1f23","line":266,"range":{"start_line":262,"start_character":12,"end_line":266,"end_character":50},"updated":"2019-02-27 21:00:17.000000000","message":"I\u0027m a little lost here. You want to migrate (a) instance mappings for SOFT_DELETED instances (which can be restored with the restore server action API), but you also want to migrate (b) mappings for instances that are not deleted (soft or hard). Looking at:\n\nhttps://github.com/openstack/nova/blob/eb93d0cffd11fcfca97b3d4679a0043142a5d998/nova/db/sqlalchemy/api.py#L2136L2161\n\nYou can pass both a \"deleted\" and \"soft_deleted\" filter, but it looks like what you\u0027re saying is you can\u0027t do something like:\n\ndeleted\u003dFalse\nsoft_deleted\u003dTrue\n\nBecause then you\u0027ll only get back SOFT_DELETED instances, i.e. (a) above but not (b).\n\nIs that correct? So you just get them all, even if they are otherwise hard deleted.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"4dd4076739cfd3e653e303f9c53f00420a0e240c","unresolved":false,"context_lines":[{"line_number":259,"context_line":"            # restored at any time in the future, preventing us from being able"},{"line_number":260,"context_line":"            # to remove any other interim online data migration code we have,"},{"line_number":261,"context_line":"            # if we don\u0027t migrate them here."},{"line_number":262,"context_line":"            # NOTE: it\u0027s not possible to query only for soft-deleted instances."},{"line_number":263,"context_line":"            # We must query for both deleted and soft-deleted instances."},{"line_number":264,"context_line":"            filters \u003d {\u0027uuid\u0027: list(ims_by_inst.keys())}"},{"line_number":265,"context_line":"            instances \u003d objects.InstanceList.get_by_filters("},{"line_number":266,"context_line":"                cctxt, filters, expected_attrs\u003d[])"},{"line_number":267,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":268,"context_line":"        # and update it"},{"line_number":269,"context_line":"        for instance in instances:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_9d8039f9","line":266,"range":{"start_line":262,"start_character":12,"end_line":266,"end_character":50},"in_reply_to":"9fdfeff1_768fc1cc","updated":"2019-02-28 10:27:40.000000000","message":"\u003e Right. Based on the code, it won\u0027t look at the \u0027soft_deleted\u0027\n \u003e filter unless the \u0027deleted\u0027 filter is also True:\n \u003e \n \u003e https://github.com/openstack/nova/blob/eb93d0cffd11fcfca97b3d4679a0043142a5d998/nova/db/sqlalchemy/api.py#L2136-L2141\n\nthis is unfortunate :(","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"39c8d2ab248c5f3f3ed23c5ff247e3c0cca61a27","unresolved":false,"context_lines":[{"line_number":259,"context_line":"            # restored at any time in the future, preventing us from being able"},{"line_number":260,"context_line":"            # to remove any other interim online data migration code we have,"},{"line_number":261,"context_line":"            # if we don\u0027t migrate them here."},{"line_number":262,"context_line":"            # NOTE: it\u0027s not possible to query only for soft-deleted instances."},{"line_number":263,"context_line":"            # We must query for both deleted and soft-deleted instances."},{"line_number":264,"context_line":"            filters \u003d {\u0027uuid\u0027: list(ims_by_inst.keys())}"},{"line_number":265,"context_line":"            instances \u003d objects.InstanceList.get_by_filters("},{"line_number":266,"context_line":"                cctxt, filters, expected_attrs\u003d[])"},{"line_number":267,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":268,"context_line":"        # and update it"},{"line_number":269,"context_line":"        for instance in instances:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_768fc1cc","line":266,"range":{"start_line":262,"start_character":12,"end_line":266,"end_character":50},"in_reply_to":"9fdfeff1_ac3f1f23","updated":"2019-02-27 23:11:25.000000000","message":"Right. Based on the code, it won\u0027t look at the \u0027soft_deleted\u0027 filter unless the \u0027deleted\u0027 filter is also True:\n\nhttps://github.com/openstack/nova/blob/eb93d0cffd11fcfca97b3d4679a0043142a5d998/nova/db/sqlalchemy/api.py#L2136-L2141","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6ac1f536d2786b636391d1f98899dcc388883bd0","unresolved":false,"context_lines":[{"line_number":272,"context_line":"            # else we could get into an infinite migration loop"},{"line_number":273,"context_line":"            if instance.user_id is None:"},{"line_number":274,"context_line":"                continue"},{"line_number":275,"context_line":"            # Skip any \u0027deleted\u0027 instance because they cannot be restored (only"},{"line_number":276,"context_line":"            # soft-deleted instances can be restored)."},{"line_number":277,"context_line":"            if instance.deleted:"},{"line_number":278,"context_line":"                continue"},{"line_number":279,"context_line":"            im \u003d ims_by_inst[instance.uuid]"},{"line_number":280,"context_line":"            im.user_id \u003d instance.user_id"},{"line_number":281,"context_line":"            context.session.add(im)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_ac287fe1","line":278,"range":{"start_line":275,"start_character":12,"end_line":278,"end_character":24},"updated":"2019-02-27 21:00:17.000000000","message":"OK yeah based on my question above I was hoping I\u0027d see this.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"4dd4076739cfd3e653e303f9c53f00420a0e240c","unresolved":false,"context_lines":[{"line_number":207,"context_line":"    cms_by_id \u003d {cell.id: cell for cell in cells}"},{"line_number":208,"context_line":"    done \u003d 0"},{"line_number":209,"context_line":"    ims \u003d ("},{"line_number":210,"context_line":"        # Get a list of instance mappings for this cell which do not have"},{"line_number":211,"context_line":"        # user_id populated. We need to include records with"},{"line_number":212,"context_line":"        # queued_for_delete\u003dTrue because they include soft-deleted"},{"line_number":213,"context_line":"        # instances, which could be restored at any time in the future."}],"source_content_type":"text/x-python","patch_set":15,"id":"9fdfeff1_3dc10d4f","line":210,"range":{"start_line":210,"start_character":42,"end_line":210,"end_character":55},"updated":"2019-02-28 10:27:40.000000000","message":"nit: needs to be removed since the logic was changed.","commit_id":"65910122ef5d66d6804c3138f16f7e989be162ee"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"4c8103b0d88a67f6a4a0f718c614e52f06fa9deb","unresolved":false,"context_lines":[{"line_number":244,"context_line":"            if instance.user_id is None:"},{"line_number":245,"context_line":"                done +\u003d 1"},{"line_number":246,"context_line":"                continue"},{"line_number":247,"context_line":"            # Ignore any \u0027deleted\u0027 instance because they cannot be restored"},{"line_number":248,"context_line":"            # (only soft-deleted instances can be restored)."},{"line_number":249,"context_line":"            if instance.deleted:"},{"line_number":250,"context_line":"                done +\u003d 1"},{"line_number":251,"context_line":"                continue"},{"line_number":252,"context_line":"            im \u003d ims_by_inst_uuid[instance.uuid]"},{"line_number":253,"context_line":"            im.user_id \u003d instance.user_id"},{"line_number":254,"context_line":"            context.session.add(im)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9fdfeff1_127753a2","line":251,"range":{"start_line":247,"start_character":12,"end_line":251,"end_character":24},"updated":"2019-02-28 11:44:57.000000000","message":"maybe this is why its on infinite loop ? we just ignore the updation of those mappings though we count them as migrated and they keep appearing in the next run ?","commit_id":"65910122ef5d66d6804c3138f16f7e989be162ee"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"6099cc383b3d346fb1a64d776ec97d321bb74270","unresolved":false,"context_lines":[{"line_number":244,"context_line":"            if instance.user_id is None:"},{"line_number":245,"context_line":"                done +\u003d 1"},{"line_number":246,"context_line":"                continue"},{"line_number":247,"context_line":"            # Ignore any \u0027deleted\u0027 instance because they cannot be restored"},{"line_number":248,"context_line":"            # (only soft-deleted instances can be restored)."},{"line_number":249,"context_line":"            if instance.deleted:"},{"line_number":250,"context_line":"                done +\u003d 1"},{"line_number":251,"context_line":"                continue"},{"line_number":252,"context_line":"            im \u003d ims_by_inst_uuid[instance.uuid]"},{"line_number":253,"context_line":"            im.user_id \u003d instance.user_id"},{"line_number":254,"context_line":"            context.session.add(im)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9fdfeff1_52515bd8","line":251,"range":{"start_line":247,"start_character":12,"end_line":251,"end_character":24},"updated":"2019-02-28 11:52:43.000000000","message":"seems true, maybe we don\u0027t need to ignore these, is the consideration tha there could tones of delted instances and thats going to take alot of time?","commit_id":"65910122ef5d66d6804c3138f16f7e989be162ee"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"03fe48731b47a8c9684207aba4c938d7a338c333","unresolved":false,"context_lines":[{"line_number":244,"context_line":"            if instance.user_id is None:"},{"line_number":245,"context_line":"                done +\u003d 1"},{"line_number":246,"context_line":"                continue"},{"line_number":247,"context_line":"            # Ignore any \u0027deleted\u0027 instance because they cannot be restored"},{"line_number":248,"context_line":"            # (only soft-deleted instances can be restored)."},{"line_number":249,"context_line":"            if instance.deleted:"},{"line_number":250,"context_line":"                done +\u003d 1"},{"line_number":251,"context_line":"                continue"},{"line_number":252,"context_line":"            im \u003d ims_by_inst_uuid[instance.uuid]"},{"line_number":253,"context_line":"            im.user_id \u003d instance.user_id"},{"line_number":254,"context_line":"            context.session.add(im)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9fdfeff1_b21d2710","line":251,"range":{"start_line":247,"start_character":12,"end_line":251,"end_character":24},"in_reply_to":"9fdfeff1_52515bd8","updated":"2019-02-28 11:55:39.000000000","message":"\u003e seems true, maybe we don\u0027t need to ignore these, is the\n \u003e consideration tha there could tones of delted instances and thats\n \u003e going to take alot of time?\n\nyea exactly that\u0027s the reasoning and that there is no real reason for updating the deleted instances since they will be archived soon anyways. But since this is causing too much trouble with the infinite loop I guess we should just take the perf thing out of the picture and go ahead and update all mappings.","commit_id":"65910122ef5d66d6804c3138f16f7e989be162ee"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"387f5e4c8d61b15f96c20ea997bb88fd4c79787f","unresolved":false,"context_lines":[{"line_number":244,"context_line":"            if instance.user_id is None:"},{"line_number":245,"context_line":"                done +\u003d 1"},{"line_number":246,"context_line":"                continue"},{"line_number":247,"context_line":"            # Ignore any \u0027deleted\u0027 instance because they cannot be restored"},{"line_number":248,"context_line":"            # (only soft-deleted instances can be restored)."},{"line_number":249,"context_line":"            if instance.deleted:"},{"line_number":250,"context_line":"                done +\u003d 1"},{"line_number":251,"context_line":"                continue"},{"line_number":252,"context_line":"            im \u003d ims_by_inst_uuid[instance.uuid]"},{"line_number":253,"context_line":"            im.user_id \u003d instance.user_id"},{"line_number":254,"context_line":"            context.session.add(im)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9fdfeff1_b2f447d0","line":251,"range":{"start_line":247,"start_character":12,"end_line":251,"end_character":24},"in_reply_to":"9fdfeff1_52515bd8","updated":"2019-02-28 11:54:52.000000000","message":"Maybe we could add another field indicating that this one has been handled and use that one as filter too.","commit_id":"65910122ef5d66d6804c3138f16f7e989be162ee"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9107df04c324b61ae965b05adcf815a7aff94dd0","unresolved":false,"context_lines":[{"line_number":244,"context_line":"            if instance.user_id is None:"},{"line_number":245,"context_line":"                done +\u003d 1"},{"line_number":246,"context_line":"                continue"},{"line_number":247,"context_line":"            # Ignore any \u0027deleted\u0027 instance because they cannot be restored"},{"line_number":248,"context_line":"            # (only soft-deleted instances can be restored)."},{"line_number":249,"context_line":"            if instance.deleted:"},{"line_number":250,"context_line":"                done +\u003d 1"},{"line_number":251,"context_line":"                continue"},{"line_number":252,"context_line":"            im \u003d ims_by_inst_uuid[instance.uuid]"},{"line_number":253,"context_line":"            im.user_id \u003d instance.user_id"},{"line_number":254,"context_line":"            context.session.add(im)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9fdfeff1_7c3d498c","line":251,"range":{"start_line":247,"start_character":12,"end_line":251,"end_character":24},"in_reply_to":"9fdfeff1_b21d2710","updated":"2019-02-28 17:44:08.000000000","message":"Yeah, I don\u0027t know what I was thinking when I wrote this yesterday. This is definitely one of the causes of the infinite loop. Incrementing \u0027done\u0027 doesn\u0027t help anything because the instance will just show up as \u0027found\u0027 again and again. What I should have done here is decrement \u0027found\u0027 instead of incrementing \u0027done\u0027.\n\nCorrect that I want to avoid updating deleted instances because I know (from my previous job) that some operators rarely purge instances from the database, and migrating the deleted instances (10k? 100k?) could be a major cost.","commit_id":"65910122ef5d66d6804c3138f16f7e989be162ee"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"4dd4076739cfd3e653e303f9c53f00420a0e240c","unresolved":false,"context_lines":[{"line_number":257,"context_line":"        if max_count \u003c\u003d 0:"},{"line_number":258,"context_line":"            break"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"    return found, done"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":15,"id":"9fdfeff1_dd31e10c","line":260,"range":{"start_line":260,"start_character":11,"end_line":260,"end_character":22},"updated":"2019-02-28 10:27:40.000000000","message":"Just realized something when I saw this, for the recent migrations we assume every instance_mapping/request spec has a corresponding instance left.\n\nThere can be instance_mappings older than Queens which no longer have instances since they got archived - orphaned ones (we have a lot of them downstream probably I should try to get https://review.openstack.org/#/c/560042/ in at least downstream before we run this and the queued_for_delete, else its never going to complete I guess :( and we return (done, done) for the queued_for_delete migration which will make it believe like its done. ).","commit_id":"65910122ef5d66d6804c3138f16f7e989be162ee"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"5e02abebf24a71e6287ef341193367ade79bc051","unresolved":false,"context_lines":[{"line_number":257,"context_line":"        if max_count \u003c\u003d 0:"},{"line_number":258,"context_line":"            break"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"    return found, done"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":15,"id":"9fdfeff1_07a94a7a","line":260,"range":{"start_line":260,"start_character":11,"end_line":260,"end_character":22},"in_reply_to":"9fdfeff1_5c1de50c","updated":"2019-02-28 19:16:32.000000000","message":"yea you are right, sorry this comment has nothing to do with this patch, I was just speaking out loud.","commit_id":"65910122ef5d66d6804c3138f16f7e989be162ee"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9107df04c324b61ae965b05adcf815a7aff94dd0","unresolved":false,"context_lines":[{"line_number":257,"context_line":"        if max_count \u003c\u003d 0:"},{"line_number":258,"context_line":"            break"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"    return found, done"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":15,"id":"9fdfeff1_5c1de50c","line":260,"range":{"start_line":260,"start_character":11,"end_line":260,"end_character":22},"in_reply_to":"9fdfeff1_dd31e10c","updated":"2019-02-28 17:44:08.000000000","message":"That should be OK though, right? Meaning, you just don\u0027t count the orphaned instance mappings as \u0027found\u0027, as they don\u0027t matter to you. True, it would be good to clean them up, but I don\u0027t think that\u0027s related to the migration being OK. Because the migration only considers un-orphaned instance mappings, it will always complete. Let me know if I\u0027m missing something here.","commit_id":"65910122ef5d66d6804c3138f16f7e989be162ee"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"54d7c48ec03e13a8a53976ce1ac03e6ce1238c25","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            # NOTE: it\u0027s not possible to query only for soft-deleted instances."},{"line_number":234,"context_line":"            # We must query for both deleted and soft-deleted instances."},{"line_number":235,"context_line":"            filters \u003d {\u0027uuid\u0027: inst_uuids}"},{"line_number":236,"context_line":"            instances \u003d objects.InstanceList.get_by_filters("},{"line_number":237,"context_line":"                cctxt, filters, expected_attrs\u003d[])"},{"line_number":238,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":239,"context_line":"        # and update it."}],"source_content_type":"text/x-python","patch_set":16,"id":"9fdfeff1_2289dc6d","line":236,"range":{"start_line":236,"start_character":24,"end_line":236,"end_character":59},"updated":"2019-02-28 19:11:11.000000000","message":"I was just chatting with Surya and realized, instead of using this method to get the instances, we could just do the query manually and filter the SOFT_DELETED instances by hand instead of being limited by the get_by_filters functionality. That way, we can avoid querying for potentially a very large number of deleted!\u003d0 instances. The other problem Surya pointed out is that by pulling deleted!\u003d0 instances in an env with a large number of them, when the operator specifies max_count to the script, they\u0027ll get a bunch of deleted instances counting against max_count. And in the worst case, they would never be able to finish migrating because deleted!\u003d0 instances would always be greater than max_count.\n\nSo, I think we have to do the instance query manually.","commit_id":"22f3b7e3e7a96b39e9492dd66dd8047628ba472d"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1047f289d113ede84b7ddd894c4f2702e87e352c","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            # NOTE: it\u0027s not possible to query only for soft-deleted instances."},{"line_number":234,"context_line":"            # We must query for both deleted and soft-deleted instances."},{"line_number":235,"context_line":"            filters \u003d {\u0027uuid\u0027: inst_uuids}"},{"line_number":236,"context_line":"            instances \u003d objects.InstanceList.get_by_filters("},{"line_number":237,"context_line":"                cctxt, filters, expected_attrs\u003d[])"},{"line_number":238,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":239,"context_line":"        # and update it."}],"source_content_type":"text/x-python","patch_set":16,"id":"9fdfeff1_0bf947c6","line":236,"range":{"start_line":236,"start_character":24,"end_line":236,"end_character":59},"in_reply_to":"9fdfeff1_0dfac941","updated":"2019-03-01 09:56:01.000000000","message":"\u003e (later) Sigh... I just tried this and while it works fine for\n \u003e instance records, it doesn\u0027t really solve our problem, because\n \u003e there\u0027s no way to differentiate the instance mapping records for\n \u003e deleted instances vs SOFT_DELETED instances. In either case, they\n \u003e have queued_for_delete\u003dTrue. So we still get the same problem, if\n \u003e we avoid migrating deleted instances.\n \u003e \n \u003e So, it seems the only way to deal with this is: migrate deleted\n \u003e instances, or, have to maintain code that will set user_id upon a\n \u003e SOFT_DELETED instance \u0027restore\u0027 in compute/api, indefinitely.\n\nyea, ok at this point I guess you should choose either of those solutions and go ahead, I would vote to have the user_id set on restore indefinitely but let\u0027s head what mriedem or dansmith suggest.","commit_id":"22f3b7e3e7a96b39e9492dd66dd8047628ba472d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f05de0b94d12b61def895fb1cc2a2a0194d7e220","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            # NOTE: it\u0027s not possible to query only for soft-deleted instances."},{"line_number":234,"context_line":"            # We must query for both deleted and soft-deleted instances."},{"line_number":235,"context_line":"            filters \u003d {\u0027uuid\u0027: inst_uuids}"},{"line_number":236,"context_line":"            instances \u003d objects.InstanceList.get_by_filters("},{"line_number":237,"context_line":"                cctxt, filters, expected_attrs\u003d[])"},{"line_number":238,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":239,"context_line":"        # and update it."}],"source_content_type":"text/x-python","patch_set":16,"id":"9fdfeff1_0dfac941","line":236,"range":{"start_line":236,"start_character":24,"end_line":236,"end_character":59},"in_reply_to":"9fdfeff1_2289dc6d","updated":"2019-02-28 20:30:02.000000000","message":"(later) Sigh... I just tried this and while it works fine for instance records, it doesn\u0027t really solve our problem, because there\u0027s no way to differentiate the instance mapping records for deleted instances vs SOFT_DELETED instances. In either case, they have queued_for_delete\u003dTrue. So we still get the same problem, if we avoid migrating deleted instances.\n\nSo, it seems the only way to deal with this is: migrate deleted instances, or, have to maintain code that will set user_id upon a SOFT_DELETED instance \u0027restore\u0027 in compute/api, indefinitely.","commit_id":"22f3b7e3e7a96b39e9492dd66dd8047628ba472d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1033d17765613abe0b550bb70cba56520aee9359","unresolved":false,"context_lines":[{"line_number":208,"context_line":"    cms_by_id \u003d {cell.id: cell for cell in cells}"},{"line_number":209,"context_line":"    done \u003d 0"},{"line_number":210,"context_line":"    ims \u003d ("},{"line_number":211,"context_line":"        # Get a list of instance mappings for this cell which do not have"},{"line_number":212,"context_line":"        # user_id populated. We need to include records with"},{"line_number":213,"context_line":"        # queued_for_delete\u003dTrue because they include soft-deleted"},{"line_number":214,"context_line":"        # instances, which could be restored at any time in the future."}],"source_content_type":"text/x-python","patch_set":17,"id":"9fdfeff1_32858e08","line":211,"range":{"start_line":211,"start_character":42,"end_line":211,"end_character":55},"updated":"2019-03-01 20:54:03.000000000","message":"This is old now.","commit_id":"a152f0922e97a5f6e6dde36fc0b5d18bdfb04e1f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1033d17765613abe0b550bb70cba56520aee9359","unresolved":false,"context_lines":[{"line_number":223,"context_line":"        .limit(max_count).all())"},{"line_number":224,"context_line":"    found \u003d len(ims)"},{"line_number":225,"context_line":"    ims_by_inst_uuid \u003d {im.instance_uuid: im for im in ims}"},{"line_number":226,"context_line":"    inst_uuids_by_cell_id \u003d collections.defaultdict(set)"},{"line_number":227,"context_line":"    for im in ims:"},{"line_number":228,"context_line":"        inst_uuids_by_cell_id[im.cell_id].add(im.instance_uuid)"},{"line_number":229,"context_line":"    for cell_id, inst_uuids in inst_uuids_by_cell_id.items():"}],"source_content_type":"text/x-python","patch_set":17,"id":"9fdfeff1_728b161e","line":226,"range":{"start_line":226,"start_character":52,"end_line":226,"end_character":55},"updated":"2019-03-01 20:54:03.000000000","message":"This could just be a list - doesn\u0027t really matter I guess, but we shouldn\u0027t have duplicate instance UUIDs ever.","commit_id":"a152f0922e97a5f6e6dde36fc0b5d18bdfb04e1f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1033d17765613abe0b550bb70cba56520aee9359","unresolved":false,"context_lines":[{"line_number":229,"context_line":"    for cell_id, inst_uuids in inst_uuids_by_cell_id.items():"},{"line_number":230,"context_line":"        # We cannot migrate instance mappings that don\u0027t have a cell yet."},{"line_number":231,"context_line":"        if cell_id is None:"},{"line_number":232,"context_line":"            continue"},{"line_number":233,"context_line":"        with nova_context.target_cell(context, cms_by_id[cell_id]) as cctxt:"},{"line_number":234,"context_line":"            # We need to migrate soft-deleted instances because they could be"},{"line_number":235,"context_line":"            # restored at any time in the future, preventing us from being able"}],"source_content_type":"text/x-python","patch_set":17,"id":"9fdfeff1_32e82eb2","line":232,"range":{"start_line":232,"start_character":12,"end_line":232,"end_character":20},"updated":"2019-03-01 20:54:03.000000000","message":"Hitting this in flight should be relatively rare I would think so it might be worth logging this instance mapping as not having a cell yet during the migration. I\u0027m thinking about stuff like where mnaser has seen some weird things where an instance mapping doesn\u0027t have a cell but the build request is gone. So if I were running this over and over and over and continued to see \u0027found\u0027 results but nothing \u0027done\u0027 I\u0027d want some logging, because if that\u0027s a persistent issue we\u0027d have to consider trying to lookup the RequestSpec to fill in the user_id here.","commit_id":"a152f0922e97a5f6e6dde36fc0b5d18bdfb04e1f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1033d17765613abe0b550bb70cba56520aee9359","unresolved":false,"context_lines":[{"line_number":238,"context_line":"            # NOTE: it\u0027s not possible to query only for soft-deleted instances."},{"line_number":239,"context_line":"            # We must query for both deleted and soft-deleted instances."},{"line_number":240,"context_line":"            filters \u003d {\u0027uuid\u0027: inst_uuids}"},{"line_number":241,"context_line":"            instances \u003d objects.InstanceList.get_by_filters("},{"line_number":242,"context_line":"                cctxt, filters, expected_attrs\u003d[])"},{"line_number":243,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":244,"context_line":"        # and update it."}],"source_content_type":"text/x-python","patch_set":17,"id":"9fdfeff1_92447aba","line":241,"updated":"2019-03-01 20:54:03.000000000","message":"Just thinking out loud, but if the cell is down here this would blow up in some weird way right, like some kind of DBError? And we\u0027d just choke and not be able to process any other instances in up cells. I realize this is an issue in other data migrations that are iterating all the cells (the one for queued_for_delete and the virtual_interfaces one). Seems we should be more resilient, but we could also deal with that in a follow up.","commit_id":"a152f0922e97a5f6e6dde36fc0b5d18bdfb04e1f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"76d6e58628d30f8e6ef1ec58fbef4065ad46a119","unresolved":false,"context_lines":[{"line_number":227,"context_line":"    cms_by_id \u003d {cell.id: cell for cell in cells}"},{"line_number":228,"context_line":"    done \u003d 0"},{"line_number":229,"context_line":"    ims \u003d ("},{"line_number":230,"context_line":"        # Get a list of instance mappings for this cell which do not have"},{"line_number":231,"context_line":"        # user_id populated. We need to include records with"},{"line_number":232,"context_line":"        # queued_for_delete\u003dTrue because they include SOFT_DELETED"},{"line_number":233,"context_line":"        # instances, which could be restored at any time in the future."}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_a9bd7269","line":230,"range":{"start_line":230,"start_character":42,"end_line":230,"end_character":55},"updated":"2019-03-06 19:22:35.000000000","message":"As noted in PS17, this is old now, i.e. the query for instance mappings does not filter by cell.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2151060d3ffc35ae89a0221b8aac2d75136ac6db","unresolved":false,"context_lines":[{"line_number":227,"context_line":"    cms_by_id \u003d {cell.id: cell for cell in cells}"},{"line_number":228,"context_line":"    done \u003d 0"},{"line_number":229,"context_line":"    ims \u003d ("},{"line_number":230,"context_line":"        # Get a list of instance mappings for this cell which do not have"},{"line_number":231,"context_line":"        # user_id populated. We need to include records with"},{"line_number":232,"context_line":"        # queued_for_delete\u003dTrue because they include SOFT_DELETED"},{"line_number":233,"context_line":"        # instances, which could be restored at any time in the future."}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_f13604b5","line":230,"range":{"start_line":230,"start_character":42,"end_line":230,"end_character":55},"in_reply_to":"5fc1f717_a9bd7269","updated":"2019-03-06 19:35:03.000000000","message":"Ah, yep, thanks.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"33b217e50bde10ad8ee69082edc3846aa90c9bc1","unresolved":false,"context_lines":[{"line_number":243,"context_line":"    found \u003d len(ims)"},{"line_number":244,"context_line":"    ims_by_inst_uuid \u003d {im.instance_uuid: im for im in ims}"},{"line_number":245,"context_line":"    inst_uuids_by_cell_id \u003d collections.defaultdict(set)"},{"line_number":246,"context_line":"    for im in ims:"},{"line_number":247,"context_line":"        inst_uuids_by_cell_id[im.cell_id].add(im.instance_uuid)"},{"line_number":248,"context_line":"    for cell_id, inst_uuids in inst_uuids_by_cell_id.items():"},{"line_number":249,"context_line":"        # We cannot migrate instance mappings that don\u0027t have a cell yet."}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_e41e496f","line":246,"updated":"2019-03-06 19:20:10.000000000","message":"You\u0027re iterating the mappings twice back to back. Why not just make this one loop and build both the ims_by_inst_uuid dict and the inst_uuids_by_cell_id dict at the same time?","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2151060d3ffc35ae89a0221b8aac2d75136ac6db","unresolved":false,"context_lines":[{"line_number":243,"context_line":"    found \u003d len(ims)"},{"line_number":244,"context_line":"    ims_by_inst_uuid \u003d {im.instance_uuid: im for im in ims}"},{"line_number":245,"context_line":"    inst_uuids_by_cell_id \u003d collections.defaultdict(set)"},{"line_number":246,"context_line":"    for im in ims:"},{"line_number":247,"context_line":"        inst_uuids_by_cell_id[im.cell_id].add(im.instance_uuid)"},{"line_number":248,"context_line":"    for cell_id, inst_uuids in inst_uuids_by_cell_id.items():"},{"line_number":249,"context_line":"        # We cannot migrate instance mappings that don\u0027t have a cell yet."}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_9147801e","line":246,"in_reply_to":"5fc1f717_e41e496f","updated":"2019-03-06 19:35:03.000000000","message":"Urgh, right. Will update.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"33b217e50bde10ad8ee69082edc3846aa90c9bc1","unresolved":false,"context_lines":[{"line_number":246,"context_line":"    for im in ims:"},{"line_number":247,"context_line":"        inst_uuids_by_cell_id[im.cell_id].add(im.instance_uuid)"},{"line_number":248,"context_line":"    for cell_id, inst_uuids in inst_uuids_by_cell_id.items():"},{"line_number":249,"context_line":"        # We cannot migrate instance mappings that don\u0027t have a cell yet."},{"line_number":250,"context_line":"        if cell_id is None:"},{"line_number":251,"context_line":"            continue"},{"line_number":252,"context_line":"        with nova_context.target_cell(context, cms_by_id[cell_id]) as cctxt:"}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_247d31b1","line":249,"updated":"2019-03-06 19:20:10.000000000","message":"And they\u0027re probably going to be set by the new api code if we\u0027re running this anyway.\n\nWhy did you not just filter these out of the query? That would make us process less stuff in python, use less memory, and do more useful work each batch.\n\n...later...\n\nOkay, in your test you\u0027ve got one that doesn\u0027t have a cell yet, so that is included in found but not done. That\u0027s probably worth it so we\u0027re not just always returning N,N in cases where things really don\u0027t get migrated.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"76d6e58628d30f8e6ef1ec58fbef4065ad46a119","unresolved":false,"context_lines":[{"line_number":248,"context_line":"    for cell_id, inst_uuids in inst_uuids_by_cell_id.items():"},{"line_number":249,"context_line":"        # We cannot migrate instance mappings that don\u0027t have a cell yet."},{"line_number":250,"context_line":"        if cell_id is None:"},{"line_number":251,"context_line":"            continue"},{"line_number":252,"context_line":"        with nova_context.target_cell(context, cms_by_id[cell_id]) as cctxt:"},{"line_number":253,"context_line":"            # We need to migrate SOFT_DELETED instances because they could be"},{"line_number":254,"context_line":"            # restored at any time in the future, preventing us from being able"}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_4973aee8","line":251,"range":{"start_line":251,"start_character":12,"end_line":251,"end_character":20},"updated":"2019-03-06 19:22:35.000000000","message":"Repeating this from PS17 since it looks like you didn\u0027t see it:\n\nHitting this in flight should be relatively rare I would think so it might be worth logging this instance mapping as not having a cell yet during the migration. I\u0027m thinking about stuff like where mnaser has seen some weird things where an instance mapping doesn\u0027t have a cell but the build request is gone. So if I were running this over and over and over and continued to see \u0027found\u0027 results but nothing \u0027done\u0027 I\u0027d want some logging, because if that\u0027s a persistent issue we\u0027d have to consider trying to lookup the RequestSpec to fill in the user_id here.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2151060d3ffc35ae89a0221b8aac2d75136ac6db","unresolved":false,"context_lines":[{"line_number":248,"context_line":"    for cell_id, inst_uuids in inst_uuids_by_cell_id.items():"},{"line_number":249,"context_line":"        # We cannot migrate instance mappings that don\u0027t have a cell yet."},{"line_number":250,"context_line":"        if cell_id is None:"},{"line_number":251,"context_line":"            continue"},{"line_number":252,"context_line":"        with nova_context.target_cell(context, cms_by_id[cell_id]) as cctxt:"},{"line_number":253,"context_line":"            # We need to migrate SOFT_DELETED instances because they could be"},{"line_number":254,"context_line":"            # restored at any time in the future, preventing us from being able"}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_110ab0d1","line":251,"range":{"start_line":251,"start_character":12,"end_line":251,"end_character":20},"in_reply_to":"5fc1f717_4973aee8","updated":"2019-03-06 19:35:03.000000000","message":"Yes, apologies, I must not have seen it. Will add logging.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"76d6e58628d30f8e6ef1ec58fbef4065ad46a119","unresolved":false,"context_lines":[{"line_number":257,"context_line":"            # NOTE: it\u0027s not possible to query only for SOFT_DELETED instances."},{"line_number":258,"context_line":"            # We must query for both deleted and SOFT_DELETED instances."},{"line_number":259,"context_line":"            filters \u003d {\u0027uuid\u0027: inst_uuids}"},{"line_number":260,"context_line":"            instances \u003d objects.InstanceList.get_by_filters("},{"line_number":261,"context_line":"                cctxt, filters, expected_attrs\u003d[])"},{"line_number":262,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":263,"context_line":"        # and update it."}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_c9355ef9","line":260,"updated":"2019-03-06 19:22:35.000000000","message":"Repeating from PS17:\n\nJust thinking out loud, but if the cell is down here this would blow up in some weird way right, like some kind of DBError? And we\u0027d just choke and not be able to process any other instances in up cells. I realize this is an issue in other data migrations that are iterating all the cells (the one for queued_for_delete and the virtual_interfaces one). Seems we should be more resilient, but we could also deal with that in a follow up - maybe just leave a TODO for now since we probably need to handle down-cells in all of these multi-cell data migrations.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2151060d3ffc35ae89a0221b8aac2d75136ac6db","unresolved":false,"context_lines":[{"line_number":257,"context_line":"            # NOTE: it\u0027s not possible to query only for SOFT_DELETED instances."},{"line_number":258,"context_line":"            # We must query for both deleted and SOFT_DELETED instances."},{"line_number":259,"context_line":"            filters \u003d {\u0027uuid\u0027: inst_uuids}"},{"line_number":260,"context_line":"            instances \u003d objects.InstanceList.get_by_filters("},{"line_number":261,"context_line":"                cctxt, filters, expected_attrs\u003d[])"},{"line_number":262,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":263,"context_line":"        # and update it."}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_d113c80c","line":260,"in_reply_to":"5fc1f717_c9355ef9","updated":"2019-03-06 19:35:03.000000000","message":"Good point, I think it\u0027d be easy enough to try-except around this and continue.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"33b217e50bde10ad8ee69082edc3846aa90c9bc1","unresolved":false,"context_lines":[{"line_number":266,"context_line":"            im.user_id \u003d instance.user_id"},{"line_number":267,"context_line":"            context.session.add(im)"},{"line_number":268,"context_line":"            done +\u003d 1"},{"line_number":269,"context_line":"        max_count -\u003d done"},{"line_number":270,"context_line":"        if max_count \u003c\u003d 0:"},{"line_number":271,"context_line":"            break"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"    return found, done"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_c74c6f78","line":271,"range":{"start_line":269,"start_character":0,"end_line":271,"end_character":17},"updated":"2019-03-06 19:20:10.000000000","message":"Heh, or just \"if done \u003e\u003d max_count; break\" but ... sure :)","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2151060d3ffc35ae89a0221b8aac2d75136ac6db","unresolved":false,"context_lines":[{"line_number":266,"context_line":"            im.user_id \u003d instance.user_id"},{"line_number":267,"context_line":"            context.session.add(im)"},{"line_number":268,"context_line":"            done +\u003d 1"},{"line_number":269,"context_line":"        max_count -\u003d done"},{"line_number":270,"context_line":"        if max_count \u003c\u003d 0:"},{"line_number":271,"context_line":"            break"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"    return found, done"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_31024cb6","line":271,"range":{"start_line":269,"start_character":0,"end_line":271,"end_character":17},"in_reply_to":"5fc1f717_c74c6f78","updated":"2019-03-06 19:35:03.000000000","message":"Haha, whoops.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7604438af9bb31aa8f834ac63cf8bbebaaead38c","unresolved":false,"context_lines":[{"line_number":266,"context_line":"            except Exception as exp:"},{"line_number":267,"context_line":"                LOG.debug(\u0027Encountered exception: \"%s\" while querying \u0027"},{"line_number":268,"context_line":"                          \u0027instances from cell: %s. Continuing to the next \u0027"},{"line_number":269,"context_line":"                          \u0027cell.\u0027, cell_id, six.text_type(exp))"},{"line_number":270,"context_line":"                continue"},{"line_number":271,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":272,"context_line":"        # and update it."}],"source_content_type":"text/x-python","patch_set":22,"id":"5fc1f717_2030b056","line":269,"range":{"start_line":269,"start_character":35,"end_line":269,"end_character":62},"updated":"2019-03-06 23:36:58.000000000","message":"gdi, these need to be swapped ordering","commit_id":"8185bed4415ce0ebeea00e895433ec20ab5f59b7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0303770a47aaeecee613d72b7aec8f950fe064a9","unresolved":false,"context_lines":[{"line_number":250,"context_line":"        # We cannot migrate instance mappings that don\u0027t have a cell yet."},{"line_number":251,"context_line":"        if cell_id is None:"},{"line_number":252,"context_line":"            LOG.debug(\u0027Skipping migration of instance mappings %s because \u0027"},{"line_number":253,"context_line":"                      \u0027they do not have a cell.\u0027, inst_uuids)"},{"line_number":254,"context_line":"            continue"},{"line_number":255,"context_line":"        with nova_context.target_cell(context, cms_by_id[cell_id]) as cctxt:"},{"line_number":256,"context_line":"            # We need to migrate SOFT_DELETED instances because they could be"}],"source_content_type":"text/x-python","patch_set":24,"id":"5fc1f717_8e042519","line":253,"range":{"start_line":253,"start_character":50,"end_line":253,"end_character":60},"updated":"2019-03-07 16:02:11.000000000","message":"This could be like 1000+ UUIDs right? That would explode the console output. Seems like this is something you\u0027d want to actually check when populating inst_uuids_by_cell_id above so you can log which instance mappings don\u0027t have a matching cell_id.","commit_id":"cbfa45f257381b50104d78083bccc9e410e2d3db"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8eb4d3cf03bc8bb4b63db9abee5e877c9fbfe7e","unresolved":false,"context_lines":[{"line_number":250,"context_line":"        # We cannot migrate instance mappings that don\u0027t have a cell yet."},{"line_number":251,"context_line":"        if cell_id is None:"},{"line_number":252,"context_line":"            LOG.debug(\u0027Skipping migration of instance mappings %s because \u0027"},{"line_number":253,"context_line":"                      \u0027they do not have a cell.\u0027, inst_uuids)"},{"line_number":254,"context_line":"            continue"},{"line_number":255,"context_line":"        with nova_context.target_cell(context, cms_by_id[cell_id]) as cctxt:"},{"line_number":256,"context_line":"            # We need to migrate SOFT_DELETED instances because they could be"}],"source_content_type":"text/x-python","patch_set":24,"id":"5fc1f717_b927ed12","line":253,"range":{"start_line":253,"start_character":50,"end_line":253,"end_character":60},"in_reply_to":"5fc1f717_8e042519","updated":"2019-03-07 16:17:47.000000000","message":"Sorry, rather than log per mapping that doesn\u0027t have a cell, just log once that there are mappings without a cell. We won\u0027t know if those are in-flight or some other issue is there like what I mentioned here:\n\nhttps://review.openstack.org/#/c/633351/21/nova/objects/instance_mapping.py@251\n\nBut we don\u0027t need to spam the console with it.","commit_id":"cbfa45f257381b50104d78083bccc9e410e2d3db"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e2aa2614a22d67b383527d42c39625715749fff","unresolved":false,"context_lines":[{"line_number":250,"context_line":"        # We cannot migrate instance mappings that don\u0027t have a cell yet."},{"line_number":251,"context_line":"        if cell_id is None:"},{"line_number":252,"context_line":"            LOG.debug(\u0027Skipping migration of instance mappings %s because \u0027"},{"line_number":253,"context_line":"                      \u0027they do not have a cell.\u0027, inst_uuids)"},{"line_number":254,"context_line":"            continue"},{"line_number":255,"context_line":"        with nova_context.target_cell(context, cms_by_id[cell_id]) as cctxt:"},{"line_number":256,"context_line":"            # We need to migrate SOFT_DELETED instances because they could be"}],"source_content_type":"text/x-python","patch_set":24,"id":"5fc1f717_796d0597","line":253,"range":{"start_line":253,"start_character":50,"end_line":253,"end_character":60},"in_reply_to":"5fc1f717_8e042519","updated":"2019-03-07 16:04:43.000000000","message":"Yeah I had suggested a warn-once type behavior.. This will get really ugly on the console...","commit_id":"cbfa45f257381b50104d78083bccc9e410e2d3db"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0303770a47aaeecee613d72b7aec8f950fe064a9","unresolved":false,"context_lines":[{"line_number":264,"context_line":"                instances \u003d objects.InstanceList.get_by_filters("},{"line_number":265,"context_line":"                    cctxt, filters, expected_attrs\u003d[])"},{"line_number":266,"context_line":"            except Exception as exp:"},{"line_number":267,"context_line":"                LOG.debug(\u0027Encountered exception: \"%s\" while querying \u0027"},{"line_number":268,"context_line":"                          \u0027instances from cell: %d. Continuing to the next \u0027"},{"line_number":269,"context_line":"                          \u0027cell.\u0027, six.text_type(exp), cell_id)"},{"line_number":270,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":24,"id":"5fc1f717_9951d1db","line":267,"range":{"start_line":267,"start_character":20,"end_line":267,"end_character":25},"updated":"2019-03-07 16:02:11.000000000","message":"This should be more than a debug right? At least warning or error.","commit_id":"cbfa45f257381b50104d78083bccc9e410e2d3db"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0303770a47aaeecee613d72b7aec8f950fe064a9","unresolved":false,"context_lines":[{"line_number":266,"context_line":"            except Exception as exp:"},{"line_number":267,"context_line":"                LOG.debug(\u0027Encountered exception: \"%s\" while querying \u0027"},{"line_number":268,"context_line":"                          \u0027instances from cell: %d. Continuing to the next \u0027"},{"line_number":269,"context_line":"                          \u0027cell.\u0027, six.text_type(exp), cell_id)"},{"line_number":270,"context_line":"                continue"},{"line_number":271,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":272,"context_line":"        # and update it."}],"source_content_type":"text/x-python","patch_set":24,"id":"5fc1f717_791c256a","line":269,"range":{"start_line":269,"start_character":55,"end_line":269,"end_character":62},"updated":"2019-03-07 16:02:11.000000000","message":"This is just the primary key and not very user friendly. It would be better to use something like:\n\ncms_by_id[cell_id].identity","commit_id":"cbfa45f257381b50104d78083bccc9e410e2d3db"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0303770a47aaeecee613d72b7aec8f950fe064a9","unresolved":false,"context_lines":[{"line_number":277,"context_line":"            done +\u003d 1"},{"line_number":278,"context_line":"        if ims_by_inst_uuid:"},{"line_number":279,"context_line":"            LOG.debug(\u0027Skipping migration of instance mappings %s because \u0027"},{"line_number":280,"context_line":"                      \u0027they do not have instances.\u0027, ims_by_inst_uuid.keys())"},{"line_number":281,"context_line":"            if not orphaned_ims_warned:"},{"line_number":282,"context_line":"                LOG.warning(\u0027Orphaned instance mappings were found. They must \u0027"},{"line_number":283,"context_line":"                            \u0027be cleaned up/removed manually before this \u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"5fc1f717_d9abd9d3","line":280,"range":{"start_line":280,"start_character":53,"end_line":280,"end_character":76},"updated":"2019-03-07 16:02:11.000000000","message":"This is another potentially large amount of UUIDs if let\u0027s say 500 instances have been deleted but the db archive/purge hasn\u0027t happened yet. I would probably just drop this logging since the orphan warning below is the more useful one.","commit_id":"cbfa45f257381b50104d78083bccc9e410e2d3db"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0303770a47aaeecee613d72b7aec8f950fe064a9","unresolved":false,"context_lines":[{"line_number":279,"context_line":"            LOG.debug(\u0027Skipping migration of instance mappings %s because \u0027"},{"line_number":280,"context_line":"                      \u0027they do not have instances.\u0027, ims_by_inst_uuid.keys())"},{"line_number":281,"context_line":"            if not orphaned_ims_warned:"},{"line_number":282,"context_line":"                LOG.warning(\u0027Orphaned instance mappings were found. They must \u0027"},{"line_number":283,"context_line":"                            \u0027be cleaned up/removed manually before this \u0027"},{"line_number":284,"context_line":"                            \u0027online data migration can complete.\u0027)"},{"line_number":285,"context_line":"                orphaned_ims_warned \u003d True"}],"source_content_type":"text/x-python","patch_set":24,"id":"5fc1f717_79b8a529","line":282,"range":{"start_line":282,"start_character":29,"end_line":282,"end_character":66},"updated":"2019-03-07 16:02:11.000000000","message":"nit: maybe log the len(ims_by_inst_uuid) here as an indicator of the size of the issue?","commit_id":"cbfa45f257381b50104d78083bccc9e410e2d3db"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0303770a47aaeecee613d72b7aec8f950fe064a9","unresolved":false,"context_lines":[{"line_number":281,"context_line":"            if not orphaned_ims_warned:"},{"line_number":282,"context_line":"                LOG.warning(\u0027Orphaned instance mappings were found. They must \u0027"},{"line_number":283,"context_line":"                            \u0027be cleaned up/removed manually before this \u0027"},{"line_number":284,"context_line":"                            \u0027online data migration can complete.\u0027)"},{"line_number":285,"context_line":"                orphaned_ims_warned \u003d True"},{"line_number":286,"context_line":"        if done \u003e\u003d max_count:"},{"line_number":287,"context_line":"            break"}],"source_content_type":"text/x-python","patch_set":24,"id":"5fc1f717_b997ed90","line":284,"updated":"2019-03-07 16:02:11.000000000","message":"I would add a recommendation to run nova-manage db archive_deleted_rows here so the person reading this doesn\u0027t have to guess what they need to do to clean these up.","commit_id":"cbfa45f257381b50104d78083bccc9e410e2d3db"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d50f9489fd0386d4e53119b8ae0e9bacc5443bce","unresolved":false,"context_lines":[{"line_number":284,"context_line":"        LOG.warning(\u0027Some instance mappings were not migratable. This may \u0027"},{"line_number":285,"context_line":"                    \u0027be transient due to in-flight instance builds, or could \u0027"},{"line_number":286,"context_line":"                    \u0027be due to stale data that will be cleaned up after \u0027"},{"line_number":287,"context_line":"                    \u0027running nova-manage db archive_deleted_rows --purge\u0027)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    return found, done"},{"line_number":290,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"5fc1f717_f4ffbc64","line":287,"range":{"start_line":287,"start_character":29,"end_line":287,"end_character":72},"updated":"2019-03-07 16:50:45.000000000","message":"nit: Throw this in quotes:\n\n\u0027after running \"nova-manage db archive_deleted_rows --purge\".\u0027\n\nDo you also want to mention if there were down cells here? I guess not since we already warn for that above on a per-cell basis.","commit_id":"df6a648748802205a60848df3401dcf9a9d15b68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3d118c804bae46e8df3f80222967b9a1aed168d3","unresolved":false,"context_lines":[{"line_number":284,"context_line":"        LOG.warning(\u0027Some instance mappings were not migratable. This may \u0027"},{"line_number":285,"context_line":"                    \u0027be transient due to in-flight instance builds, or could \u0027"},{"line_number":286,"context_line":"                    \u0027be due to stale data that will be cleaned up after \u0027"},{"line_number":287,"context_line":"                    \u0027running nova-manage db archive_deleted_rows --purge\u0027)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    return found, done"},{"line_number":290,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"5fc1f717_5419f0d4","line":287,"range":{"start_line":287,"start_character":29,"end_line":287,"end_character":72},"in_reply_to":"5fc1f717_f4ffbc64","updated":"2019-03-07 16:51:33.000000000","message":"Yeah, I left that guy in there since it would only be one per cell and might have useful per-cell info.","commit_id":"df6a648748802205a60848df3401dcf9a9d15b68"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"971f32bb4ea4bf7fb3bcb52ff1eb3e7a6fd63076","unresolved":false,"context_lines":[{"line_number":284,"context_line":"        LOG.warning(\u0027Some instance mappings were not migratable. This may \u0027"},{"line_number":285,"context_line":"                    \u0027be transient due to in-flight instance builds, or could \u0027"},{"line_number":286,"context_line":"                    \u0027be due to stale data that will be cleaned up after \u0027"},{"line_number":287,"context_line":"                    \u0027running \"nova-manage db archive_deleted_rows --purge\"\u0027)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    return found, done"},{"line_number":290,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"5fc1f717_b46cd436","line":287,"range":{"start_line":287,"start_character":73,"end_line":287,"end_character":74},"updated":"2019-03-07 16:52:52.000000000","message":"\".\n\nso close","commit_id":"99732383422d5d262a6df13f22f651fd9f0078ad"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fe4a38f40aba80be971be43fe92dd6247206f6ec","unresolved":false,"context_lines":[{"line_number":284,"context_line":"        LOG.warning(\u0027Some instance mappings were not migratable. This may \u0027"},{"line_number":285,"context_line":"                    \u0027be transient due to in-flight instance builds, or could \u0027"},{"line_number":286,"context_line":"                    \u0027be due to stale data that will be cleaned up after \u0027"},{"line_number":287,"context_line":"                    \u0027running \"nova-manage db archive_deleted_rows --purge\"\u0027)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    return found, done"},{"line_number":290,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"5fc1f717_487b908b","line":287,"range":{"start_line":287,"start_character":73,"end_line":287,"end_character":74},"in_reply_to":"5fc1f717_b46cd436","updated":"2019-03-07 17:09:53.000000000","message":"I hate the way this looks so I subconsciously just don\u0027t do it, but I\u0027ll fix.","commit_id":"99732383422d5d262a6df13f22f651fd9f0078ad"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9cec6ec122488a3d4d58aa0f78beab49804a4db2","unresolved":false,"context_lines":[{"line_number":271,"context_line":"        # Walk through every instance that has a mapping needing to be updated"},{"line_number":272,"context_line":"        # and update it."},{"line_number":273,"context_line":"        for instance in instances:"},{"line_number":274,"context_line":"            im \u003d ims_by_inst_uuid.pop(instance.uuid)"},{"line_number":275,"context_line":"            im.user_id \u003d instance.user_id"},{"line_number":276,"context_line":"            context.session.add(im)"},{"line_number":277,"context_line":"            done +\u003d 1"}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_4bbdcab5","line":274,"range":{"start_line":274,"start_character":12,"end_line":274,"end_character":52},"updated":"2019-03-07 18:08:21.000000000","message":"gah, this slipped past me. doink, thanks Dan.","commit_id":"ddc0aba5a54377178a5b2748afe2718aabbc267e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"578d4298a0463e92080347562d08e7e7faec0fab","unresolved":false,"context_lines":[{"line_number":275,"context_line":"            im.user_id \u003d instance.user_id"},{"line_number":276,"context_line":"            context.session.add(im)"},{"line_number":277,"context_line":"            done +\u003d 1"},{"line_number":278,"context_line":"        if ims_by_inst_uuid:"},{"line_number":279,"context_line":"            unmigratable_ims \u003d True"},{"line_number":280,"context_line":"        if done \u003e\u003d max_count:"},{"line_number":281,"context_line":"            break"},{"line_number":282,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_4bd86a52","line":279,"range":{"start_line":278,"start_character":0,"end_line":279,"end_character":35},"updated":"2019-03-07 18:03:31.000000000","message":"I don\u0027t get this conditional. Can someone explain why this is here?","commit_id":"ddc0aba5a54377178a5b2748afe2718aabbc267e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"803137c574b2a7ecbc506c01612933fb4ba18e43","unresolved":false,"context_lines":[{"line_number":275,"context_line":"            im.user_id \u003d instance.user_id"},{"line_number":276,"context_line":"            context.session.add(im)"},{"line_number":277,"context_line":"            done +\u003d 1"},{"line_number":278,"context_line":"        if ims_by_inst_uuid:"},{"line_number":279,"context_line":"            unmigratable_ims \u003d True"},{"line_number":280,"context_line":"        if done \u003e\u003d max_count:"},{"line_number":281,"context_line":"            break"},{"line_number":282,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_cbcc5a8b","line":279,"range":{"start_line":278,"start_character":0,"end_line":279,"end_character":35},"in_reply_to":"5fc1f717_4bd86a52","updated":"2019-03-07 18:05:42.000000000","message":"These are any IMs that didn\u0027t have an instance in a cell. So if there are any left, we need to warn that some were orphaned. See L274.","commit_id":"ddc0aba5a54377178a5b2748afe2718aabbc267e"}],"nova/tests/functional/db/test_instance_mapping.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6ac1f536d2786b636391d1f98899dcc388883bd0","unresolved":false,"context_lines":[{"line_number":287,"context_line":"        self.assertEqual(3, done)"},{"line_number":288,"context_line":"        self.assertEqual(3, total)"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"        # Check that we have only the expected number of records with"},{"line_number":291,"context_line":"        # user_id set. We created 10 instances (5 per cell with 2 per cell"},{"line_number":292,"context_line":"        # with NULL user_id), 1 soft-deleted instance with NULL user_id,"},{"line_number":293,"context_line":"        # 1 deleted instance with NULL user_id, and 1 not-yet-scheduled"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_2cbfaf61","line":290,"updated":"2019-02-27 21:00:17.000000000","message":"Good comment, I was doing this tracking in my head...","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6ac1f536d2786b636391d1f98899dcc388883bd0","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        # Check that one instance mapping record (not yet scheduled) has not"},{"line_number":303,"context_line":"        # been migrated by this script. If the instance began creation on the"},{"line_number":304,"context_line":"        # old side of an upgrade and became scheduled on the new side of the"},{"line_number":305,"context_line":"        # upgrade, the active migration in conductor will populate instance"},{"line_number":306,"context_line":"        # mapping user_id."},{"line_number":307,"context_line":"        # Check that one other instance mapping record (deleted instance) has"},{"line_number":308,"context_line":"        # not been migrated by this script."},{"line_number":309,"context_line":"        self.assertEqual(13, len(mappings))"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_6ca8571f","line":306,"range":{"start_line":305,"start_character":23,"end_line":306,"end_character":26},"updated":"2019-02-27 21:00:17.000000000","message":"Well, I think we\u0027re going to drop that code as being over-eager for a tight window, but the next run of the data migration should hit it.","commit_id":"1c948c7a247221a7c9f2e6bfb89ae1144d8bf3bc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"33b217e50bde10ad8ee69082edc3846aa90c9bc1","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        # instance with NULL user_id."},{"line_number":290,"context_line":"        # We expect 12 of them to have user_id set after migration (13 total,"},{"line_number":291,"context_line":"        # with the not-yet-scheduled instance ignored)."},{"line_number":292,"context_line":"        # We need to load the raw database objects instead of versioned objects"},{"line_number":293,"context_line":"        # because InstanceMapping.user_id is not loadable when NULL."},{"line_number":294,"context_line":"        ims \u003d instance_mapping.InstanceMappingList._get_by_project_id_from_db("},{"line_number":295,"context_line":"            self.context, self.context.project_id)"},{"line_number":296,"context_line":"        self.assertEqual(12, len("}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_27e3f335","line":293,"range":{"start_line":292,"start_character":8,"end_line":293,"end_character":68},"updated":"2019-03-06 19:20:10.000000000","message":"Or just use the \"not in\" test on the object instead of \"is None\" but it doesn\u0027t really matter.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2151060d3ffc35ae89a0221b8aac2d75136ac6db","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        # instance with NULL user_id."},{"line_number":290,"context_line":"        # We expect 12 of them to have user_id set after migration (13 total,"},{"line_number":291,"context_line":"        # with the not-yet-scheduled instance ignored)."},{"line_number":292,"context_line":"        # We need to load the raw database objects instead of versioned objects"},{"line_number":293,"context_line":"        # because InstanceMapping.user_id is not loadable when NULL."},{"line_number":294,"context_line":"        ims \u003d instance_mapping.InstanceMappingList._get_by_project_id_from_db("},{"line_number":295,"context_line":"            self.context, self.context.project_id)"},{"line_number":296,"context_line":"        self.assertEqual(12, len("}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_11d87051","line":293,"range":{"start_line":292,"start_character":8,"end_line":293,"end_character":68},"in_reply_to":"5fc1f717_27e3f335","updated":"2019-03-06 19:35:03.000000000","message":"\u003e Or just use the \"not in\" test on the object instead of \"is None\"\n \u003e but it doesn\u0027t really matter.\n\nOh, true. Can do that instead.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d32c145988120aa3ed841761b2944781527fb6e6","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        # instance with NULL user_id."},{"line_number":290,"context_line":"        # We expect 12 of them to have user_id set after migration (13 total,"},{"line_number":291,"context_line":"        # with the not-yet-scheduled instance ignored)."},{"line_number":292,"context_line":"        # We need to load the raw database objects instead of versioned objects"},{"line_number":293,"context_line":"        # because InstanceMapping.user_id is not loadable when NULL."},{"line_number":294,"context_line":"        ims \u003d instance_mapping.InstanceMappingList._get_by_project_id_from_db("},{"line_number":295,"context_line":"            self.context, self.context.project_id)"},{"line_number":296,"context_line":"        self.assertEqual(12, len("}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_314a4c55","line":293,"range":{"start_line":292,"start_character":8,"end_line":293,"end_character":68},"in_reply_to":"5fc1f717_27e3f335","updated":"2019-03-06 19:25:26.000000000","message":"The comment in the code here is old too now - the InstanceMapping *is* loadable even if user_id is not in the DB record.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2151060d3ffc35ae89a0221b8aac2d75136ac6db","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        # instance with NULL user_id."},{"line_number":290,"context_line":"        # We expect 12 of them to have user_id set after migration (13 total,"},{"line_number":291,"context_line":"        # with the not-yet-scheduled instance ignored)."},{"line_number":292,"context_line":"        # We need to load the raw database objects instead of versioned objects"},{"line_number":293,"context_line":"        # because InstanceMapping.user_id is not loadable when NULL."},{"line_number":294,"context_line":"        ims \u003d instance_mapping.InstanceMappingList._get_by_project_id_from_db("},{"line_number":295,"context_line":"            self.context, self.context.project_id)"},{"line_number":296,"context_line":"        self.assertEqual(12, len("}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_11a630c3","line":293,"range":{"start_line":292,"start_character":8,"end_line":293,"end_character":68},"in_reply_to":"5fc1f717_314a4c55","updated":"2019-03-06 19:35:03.000000000","message":"This is \"InstanceMapping.user_id\" specifically talking about the user_id field. It\u0027s not loaded/set by and of the InstanceMapping.get_* methods. And it\u0027s not lazy-loadable.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"33b217e50bde10ad8ee69082edc3846aa90c9bc1","unresolved":false,"context_lines":[{"line_number":301,"context_line":"        # Check that one other instance mapping record (deleted instance) has"},{"line_number":302,"context_line":"        # not been migrated by this script."},{"line_number":303,"context_line":"        self.assertEqual(13, len(ims))"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        # Set the cell and create the instance for the mapping without a cell,"},{"line_number":306,"context_line":"        # then run the migration to completion."},{"line_number":307,"context_line":"        unscheduled \u003d instance_mapping.InstanceMapping.get_by_instance_uuid("}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_e7096bdd","line":304,"updated":"2019-03-06 19:20:10.000000000","message":"How about a mapping for an instance that doesn\u0027t exist? Like residue after manual cleanup or after a periodic compute purge and before a db archive?","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2151060d3ffc35ae89a0221b8aac2d75136ac6db","unresolved":false,"context_lines":[{"line_number":301,"context_line":"        # Check that one other instance mapping record (deleted instance) has"},{"line_number":302,"context_line":"        # not been migrated by this script."},{"line_number":303,"context_line":"        self.assertEqual(13, len(ims))"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        # Set the cell and create the instance for the mapping without a cell,"},{"line_number":306,"context_line":"        # then run the migration to completion."},{"line_number":307,"context_line":"        unscheduled \u003d instance_mapping.InstanceMapping.get_by_instance_uuid("}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_9191c055","line":304,"in_reply_to":"5fc1f717_e7096bdd","updated":"2019-03-06 19:35:03.000000000","message":"Good idea, will add.","commit_id":"226dc89ffa5b5d83ebe6b4d30c44e7e275b19d4c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0303770a47aaeecee613d72b7aec8f950fe064a9","unresolved":false,"context_lines":[{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        # Delete the orphaned instance mapping (simulate manual cleanup by an"},{"line_number":352,"context_line":"        # operator)."},{"line_number":353,"context_line":"        for i in range(2):"},{"line_number":354,"context_line":"            nonexist \u003d instance_mapping.InstanceMapping.get_by_instance_uuid("},{"line_number":355,"context_line":"                self.context, nonexistent[i][\u0027instance_uuid\u0027])"},{"line_number":356,"context_line":"            nonexist.destroy()"}],"source_content_type":"text/x-python","patch_set":24,"id":"5fc1f717_f940150a","line":353,"range":{"start_line":353,"start_character":8,"end_line":353,"end_character":26},"updated":"2019-03-07 16:02:11.000000000","message":"Can\u0027t you just loop over nonexistent here?","commit_id":"cbfa45f257381b50104d78083bccc9e410e2d3db"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1d1e69d2fd19b52df99d2e5f1ae8debc2a35df68","unresolved":false,"context_lines":[{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        # Delete the orphaned instance mapping (simulate manual cleanup by an"},{"line_number":352,"context_line":"        # operator)."},{"line_number":353,"context_line":"        for i in range(2):"},{"line_number":354,"context_line":"            nonexist \u003d instance_mapping.InstanceMapping.get_by_instance_uuid("},{"line_number":355,"context_line":"                self.context, nonexistent[i][\u0027instance_uuid\u0027])"},{"line_number":356,"context_line":"            nonexist.destroy()"}],"source_content_type":"text/x-python","patch_set":24,"id":"5fc1f717_9956b129","line":353,"range":{"start_line":353,"start_character":8,"end_line":353,"end_character":26},"in_reply_to":"5fc1f717_f940150a","updated":"2019-03-07 16:27:44.000000000","message":"nonexistent is a sqla instance_mapping, not an object, but yeah, I will change this to use that list instead of requiring these two magic numbers line up.","commit_id":"cbfa45f257381b50104d78083bccc9e410e2d3db"}]}