Redundant instance group lookup during scheduling of move operations

Bug #1788527 reported by Matt Riedemann on 2018-08-23
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Low
Balazs Gibizer
Rocky
Low
Balazs Gibizer
Stein
Low
Balazs Gibizer

Bug Description

This change:

https://github.com/openstack/nova/commit/459ca56de2366aea53efc9ad3295fdf4ddcd452c

Added code to the setup_instance_group flow to get the instance group fresh so we had the latest hosts for members of the group.

Then change:

https://github.com/openstack/nova/commit/94fd36f0582c5dbcf2b9886da7c7bf986d3ad5d1#diff-cbbdc4d7c140314a7e0b2d97ebcd1f9c

Was added to not persist group hosts/members in the RequestSpec since they could be stale after the initial server create. This means when we move a server (evacuate, resize, unshelve, live migrate), we get the request spec with the group plus the current hosts/members of the group. So if the request spec has the group hosts set by the time it gets to setup_instance_group, the call in _get_group_details to get the group fresh is redundant.

pandatt (pandatt) on 2018-11-16
Changed in nova:
assignee: nobody → Guo Jingyu (pandatt)
pandatt (pandatt) on 2019-05-23
Changed in nova:
assignee: pandatt (pandatt) → nobody
Matt Riedemann (mriedem) wrote :

Looking at this again, this statement in the description is not accurate:

> we get the request spec with the group plus the current hosts/members of the group.

The RequestSpec will load the InstanceGroup by uuid but the InstanceGroup.hosts field is not loaded:

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

That's why we have this check in setup_instance_group so we load the InstanceGroup.hosts from the correct cell(s) based on where the member instances are:

https://github.com/openstack/nova/blob/c7e9e667426a6d88d396a59cb40d30763a3265f9/nova/scheduler/utils.py#L989

I still think it likely means, however, that this code is redundant:

https://github.com/openstack/nova/blob/c7e9e667426a6d88d396a59cb40d30763a3265f9/nova/scheduler/utils.py#L944-L950

I'm not sure in what case user_group_hosts would be different from group_hosts.

Matt Riedemann (mriedem) wrote :

I've reported bug 1830234 for some other inefficiencies in the InstanceGroup.get_hosts() query code.

Balazs Gibizer (balazs-gibizer) wrote :

Removing https://github.com/openstack/nova/blob/c7e9e667426a6d88d396a59cb40d30763a3265f9/nova/scheduler/utils.py#L944-L950 does not effect functional and unit test results. So I guess Matt is right.

Changed in nova:
status: New → Confirmed

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

Changed in nova:
assignee: nobody → Balazs Gibizer (balazs-gibizer)
status: Confirmed → In Progress

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

commit f108d7967636ec1afceff0be386da15d70e2ca37
Author: Balazs Gibizer <email address hidden>
Date: Tue Jun 18 13:17:27 2019 +0200

    Remove redundant group host setup

    It seems that all the code path that ends up calling [1] goes through
    [2] before so setting up group host at [1] is redundant.

    [1] https://github.com/openstack/nova/blob/c7e9e667426a6d88d396a59cb40d30763a3265f9/nova/scheduler/utils.py#L944-L950
    [2] https://github.com/openstack/nova/blob/c7e9e667426a6d88d396a59cb40d30763a3265f9/nova/scheduler/utils.py#L996-L1000

    Change-Id: I745aecb008f0867fcda2d3d2c7691621dd8f6168
    Closes-Bug: #1788527

Changed in nova:
status: In Progress → Fix Released

Reviewed: https://review.opendev.org/667334
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=0313d8143309157067aa85f7333d0d177a5b6609
Submitter: Zuul
Branch: stable/stein

commit 0313d8143309157067aa85f7333d0d177a5b6609
Author: Balazs Gibizer <email address hidden>
Date: Tue Jun 18 13:17:27 2019 +0200

    Remove redundant group host setup

    It seems that all the code path that ends up calling [1] goes through
    [2] before so setting up group host at [1] is redundant.

    [1] https://github.com/openstack/nova/blob/c7e9e667426a6d88d396a59cb40d30763a3265f9/nova/scheduler/utils.py#L944-L950
    [2] https://github.com/openstack/nova/blob/c7e9e667426a6d88d396a59cb40d30763a3265f9/nova/scheduler/utils.py#L996-L1000

    Change-Id: I745aecb008f0867fcda2d3d2c7691621dd8f6168
    Closes-Bug: #1788527
    (cherry picked from commit f108d7967636ec1afceff0be386da15d70e2ca37)

This issue was fixed in the openstack/nova 19.0.2 release.

This issue was fixed in the openstack/nova 20.0.0.0rc1 release candidate.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers