Pinned instance with thread policy can consume VCPU

Bug #1889633 reported by Stephen Finucane
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
High
Stephen Finucane
Train
Fix Released
High
Stephen Finucane
Ussuri
Fix Released
High
Stephen Finucane

Bug Description

In Train, we introduced the concept of the 'PCPU' resource type to track pinned instance CPU usage. The '[compute] cpu_dedicated_set' is used to indicate which host cores should be used by pinned instances and, once this config option was set, nova would start reporting 'PCPU' resource types in addition to (or entirely instead of, if 'cpu_shared_set' was unset) 'VCPU'. Requests for pinned instances (via the 'hw:cpu_policy=dedicated' flavor extra spec or equivalent image metadata property) would result in a query for 'PCPU' inventory rather than 'VCPU', as previously done.

We anticipated some upgrade issues with this change, whereby there could be a period during an upgrade in which some hosts would have the new configuration, meaning they'd be reporting PCPU, but the remainder would still be on legacy config and therefore would continue reporting just VCPU. An instance could be reasonably expected to land on any host, but since only the hosts with the new configuration were reporting 'PCPU' inventory and the 'hw:cpu_policy=dedicated' extra spec was resulting in a request for 'PCPU', the hosts with legacy configuration would never be consumed.

We worked around this issue by adding support for a fallback placement query, enabled by default, which would make a second request using 'VCPU' inventory instead of 'PCPU'. The idea behind this was that the hosts with 'PCPU' inventory would be preferred, meaning we'd only try the 'VCPU' allocation if the preferred path failed. Crucially, we anticipated that if a host with new style configuration was picked up by this second 'VCPU' query, an instance would never actually be able to build there. This is because the new-style configuration would be reflected in the 'numa_topology' blob of the 'ComputeNode' object, specifically via the 'cpuset' (for cores allocated to 'VCPU') and 'pcpuset' (for cores allocated to 'PCPU') fields. With new-style configuration, both of these are set to unique values. If the scheduler had determined that there wasn't enough 'PCPU' inventory available for the instance, that would implicitly mean there weren't enough of the cores listed in the 'pcpuset' field still available.

Turns out there's a gap in this thinking: thread policies. The 'isolate' CPU thread policy previously meant "give me a host with no hyperthreads, else a host with hyperthreads but mark the thread siblings of the cores used by the instance as reserved". This didn't translate to a new 'PCPU' world where we needed to know how many cores we were consuming up front before landing on the host. To work around this, we removed support for the latter case and instead relied on a trait, 'HW_CPU_HYPERTHEADING', to indicate whether a host had hyperthread support or not. Using the 'isolate' policy meant that trait could not be defined on the host, or the trait was "forbidden". The gap comes via a combination of this trait request and the fallback query. If we request the isolate thread policy, hosts with new-style configuration and sufficient PCPU inventory would nonetheless be rejected if they reported the 'HW_CPU_HYPERTHEADING' trait. However, these could get picked up in the fallback query and the instance would not fail to build on the host because of lack of 'PCPU' inventory. This means we end up with a pinned instance on a host using new-style configuration that is consuming 'VCPU' inventory. Boo.

# Steps to reproduce

1. Using a host with hyperthreading support enabled, configure both '[compute] cpu_dedicated_set' and '[compute] cpu_shared_set'

2. Boot an instance with the 'hw:cpu_thread_policy=isolate' extra spec.

# Expected result

Instance should not boot since the host has hyperthreads.

# Actual result

Instance boots.

Revision history for this message
sean mooney (sean-k-mooney) wrote :

this has a signicant upgrade impact so i think this is imporant to fix and backport.
i have repoduced this locally too so moveing to triaged.

Changed in nova:
importance: Undecided → High
status: New → Triaged
tags: added: libvirt
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (master)

Related fix proposed to branch: master
Review: https://review.opendev.org/744020

