yahoo-eng-team team mailing list archive

[Jay Pipes <1385489@xxxxxxxxxxxxxxxxxx>]

Public bug reported:

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...

** Affects: nova
     Importance: Undecided
         Status: New


** Tags: low-hanging-fruit

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Compute (nova).
https://bugs.launchpad.net/bugs/1385489

Title:
  ResourceTracker._update_usage_from_migration() is inefficient due to
  multiple Instance.get_by_uuid() lookups

Status in OpenStack Compute (Nova):
  New

Bug description:
  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...

To manage notifications about this bug go to:
https://bugs.launchpad.net/nova/+bug/1385489/+subscriptions