InstanceGroup.get_hosts() uses inefficient DB queries

[Matt Riedemann]

The InstanceGroup.get_hosts() method is pretty inefficient when pulling instances out of the database here:

https://github.com/openstack/nova/blob/c7e9e667426a6d88d396a59cb40d30763a3265f9/nova/objects/instance_group.py#L500

because that by default is going to join on the following tables:

https://github.com/openstack/nova/blob/c7e9e667426a6d88d396a59cb40d30763a3265f9/nova/db/sqlalchemy/api.py#L2098

    if columns_to_join is None:
        columns_to_join_new = ['info_cache', 'security_groups']
        manual_joins = ['metadata', 'system_metadata']

And then just turn around and only use the instance.host value:

https://github.com/openstack/nova/blob/c7e9e667426a6d88d396a59cb40d30763a3265f9/nova/objects/instance_group.py#L502

return list(set([instance.host for instance in instances
                 if instance.host]))

We should be:

1. Avoiding those unnecessary joins by passing expected_attrs=[] which is a simple backportable fix.

2. Write a new DB API method which would get the set of distinct instances.host values for the list of instance uuids where the instances.host value is not None, so I think:

SELECT host FROM instances WHERE host IS NOT NULL AND deleted == 0 AND uuid IN ($instance_uuids) GROUP BY instances.host;

That way we let the DB query do the work and we don't have to load up all of the additional instances fields we don't care about.

The DB API optimization would likely need to be a remotable method because InstanceGroup.get_hosts() is calling in the late affinity check ComputeManager._validate_instance_group_policy() method in the nova-compute service (and because InstanceGroup.get_hosts() is remotable itself).

[Matt Riedemann (mriedem)]

The inefficient query goes back to Icehouse when the InstanceGroup.get_hosts() method was introduced:

https://review.opendev.org/#/c/77786/

So we can at least backport the #1 fix above to stable/ocata.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/661032

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/661032
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=15ccf2ddfbf857186fc05f77a7881243c6311062
Submitter: Zuul
Branch: master

commit 15ccf2ddfbf857186fc05f77a7881243c6311062
Author: Matt Riedemann <email address hidden>
Date: Thu May 23 11:14:55 2019 -0400

    Avoid unnecessary joins in InstanceGroup.get_hosts

    The InstanceList.get_by_filters query is joining on
    info_cache, security_groups, metadata and system_metadata
    because of how instance_get_all_by_filters_sort in the DB API
    works if columns_to_join (expected_attrs) is None. The get_hosts
    method only cares about the instance.host value of its members
    so those joins are unnecessarily expensive.

    This change simply passes expected_attrs=[] to get_by_filters
    to avoid the joins. A follow up change can further optimize this
    code by adding a new query method to just get the host values
    for a list of instance uuids, so a TODO is left in place for that.
    Note that a new query method would need to be remotable and thus
    not something we can backport.

    Change-Id: I53d4b38d12404a1641f667c537404effa837a83d
    Partial-Bug: #1830234

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/stein
Review: https://review.opendev.org/664271

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/664271
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=74e66fe8d4353be95868a68d21d882020cb40f4b
Submitter: Zuul
Branch: stable/stein

commit 74e66fe8d4353be95868a68d21d882020cb40f4b
Author: Matt Riedemann <email address hidden>
Date: Thu May 23 11:14:55 2019 -0400

    Avoid unnecessary joins in InstanceGroup.get_hosts

    The InstanceList.get_by_filters query is joining on
    info_cache, security_groups, metadata and system_metadata
    because of how instance_get_all_by_filters_sort in the DB API
    works if columns_to_join (expected_attrs) is None. The get_hosts
    method only cares about the instance.host value of its members
    so those joins are unnecessarily expensive.

    This change simply passes expected_attrs=[] to get_by_filters
    to avoid the joins. A follow up change can further optimize this
    code by adding a new query method to just get the host values
    for a list of instance uuids, so a TODO is left in place for that.
    Note that a new query method would need to be remotable and thus
    not something we can backport.

    Change-Id: I53d4b38d12404a1641f667c537404effa837a83d
    Partial-Bug: #1830234
    (cherry picked from commit 15ccf2ddfbf857186fc05f77a7881243c6311062)