Changed in nova:
assignee: nobody → Stephen Finucane (stephenfinucane)
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (master)
Download full text (3.3 KiB)

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

commit 737e0c0111acd364d1481bdabd9d23bc8d5d6a2e
Author: Stephen Finucane <email address hidden>
Date: Thu Jul 30 17:37:38 2020 +0100

    tests: Add reproducer for bug #1889633

    With the introduction of the cpu-resources work [1], (libvirt) hosts can
    now report 'PCPU' inventory separate from 'VCPU' inventory, which is
    consumed by instances with pinned CPUs ('hw:cpu_policy=dedicated'). As
    part of that effort, we had to drop support for the ability to boot
    instances with 'hw:cpu_thread_policy=isolate' (i.e. I don't want
    hyperthreads) on hosts with hyperthreading. This had been previously
    implemented by marking thread siblings of the host cores used by such an
    instance as reserved and unusable by other instances, but such a design
    wasn't possible in world where we had to track resource consumption in
    placement before landing in the host. Instead, the 'isolate' policy now
    simply means "give me a host without hyperthreads". This is enforced by
    hosts with hyperthreads reporting the 'HW_CPU_HYPERTHREADING' trait, and
    instances with the 'isolate' policy requesting
    'HW_CPU_HYPERTHREADING=forbidden'.

    Or at least, that's how it should work. We also have a fallback query
    for placement to find hosts with 'VCPU' inventory and that doesn't care
    about the 'HW_CPU_HYPERTHREADING' trait. This was envisioned to ensure
    hosts with old style configuration ('[DEFAULT] vcpu_pin_set') could
    continue to be scheduled to. We figured that this second fallback query
    could accidentally pick up hosts with new-style configuration, but we
    are also tracking the available and used cores from those listed in the
    '[compute] cpu_dedicated_set' as part of the host 'NUMATopology' objects
    (specifically, via the 'pcpuset' and 'cpu_pinning' fields of the
    'NUMACell' child objects). These are validated by both the
    'NUMATopologyFilter' and the virt driver itself, which means hosts with
    new style configuration that got caught up in this second query would be
    rejected by this filter or by a late failure on the host. (Hint: there's
    much more detail on this in the spec).

    Unfortunately we didn't think about hyperthreading. If a host gets
    picked up in the second request, it might well have enough PCPU
    inventory but simply be rejected in the first query since it had
    hyperthreads. In this case, because it has enough free cores available
    for pinning, neither the filter nor the virt driver will reject the
    request, resulting in a situation whereby the instance ends up falling
    back to the old code paths and consuming $flavor.vcpu host cores, plus
    the thread siblings for each of these cores. Despite this, it will be
    marked as consuming $flavor.vcpu VCPU (not PCPU) inventory in placement.

    This patch proves this to be the case, allowing us to resolve the issue
    later.

    [1] https://specs.openstack.org/openstack/nova-specs/specs/train/app...

Read more...

Changed in nova:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

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

commit 9c270332041d6b98951c0b57d7b344fd551a413c
Author: Stephen Finucane <email address hidden>
Date: Thu Jul 30 17:36:24 2020 +0100

    hardware: Reject requests for no hyperthreads on hosts with HT

    Attempting to boot an instance with 'hw:cpu_policy=dedicated' will
    result in a request from nova-scheduler to placement for allocation
    candidates with $flavor.vcpu 'PCPU' inventory. Similarly, booting an
    instance with 'hw:cpu_thread_policy=isolate' will result in a request
    for allocation candidates with 'HW_CPU_HYPERTHREADING=forbidden', i.e.
    hosts without hyperthreading. This has been the case since the
    cpu-resources feature was implemented in Train. However, as part of that
    work and to enable upgrades from hosts that predated Train, we also make
    a second request for candidates with $flavor.vcpu 'VCPU' inventory. The
    idea behind this is that old compute nodes would only report 'VCPU' and
    should be useable, and any new compute nodes that got caught up in this
    second request could never actually be scheduled to since there wouldn't
    be enough cores from 'ComputeNode.numa_topology.cells.[*].pcpuset'
    available to schedule to, resulting in rejection by the
    'NUMATopologyFilter'. However, if a host was rejected in the first
    query because it reported the 'HW_CPU_HYPERTHREADING' trait, it could
    get picked up by the second query and would happily be scheduled to,
    resulting in an instance consuming 'VCPU' inventory from a host that
    properly supported 'PCPU' inventory.

    The solution is simply, though also a huge hack. If we detect that the
    host is using new style configuration and should be able to report
    'PCPU', check if the instance asked for no hyperthreading and whether
    the host has it. If all are True, reject the request.

    Change-Id: Id39aaaac09585ca1a754b669351c86e234b89dd9
    Signed-off-by: Stephen Finucane <email address hidden>
    Closes-Bug: #1889633

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/ussuri)

