)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"08da39244cc90cbe8e0f2b90ed36377cdeb1c821","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Dan Smith \u003cdanms@us.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2013-04-09 17:01:03 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Optimize some of compute/manager\u0027s periodic tasks\u0027 DB queries"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Most of these tasks do not need to fetch instances with a full complement of"},{"line_number":10,"context_line":"metadata items. Make them say \"no thanks\" to the DB API to reduce the amount of"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"AAAAOn%2F%2F9cw%3D","line":7,"updated":"2013-04-10 03:36:19.000000000","message":"Missing full stop :)","commit_id":"4c279c795b98f34976cbe9710ef81452585b4640"}],"nova/compute/manager.py":[{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"08da39244cc90cbe8e0f2b90ed36377cdeb1c821","unresolved":false,"context_lines":[{"line_number":683,"context_line":"            # pulling system_metadata for performance, and call this function."},{"line_number":684,"context_line":"            # If we get an instance without it, re-fetch so that the call"},{"line_number":685,"context_line":"            # to network_api (which requires it for instance_type) will"},{"line_number":686,"context_line":"            # succeed."},{"line_number":687,"context_line":"            instance \u003d self.conductor_api.instance_get_by_uuid("},{"line_number":688,"context_line":"                context, instance[\u0027uuid\u0027])"},{"line_number":689,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context,"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2F9cs%3D","line":686,"updated":"2013-04-10 03:36:19.000000000","message":"I think this comment is only needed due to the newness of your change. I would be much more pithy myself: something like \u0027populate elided fields for the network_api call\u0027\n\nThat said, we shouldn\u0027t need to re-get the instance, just populate those fields, so perhaps there is more optimisation room here?","commit_id":"4c279c795b98f34976cbe9710ef81452585b4640"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5604ff582aae9a91997ea6b4486031faefd7529b","unresolved":false,"context_lines":[{"line_number":683,"context_line":"            # pulling system_metadata for performance, and call this function."},{"line_number":684,"context_line":"            # If we get an instance without it, re-fetch so that the call"},{"line_number":685,"context_line":"            # to network_api (which requires it for instance_type) will"},{"line_number":686,"context_line":"            # succeed."},{"line_number":687,"context_line":"            instance \u003d self.conductor_api.instance_get_by_uuid("},{"line_number":688,"context_line":"                context, instance[\u0027uuid\u0027])"},{"line_number":689,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context,"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2F85E%3D","line":686,"in_reply_to":"AAAAOn%2F%2F9cs%3D","updated":"2013-04-10 14:22:17.000000000","message":"Certainly, but we don\u0027t have an API to pull the finer-grained set of data (through conductor) at the moment. The idea behind this whole set is a minimal amount of change to address some performance concerns that might be a candidate for backporting. Since the situation mentioned happens only rarely in cleanup-after-failure paths, re-fetching the instance is really not too bad here. Remember, it replaces pulling all the instances and their metadata every time before we even know if we need to do some cleanup :)","commit_id":"4c279c795b98f34976cbe9710ef81452585b4640"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"08da39244cc90cbe8e0f2b90ed36377cdeb1c821","unresolved":false,"context_lines":[{"line_number":3881,"context_line":"                # NOTE(danms): We fetched instances above without the"},{"line_number":3882,"context_line":"                # system_metadata for efficiency. If we get here, we need"},{"line_number":3883,"context_line":"                # to re-fetch with it so that _delete_instace() can extract"},{"line_number":3884,"context_line":"                # instance_type information."},{"line_number":3885,"context_line":"                instance \u003d capi.instance_get_by_uuid(context, instance[\u0027uuid\u0027])"},{"line_number":3886,"context_line":"                LOG.info(_(\u0027Reclaiming deleted instance\u0027), instance\u003dinstance)"},{"line_number":3887,"context_line":"                # NOTE(comstud): Quotas were already accounted for when"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2F9co%3D","line":3884,"updated":"2013-04-10 03:36:19.000000000","message":"Ditto my comment about verbosity in the comment here: there is a balance to be struck. Up to you :)","commit_id":"4c279c795b98f34976cbe9710ef81452585b4640"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5604ff582aae9a91997ea6b4486031faefd7529b","unresolved":false,"context_lines":[{"line_number":3881,"context_line":"                # NOTE(danms): We fetched instances above without the"},{"line_number":3882,"context_line":"                # system_metadata for efficiency. If we get here, we need"},{"line_number":3883,"context_line":"                # to re-fetch with it so that _delete_instace() can extract"},{"line_number":3884,"context_line":"                # instance_type information."},{"line_number":3885,"context_line":"                instance \u003d capi.instance_get_by_uuid(context, instance[\u0027uuid\u0027])"},{"line_number":3886,"context_line":"                LOG.info(_(\u0027Reclaiming deleted instance\u0027), instance\u003dinstance)"},{"line_number":3887,"context_line":"                # NOTE(comstud): Quotas were already accounted for when"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2F844%3D","line":3884,"in_reply_to":"AAAAOn%2F%2F9co%3D","updated":"2013-04-10 14:22:17.000000000","message":"Looks about as verbose as the quota one below it. The verbosity comes, of course, from my estimation about the obscurity of what\u0027s going on for later \"optimizers\" that may find this re-fetch unnecessary :)\n\nHaving had to decode the \"why\" out of a fair number of brief \"how\" comments in the code, I\u0027m trying to look out for my successor.","commit_id":"4c279c795b98f34976cbe9710ef81452585b4640"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"6a94192f0e6e952e81f6eb39a25e76e442093b20","unresolved":false,"context_lines":[{"line_number":3468,"context_line":"                    to_unrescue.append(instance)"},{"line_number":3469,"context_line":""},{"line_number":3470,"context_line":"            for instance in to_unrescue:"},{"line_number":3471,"context_line":"                self.compute_api.unrescue(context, instance)"},{"line_number":3472,"context_line":""},{"line_number":3473,"context_line":"    @manager.periodic_task"},{"line_number":3474,"context_line":"    def _poll_unconfirmed_resizes(self, context):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAOn%2F%2F774%3D","side":"PARENT","line":3471,"updated":"2013-04-10 21:51:26.000000000","message":"I worry about not having the joined data here on the compute API call.  I wonder if we should pull in a fresh copy of the instance to pass to compute API?  It\u0027s going to be rare that we need to unrescue here.","commit_id":"e7283948ecdece9ae300af78b6ff0c501610600e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a8e96a3ab70649b418c35168d7fa76ec77dbe3c3","unresolved":false,"context_lines":[{"line_number":3468,"context_line":"                    to_unrescue.append(instance)"},{"line_number":3469,"context_line":""},{"line_number":3470,"context_line":"            for instance in to_unrescue:"},{"line_number":3471,"context_line":"                self.compute_api.unrescue(context, instance)"},{"line_number":3472,"context_line":""},{"line_number":3473,"context_line":"    @manager.periodic_task"},{"line_number":3474,"context_line":"    def _poll_unconfirmed_resizes(self, context):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAOn%2F%2F7yE%3D","side":"PARENT","line":3471,"in_reply_to":"AAAAOn%2F%2F774%3D","updated":"2013-04-10 22:48:41.000000000","message":"Well, to answer your question, I\u0027m not sure it\u0027s a problem. Compute API\u0027s unrescue records an action start, then calls into manager via RPC to unrescue_instance, which doesn\u0027t, AFAICT, need anything in system_metadata. When it calls _get_instance_nw_info() that function will re-yank the instance from conductor with the metadata filled (see a couple hunks above).\n\nThat said, it will die long before any of that because compute_api will attempt a db.instance_update() and fail the no-db-compute test! Oops!\n\nSo, I think that needs to be fixed, but it\u0027s orthogonal to this patch, as far as I can tell.","commit_id":"e7283948ecdece9ae300af78b6ff0c501610600e"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"6d1f63315357d8fc991fde222eee95153ca8ec6f","unresolved":false,"context_lines":[{"line_number":3468,"context_line":"                    to_unrescue.append(instance)"},{"line_number":3469,"context_line":""},{"line_number":3470,"context_line":"            for instance in to_unrescue:"},{"line_number":3471,"context_line":"                self.compute_api.unrescue(context, instance)"},{"line_number":3472,"context_line":""},{"line_number":3473,"context_line":"    @manager.periodic_task"},{"line_number":3474,"context_line":"    def _poll_unconfirmed_resizes(self, context):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAOn%2F%2F7ug%3D","side":"PARENT","line":3471,"in_reply_to":"AAAAOn%2F%2F7yE%3D","updated":"2013-04-10 23:10:33.000000000","message":"Ok!","commit_id":"e7283948ecdece9ae300af78b6ff0c501610600e"}]}