)]}'
{"nova/conductor/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"76b5ca7a80f97bc3bf5e8faa7b7207feef8135e8","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"                        # instance in a cell. If anything goes wrong here, we"},{"line_number":1261,"context_line":"                        # need to try and delete the instance in the cell"},{"line_number":1262,"context_line":"                        # otherwise it could be orphaned."},{"line_number":1263,"context_line":"                        inst_mapping \u003d ("},{"line_number":1264,"context_line":"                            objects.InstanceMapping.get_by_instance_uuid("},{"line_number":1265,"context_line":"                                context, instance.uuid))"},{"line_number":1266,"context_line":"                        inst_mapping.cell_mapping \u003d cell"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_79e59106","line":1263,"updated":"2018-07-27 22:18:17.000000000","message":"Looking at:\n\nhttps://review.openstack.org/#/c/501408/2/nova/conductor/manager.py\n\nWe could just call _populate_instance_mapping here, it does the same thing basically.","commit_id":"41c7cb7a19747c308b881205b4af6e30eadcbcd7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e70c213ac07ba9366ac34193fa349cdd8f1c33a5","unresolved":false,"context_lines":[{"line_number":1276,"context_line":"                            # TODO(mriedem): Do we need to do anything else?"},{"line_number":1277,"context_line":"                            # Update the build request to mark it in ERROR"},{"line_number":1278,"context_line":"                            # state?"},{"line_number":1279,"context_line":"                instances.append(instance)"},{"line_number":1280,"context_line":"                cell_mapping_cache[instance.uuid] \u003d cell"},{"line_number":1281,"context_line":""},{"line_number":1282,"context_line":"        # NOTE(melwitt): We recheck the quota after creating the"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_dd19dad1","line":1279,"range":{"start_line":1279,"start_character":16,"end_line":1279,"end_character":42},"updated":"2018-08-01 16:49:18.000000000","message":"If we destroyed the instance b/c we failed to update the mapping, then we should append None.","commit_id":"41c7cb7a19747c308b881205b4af6e30eadcbcd7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"88ec2091125e9a96390da4c60923c115c24fbca6","unresolved":false,"context_lines":[{"line_number":1276,"context_line":"                            # TODO(mriedem): Do we need to do anything else?"},{"line_number":1277,"context_line":"                            # Update the build request to mark it in ERROR"},{"line_number":1278,"context_line":"                            # state?"},{"line_number":1279,"context_line":"                instances.append(instance)"},{"line_number":1280,"context_line":"                cell_mapping_cache[instance.uuid] \u003d cell"},{"line_number":1281,"context_line":""},{"line_number":1282,"context_line":"        # NOTE(melwitt): We recheck the quota after creating the"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_d7a19794","line":1279,"range":{"start_line":1279,"start_character":16,"end_line":1279,"end_character":42},"in_reply_to":"5f7c97a3_dd19dad1","updated":"2018-08-02 19:48:28.000000000","message":"Correct.","commit_id":"41c7cb7a19747c308b881205b4af6e30eadcbcd7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"482a9f015b7523d0c9bf6a804c4081201a265d64","unresolved":false,"context_lines":[{"line_number":1325,"context_line":"                objects.InstanceAction.action_start("},{"line_number":1326,"context_line":"                    cctxt, instance.uuid, instance_actions.CREATE,"},{"line_number":1327,"context_line":"                    want_result\u003dFalse)"},{"line_number":1328,"context_line":"                instance_bdms \u003d self._create_block_device_mapping("},{"line_number":1329,"context_line":"                    cell, instance.flavor, instance.uuid, block_device_mapping)"},{"line_number":1330,"context_line":"                instance_tags \u003d self._create_tags(cctxt, instance.uuid, tags)"},{"line_number":1331,"context_line":""},{"line_number":1332,"context_line":"            # TODO(Kevin Zheng): clean this up once instance.create() handles"},{"line_number":1333,"context_line":"            # tags; we do this so the instance.create notification in"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_19801d49","line":1330,"range":{"start_line":1328,"start_character":16,"end_line":1330,"end_character":77},"updated":"2018-07-27 21:58:00.000000000","message":"Related discussion from way back:\n\nhttps://review.openstack.org/#/c/501408/1/nova/conductor/manager.py@1033\n\nAnd:\n\nhttps://review.openstack.org/#/c/501408/2/nova/conductor/manager.py@1020","commit_id":"41c7cb7a19747c308b881205b4af6e30eadcbcd7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5eb65c127afaed7398a4d70b51d28b072c279f7b","unresolved":false,"context_lines":[{"line_number":1298,"context_line":""},{"line_number":1299,"context_line":"        zipped \u003d six.moves.zip(build_requests, request_specs, host_lists,"},{"line_number":1300,"context_line":"                              instances)"},{"line_number":1301,"context_line":"        for (build_request, request_spec, host_list, instance) in zipped:"},{"line_number":1302,"context_line":"            if instance is None:"},{"line_number":1303,"context_line":"                # Skip placeholders that were buried in cell0 or had their"},{"line_number":1304,"context_line":"                # build requests deleted by the user before instance create."},{"line_number":1305,"context_line":"                continue"},{"line_number":1306,"context_line":"            cell \u003d cell_mapping_cache[instance.uuid]"},{"line_number":1307,"context_line":"            # host_list is a list of one or more Selection objects, the first"},{"line_number":1308,"context_line":"            # of which has been selected and its resources claimed."},{"line_number":1309,"context_line":"            host \u003d host_list.pop(0)"},{"line_number":1310,"context_line":"            alts \u003d [(alt.service_host, alt.nodename) for alt in host_list]"},{"line_number":1311,"context_line":"            LOG.debug(\"Selected host: %s; Selected node: %s; Alternates: %s\","},{"line_number":1312,"context_line":"                    host.service_host, host.nodename, alts, instance\u003dinstance)"},{"line_number":1313,"context_line":"            filter_props \u003d request_spec.to_legacy_filter_properties_dict()"},{"line_number":1314,"context_line":"            scheduler_utils.populate_retry(filter_props, instance.uuid)"},{"line_number":1315,"context_line":"            scheduler_utils.populate_filter_properties(filter_props,"},{"line_number":1316,"context_line":"                                                       host)"},{"line_number":1317,"context_line":"            # TODO(melwitt): Maybe we should set_target_cell on the contexts"},{"line_number":1318,"context_line":"            # once we map to a cell, and remove these separate with statements."},{"line_number":1319,"context_line":"            with obj_target_cell(instance, cell) as cctxt:"},{"line_number":1320,"context_line":"                # send a state update notification for the initial create to"},{"line_number":1321,"context_line":"                # show it going from non-existent to BUILDING"},{"line_number":1322,"context_line":"                # This can lazy-load attributes on instance."},{"line_number":1323,"context_line":"                notifications.send_update_with_states(cctxt, instance, None,"},{"line_number":1324,"context_line":"                        vm_states.BUILDING, None, None, service\u003d\"conductor\")"},{"line_number":1325,"context_line":"                objects.InstanceAction.action_start("},{"line_number":1326,"context_line":"                    cctxt, instance.uuid, instance_actions.CREATE,"},{"line_number":1327,"context_line":"                    want_result\u003dFalse)"},{"line_number":1328,"context_line":"                instance_bdms \u003d self._create_block_device_mapping("},{"line_number":1329,"context_line":"                    cell, instance.flavor, instance.uuid, block_device_mapping)"},{"line_number":1330,"context_line":"                instance_tags \u003d self._create_tags(cctxt, instance.uuid, tags)"},{"line_number":1331,"context_line":""},{"line_number":1332,"context_line":"            # TODO(Kevin Zheng): clean this up once instance.create() handles"},{"line_number":1333,"context_line":"            # tags; we do this so the instance.create notification in"},{"line_number":1334,"context_line":"            # build_and_run_instance in nova-compute doesn\u0027t lazy-load tags"},{"line_number":1335,"context_line":"            instance.tags \u003d instance_tags if instance_tags \\"},{"line_number":1336,"context_line":"                else objects.TagList()"},{"line_number":1337,"context_line":""},{"line_number":1338,"context_line":"            if not self._delete_build_request("},{"line_number":1339,"context_line":"                    context, build_request, instance, cell, instance_bdms,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_59145542","line":1336,"range":{"start_line":1301,"start_character":8,"end_line":1336,"end_character":38},"updated":"2018-07-27 21:47:58.000000000","message":"So anything that fails here would prevent us from updating the instance mapping. Also, if we\u0027re doing a multi-create request, anything *after* this in the loop that fails would prevent is from updating any other instance mappings that come after it in the loop. Which is....bad and stuff.","commit_id":"41c7cb7a19747c308b881205b4af6e30eadcbcd7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f3dc8f11ac440712cdbc8aad4bce4a915f9a5243","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"            instance.tags \u003d instance_tags if instance_tags \\"},{"line_number":1336,"context_line":"                else objects.TagList()"},{"line_number":1337,"context_line":""},{"line_number":1338,"context_line":"            if not self._delete_build_request("},{"line_number":1339,"context_line":"                    context, build_request, instance, cell, instance_bdms,"},{"line_number":1340,"context_line":"                    instance_tags):"},{"line_number":1341,"context_line":"                # The build request was deleted before/during scheduling so"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_e6456317","line":1338,"updated":"2018-07-30 14:13:32.000000000","message":"Shouldn\u0027t we keep this as close to the mapping as possible? If we map the instance above, we don\u0027t know that it had actually been deleted by the user until we get here and try to do this. Thus, they could delete it (via BR) and then it would show up again briefly before going away again when we get here. We already had a window, but it was smaller.\n\nIn fact, what we *really* should do is have a single method that converts a BR to a mapping atomically in the DB, and fails if we can\u0027t because the BR is gone...","commit_id":"41c7cb7a19747c308b881205b4af6e30eadcbcd7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"88ec2091125e9a96390da4c60923c115c24fbca6","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"            instance.tags \u003d instance_tags if instance_tags \\"},{"line_number":1336,"context_line":"                else objects.TagList()"},{"line_number":1337,"context_line":""},{"line_number":1338,"context_line":"            if not self._delete_build_request("},{"line_number":1339,"context_line":"                    context, build_request, instance, cell, instance_bdms,"},{"line_number":1340,"context_line":"                    instance_tags):"},{"line_number":1341,"context_line":"                # The build request was deleted before/during scheduling so"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_b7d2dbfd","line":1338,"in_reply_to":"5f7c97a3_1d0af225","updated":"2018-08-02 19:48:28.000000000","message":"Yeah, I agree we should do instance.create(), instance_mapping.save(), and build_request.destroy() close to each other, and maybe let\u0027s just do instance_mapping and build_request as a single database transaction, similar to what we\u0027re thinking for the opposite issue (the creates in compute/api).\n\nIf we don\u0027t do a single transaction, we need to try-except Exception around instance_mapping.save() and build_request.destroy() and clean up everything.","commit_id":"41c7cb7a19747c308b881205b4af6e30eadcbcd7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"85b454292c70985c5f68da294238a6506ef90dd4","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"            instance.tags \u003d instance_tags if instance_tags \\"},{"line_number":1336,"context_line":"                else objects.TagList()"},{"line_number":1337,"context_line":""},{"line_number":1338,"context_line":"            if not self._delete_build_request("},{"line_number":1339,"context_line":"                    context, build_request, instance, cell, instance_bdms,"},{"line_number":1340,"context_line":"                    instance_tags):"},{"line_number":1341,"context_line":"                # The build request was deleted before/during scheduling so"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_1d0af225","line":1338,"in_reply_to":"5f7c97a3_e6456317","updated":"2018-08-01 16:54:23.000000000","message":"\u003e Shouldn\u0027t we keep this as close to the mapping as possible?\n\nYes, but...we should update the mapping as soon as we create the instance in the cell, which is what I\u0027m trying to do in this change, but between the time we create the instance and delete the build request, we also create the bdms and tags in the cell database for the instance. So we could have a window of time where those don\u0027t exist, but maybe that doesn\u0027t really matter all that much while the instance is in BUILD status.\n\nIf we did destroy the build request after instance create but before creating the bdms and tags in the cell, then _delete_build_request wouldn\u0027t have to deal with cleaning up all of those resources - it would just delete the instance if the build request was already deleted by the user by the time we got here.","commit_id":"41c7cb7a19747c308b881205b4af6e30eadcbcd7"}]}