Related fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/748251

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/ussuri)

Fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/748252

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/train)

Related fix proposed to branch: stable/train
Review: https://review.opendev.org/748254

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/train)

Fix proposed to branch: stable/train
Review: https://review.opendev.org/748255

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/ussuri)
Download full text (3.4 KiB)

Reviewed: https://review.opendev.org/748251
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=49a793c8ee7a9be26e4e3d6ddd097a6ee6fea29d
Submitter: Zuul
Branch: stable/ussuri

commit 49a793c8ee7a9be26e4e3d6ddd097a6ee6fea29d
Author: Stephen Finucane <email address hidden>
Date: Thu Jul 30 17:37:38 2020 +0100

    tests: Add reproducer for bug #1889633

    With the introduction of the cpu-resources work [1], (libvirt) hosts can
    now report 'PCPU' inventory separate from 'VCPU' inventory, which is
    consumed by instances with pinned CPUs ('hw:cpu_policy=dedicated'). As
    part of that effort, we had to drop support for the ability to boot
    instances with 'hw:cpu_thread_policy=isolate' (i.e. I don't want
    hyperthreads) on hosts with hyperthreading. This had been previously
    implemented by marking thread siblings of the host cores used by such an
    instance as reserved and unusable by other instances, but such a design
    wasn't possible in world where we had to track resource consumption in
    placement before landing in the host. Instead, the 'isolate' policy now
    simply means "give me a host without hyperthreads". This is enforced by
    hosts with hyperthreads reporting the 'HW_CPU_HYPERTHREADING' trait, and
    instances with the 'isolate' policy requesting
    'HW_CPU_HYPERTHREADING=forbidden'.

    Or at least, that's how it should work. We also have a fallback query
    for placement to find hosts with 'VCPU' inventory and that doesn't care
    about the 'HW_CPU_HYPERTHREADING' trait. This was envisioned to ensure
    hosts with old style configuration ('[DEFAULT] vcpu_pin_set') could
    continue to be scheduled to. We figured that this second fallback query
    could accidentally pick up hosts with new-style configuration, but we
    are also tracking the available and used cores from those listed in the
    '[compute] cpu_dedicated_set' as part of the host 'NUMATopology' objects
    (specifically, via the 'pcpuset' and 'cpu_pinning' fields of the
    'NUMACell' child objects). These are validated by both the
    'NUMATopologyFilter' and the virt driver itself, which means hosts with
    new style configuration that got caught up in this second query would be
    rejected by this filter or by a late failure on the host. (Hint: there's
    much more detail on this in the spec).

    Unfortunately we didn't think about hyperthreading. If a host gets
    picked up in the second request, it might well have enough PCPU
    inventory but simply be rejected in the first query since it had
    hyperthreads. In this case, because it has enough free cores available
    for pinning, neither the filter nor the virt driver will reject the
    request, resulting in a situation whereby the instance ends up falling
    back to the old code paths and consuming $flavor.vcpu host cores, plus
    the thread siblings for each of these cores. Despite this, it will be
    marked as consuming $flavor.vcpu VCPU (not PCPU) inventory in placement.

    This patch proves this to be the case, allowing us to resolve the issue
    later.

    [1] https://specs.openstack.org/openstack/nova-specs/specs/tr...

