ResourceTracker._update_usage_from_migrations() is inefficient due to multiple Instance.get_by_uuid() lookups

[Jay Pipes]

Here is our ResourceTracker._update_usage_from_migration() code:

    def _update_usage_from_migrations(self, context, resources, migrations):

        self.tracked_migrations.clear()

        filtered = {}

        # do some defensive filtering against bad migrations records in the
        # database:
        for migration in migrations:

            instance = migration['instance']

            if not instance:
                # migration referencing deleted instance
                continue

            uuid = instance['uuid']

            # skip migration if instance isn't in a resize state:
            if not self._instance_in_resize_state(instance):
                LOG.warn(_("Instance not resizing, skipping migration."),
                         instance_uuid=uuid)
                continue

            # filter to most recently updated migration for each instance:
            m = filtered.get(uuid, None)
            if not m or migration['updated_at'] >= m['updated_at']:
                filtered[uuid] = migration

        for migration in filtered.values():
            instance = migration['instance']
            try:
                self._update_usage_from_migration(context, instance, None,
                                                  resources, migration)
            except exception.FlavorNotFound:
                LOG.warn(_("Flavor could not be found, skipping "
                           "migration."), instance_uuid=uuid)
                continue

Unfortunately, when the migration object's 'instance' attribute is accessed, a call across RPC and DB occurs:

https://github.com/openstack/nova/blob/stable/icehouse/nova/objects/migration.py#L77-L80

    @property
    def instance(self):
        return instance_obj.Instance.get_by_uuid(self._context,
                                                 self.instance_uuid)

For some very strange reason, the code in _update_usage_from_migration() builds a "filtered"dictionary with the migration objects that need to be accounted for in the resource usages, and then once it builds that filtered dictionary, it goes through the values and calls _update_usage_from_migration(), passing the migration object's instance object.

There's no reason to do this at all. The filtered variable can go away and the call to _update_usage_from_migration() can occur in the main for loop, using the same instance variable from the original line:

 instance = migration['instance']

That way, for each migration, we don't need to do two lookup by UUID calls through the conductor to get the migration's instance object...

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/131676

[OpenStack Infra (hudson-openstack)]

Change abandoned by Chaitanya Challa (<email address hidden>) on branch: master
Review: https://review.openstack.org/131676

[OpenStack Infra (hudson-openstack)]

Change abandoned by Joe Gordon (<email address hidden>) on branch: master
Review: https://review.openstack.org/131676
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/222439

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/222439
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=e1e47cb5e6093dfe3c17972fd26b4e5de9183961
Submitter: Jenkins
Branch: master

commit e1e47cb5e6093dfe3c17972fd26b4e5de9183961
Author: Diana Clarke <email address hidden>
Date: Thu Sep 10 22:05:46 2015 -0400

    Reduce the number of Instance.get_by_uuid calls

    Only call Instance.get_by_uuid once per migration in
    ResourceTracker._update_usage_from_migrations rather than twice in some
    cases. Also increased test coverage for
    ResourceTracker._update_usage_from_migrations to 100%.

    Closes-Bug: #1385489
    Change-Id: Ibc7e48340e103f8dcc502f7e91d8ffd939349486