Read more...

tags: added: in-stable-ussuri
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/ussuri)

Reviewed: https://review.opendev.org/748252
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=7ddab327675d36a4ba59d02d22d042d418236336
Submitter: Zuul
Branch: stable/ussuri

commit 7ddab327675d36a4ba59d02d22d042d418236336
Author: Stephen Finucane <email address hidden>
Date: Thu Jul 30 17:36:24 2020 +0100

    hardware: Reject requests for no hyperthreads on hosts with HT

    Attempting to boot an instance with 'hw:cpu_policy=dedicated' will
    result in a request from nova-scheduler to placement for allocation
    candidates with $flavor.vcpu 'PCPU' inventory. Similarly, booting an
    instance with 'hw:cpu_thread_policy=isolate' will result in a request
    for allocation candidates with 'HW_CPU_HYPERTHREADING=forbidden', i.e.
    hosts without hyperthreading. This has been the case since the
    cpu-resources feature was implemented in Train. However, as part of that
    work and to enable upgrades from hosts that predated Train, we also make
    a second request for candidates with $flavor.vcpu 'VCPU' inventory. The
    idea behind this is that old compute nodes would only report 'VCPU' and
    should be useable, and any new compute nodes that got caught up in this
    second request could never actually be scheduled to since there wouldn't
    be enough cores from 'ComputeNode.numa_topology.cells.[*].pcpuset'
    available to schedule to, resulting in rejection by the
    'NUMATopologyFilter'. However, if a host was rejected in the first
    query because it reported the 'HW_CPU_HYPERTHREADING' trait, it could
    get picked up by the second query and would happily be scheduled to,
    resulting in an instance consuming 'VCPU' inventory from a host that
    properly supported 'PCPU' inventory.

    The solution is simply, though also a huge hack. If we detect that the
    host is using new style configuration and should be able to report
    'PCPU', check if the instance asked for no hyperthreading and whether
    the host has it. If all are True, reject the request.

    Change-Id: Id39aaaac09585ca1a754b669351c86e234b89dd9
    Signed-off-by: Stephen Finucane <email address hidden>
    Closes-Bug: #1889633
    (cherry picked from commit 9c270332041d6b98951c0b57d7b344fd551a413c)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/train)
Download full text (3.4 KiB)

Reviewed: https://review.opendev.org/748254
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=b60be4a9416cb5b15b7accb99c6e5ecdac40c3c9
Submitter: Zuul
Branch: stable/train

commit b60be4a9416cb5b15b7accb99c6e5ecdac40c3c9
Author: Stephen Finucane <email address hidden>
Date: Thu Jul 30 17:37:38 2020 +0100

    tests: Add reproducer for bug #1889633

    With the introduction of the cpu-resources work [1], (libvirt) hosts can
    now report 'PCPU' inventory separate from 'VCPU' inventory, which is
    consumed by instances with pinned CPUs ('hw:cpu_policy=dedicated'). As
    part of that effort, we had to drop support for the ability to boot
    instances with 'hw:cpu_thread_policy=isolate' (i.e. I don't want
    hyperthreads) on hosts with hyperthreading. This had been previously
    implemented by marking thread siblings of the host cores used by such an
    instance as reserved and unusable by other instances, but such a design
    wasn't possible in world where we had to track resource consumption in
    placement before landing in the host. Instead, the 'isolate' policy now
    simply means "give me a host without hyperthreads". This is enforced by
    hosts with hyperthreads reporting the 'HW_CPU_HYPERTHREADING' trait, and
    instances with the 'isolate' policy requesting
    'HW_CPU_HYPERTHREADING=forbidden'.

    Or at least, that's how it should work. We also have a fallback query
    for placement to find hosts with 'VCPU' inventory and that doesn't care
    about the 'HW_CPU_HYPERTHREADING' trait. This was envisioned to ensure
    hosts with old style configuration ('[DEFAULT] vcpu_pin_set') could
    continue to be scheduled to. We figured that this second fallback query
    could accidentally pick up hosts with new-style configuration, but we
    are also tracking the available and used cores from those listed in the
    '[compute] cpu_dedicated_set' as part of the host 'NUMATopology' objects
    (specifically, via the 'pcpuset' and 'cpu_pinning' fields of the
    'NUMACell' child objects). These are validated by both the
    'NUMATopologyFilter' and the virt driver itself, which means hosts with
    new style configuration that got caught up in this second query would be
    rejected by this filter or by a late failure on the host. (Hint: there's
    much more detail on this in the spec).

    Unfortunately we didn't think about hyperthreading. If a host gets
    picked up in the second request, it might well have enough PCPU
    inventory but simply be rejected in the first query since it had
    hyperthreads. In this case, because it has enough free cores available
    for pinning, neither the filter nor the virt driver will reject the
    request, resulting in a situation whereby the instance ends up falling
    back to the old code paths and consuming $flavor.vcpu host cores, plus
    the thread siblings for each of these cores. Despite this, it will be
    marked as consuming $flavor.vcpu VCPU (not PCPU) inventory in placement.

    This patch proves this to be the case, allowing us to resolve the issue
    later.

    [1] https://specs.openstack.org/openstack/nova-specs/specs/tra...

Read more...

tags: added: in-stable-train
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/train)

Reviewed: https://review.opendev.org/748255
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=44676ddf843ba84e26721cd2e3f65dc45a881f66
Submitter: Zuul
Branch: stable/train

commit 44676ddf843ba84e26721cd2e3f65dc45a881f66
Author: Stephen Finucane <email address hidden>
Date: Thu Jul 30 17:36:24 2020 +0100

    hardware: Reject requests for no hyperthreads on hosts with HT

    Attempting to boot an instance with 'hw:cpu_policy=dedicated' will
    result in a request from nova-scheduler to placement for allocation
    candidates with $flavor.vcpu 'PCPU' inventory. Similarly, booting an
    instance with 'hw:cpu_thread_policy=isolate' will result in a request
    for allocation candidates with 'HW_CPU_HYPERTHREADING=forbidden', i.e.
    hosts without hyperthreading. This has been the case since the
    cpu-resources feature was implemented in Train. However, as part of that
    work and to enable upgrades from hosts that predated Train, we also make
    a second request for candidates with $flavor.vcpu 'VCPU' inventory. The
    idea behind this is that old compute nodes would only report 'VCPU' and
    should be useable, and any new compute nodes that got caught up in this
    second request could never actually be scheduled to since there wouldn't
    be enough cores from 'ComputeNode.numa_topology.cells.[*].pcpuset'
    available to schedule to, resulting in rejection by the
    'NUMATopologyFilter'. However, if a host was rejected in the first
    query because it reported the 'HW_CPU_HYPERTHREADING' trait, it could
    get picked up by the second query and would happily be scheduled to,
    resulting in an instance consuming 'VCPU' inventory from a host that
    properly supported 'PCPU' inventory.

    The solution is simply, though also a huge hack. If we detect that the
    host is using new style configuration and should be able to report
    'PCPU', check if the instance asked for no hyperthreading and whether
    the host has it. If all are True, reject the request.

    Change-Id: Id39aaaac09585ca1a754b669351c86e234b89dd9
    Signed-off-by: Stephen Finucane <email address hidden>
    Closes-Bug: #1889633
    (cherry picked from commit 9c270332041d6b98951c0b57d7b344fd551a413c)
    (cherry picked from commit 7ddab327675d36a4ba59d02d22d042d418236336)

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

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.