NUMA aware live migration failed when vCPU pin set

This may be a duplicate of bug 1829349.

Log analysis notes:

The XML was updated to pin both instances to CPUs 0 and 15, at very different times:

2019-09-24 14:16:14.195 6 DEBUG nova.virt.libvirt.migration [-] _update_numa_xml output xml=<domain type="kvm">
  <name>instance-00000012</name>
  <uuid>17bcf040-cf68-4ac3-b365-8a77f93af85b</uuid>
[...]
    <vcpupin vcpu="0" cpuset="0"/>
    <vcpupin vcpu="1" cpuset="15"/>

2019-09-24 14:16:42.251 6 DEBUG nova.virt.libvirt.migration [-] _update_numa_xml output xml=<domain type="kvm">
  <name>instance-00000011</name>
  <uuid>f1929d75-d6ac-45af-b54b-0e10be75d155</uuid>
[...]
    <vcpupin vcpu="0" cpuset="0"/>
    <vcpupin vcpu="1" cpuset="15"/>

For the first live migration we create the claims and the NUMAMigrateInfo:

2019-09-24 14:16:08.747 6 DEBUG nova.compute.manager [req-9c210c32-614c-4040-abc8-e8a4138d885b f28f4213c3a14121b6f7fa15140e7aef 25696a57055b4d6bb5428f45f0473a8c - default default] [instance: 17bcf040-cf68-4ac3-b365-8a77f93af85b] Created live migration claim. _live_migration_claim /var/lib/kolla/venv/lib/python2.7/site-packages/nova/compute/manager.py:6659

2019-09-24 14:16:08.760 6 DEBUG nova.virt.libvirt.driver [req-9c210c32-614c-4040-abc8-e8a4138d885b f28f4213c3a14121b6f7fa15140e7aef 25696a57055b4d6bb5428f45f0473a8c - default default] Built NUMA live migration info: LibvirtLiveMigrateNUMAInfo(cell_pins={0=set([0]),1=set([1])},cpu_pins={0=set([0]),1=set([15])},emulator_pins=set([0,15]),sched_priority=<?>,sched_vcpus=<?>) _get_live_migrate_numa_info /var/lib/kolla/venv/lib/python2.7/site-packages/nova/virt/libvirt/driver.py:8059

Same for the second live migration:

2019-09-24 14:16:35.853 6 DEBUG nova.compute.manager [req-5aeb2f2d-c69f-473a-8da4-59664d87a214 f28f4213c3a14121b6f7fa15140e7aef 25696a57055b4d6bb5428f45f0473a8c - default default] [instance: f1929d75-d6ac-45af-b54b-0e10be75d155] Created live migration claim. _live_migration_claim /var/lib/kolla/venv/lib/python2.7/site-packages/nova/compute/manager.py:6659

2019-09-24 14:16:35.861 6 DEBUG nova.virt.libvirt.driver [req-5aeb2f2d-c69f-473a-8da4-59664d87a214 f28f4213c3a14121b6f7fa15140e7aef 25696a57055b4d6bb5428f45f0473a8c - default default] Built NUMA live migration info: LibvirtLiveMigrateNUMAInfo(cell_pins={0=set([0]),1=set([1])},cpu_pins={0=set([0]),1=set([15])},emulator_pins=set([0,15]),sched_priority=<?>,sched_vcpus=<?>) _get_live_migrate_numa_info /var/lib/kolla/venv/lib/python2.7/site-packages/nova/virt/libvirt/driver.py:8059

Both claimed host CPUs 0 and 15 - but how/why? What happened between those 2 claims? Going back in time, we see:

The second live migration's claim claims CPUs 0 and 15:

2019-09-24 14:16:34.290 6 DEBUG nova.virt.hardware [req-5aeb2f2d-c69f-473a-8da4-59664d87a214 f28f4213c3a14121b6f7fa15140e7aef 25696a57055b4d6bb5428f45f0473a8c - default default] Selected cores for pinning: [(0, 0)], in cell 0 _pack_instance_onto_cores /var/lib/kolla/venv/lib/python2.7/site-packages/nova/virt/hardware.py:979

[...]

2019-09-24 14:16:34.295 6 DEBUG nova.virt.hardware [req-5aeb2f2d-c69f-473a-8da4-59664d87a214 f28f4213c3a14121b6f7fa15140e7aef 25696a57055b4d6bb5428f45f0473a8c - default default] Selected cores for pinning: [(1,...

Figured it out:

When the update resources periodic task runs, it pulls migrations from the database using [1], which filters out migrations in 'accepted' status. Live migrations are created with an 'accepted' status by the conductor [2], and are only set to 'preparing' by the compute manager here [3], which happens after all the new NUMA-aware live migrations claims stuff. So there's a time window after the claim but before the migration has been set to 'preparing' during which, if the periodic resource update task kicks in, it will miss the migration, see that the instance is still on the source host according to the database, and free its resources from the destination.

[1] https://github.com/openstack/nova/blob/0ce66605e16aca85df97acdd8c459802fcdb9aa0/nova/db/sqlalchemy/api.py#L4422
[2] https://github.com/openstack/nova/blob/0ce66605e16aca85df97acdd8c459802fcdb9aa0/nova/conductor/manager.py#L422
[3] https://github.com/openstack/nova/blob/0ce66605e16aca85df97acdd8c459802fcdb9aa0/nova/compute/manager.py#L7020

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

Ya, could you retry your tests with [1] applied, to confirm whether it fixes the issue?

[1] https://review.opendev.org/684409

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

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

commit 6ec686c26b2c8b18bcff522633bfe9715e0feec3
Author: Artom Lifshitz <email address hidden>
Date: Tue Sep 24 13:22:23 2019 -0400

    Stop filtering out 'accepted' for in-progress migrations

    Live migrations are created with an 'accepted' status. Resource claims
    on the destination are done with the migration in 'accepted' status.
    The status is set to 'preparing' a bit later, right before running
    pre_live_migration(). Migrations with status 'accepted' are filtered
    out by the database layer when getting in-progress migrations. Thus,
    there's a time window after resource claims but before 'preparing'
    during which resources have been claimed but the migration is not
    considered in-progress by the database layer. During that window, the
    instance's host is the source - that's only updated once the live
    migration finishes. If the update available resources periodic task
    runs during that window, it'll free the instance's resource from the
    destination because neither the instance nor any of its in-progress
    migrations are associated with the destination. This means that other
    incoming instances are able to consume resources that should not be
    available. This patch stops filtering out the 'accepted' status in the
    database layer when retrieving in-progress migrations.

    Change-Id: I4c56925ed35bc3275ca1ac6c30d7fd641ad84260
    Closes-bug: 1845146

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

commit 45c2ba37bc21370d9814b81cd892abc3cb8a9f04
Author: Artom Lifshitz <email address hidden>
Date: Tue Sep 24 13:22:23 2019 -0400

    Stop filtering out 'accepted' for in-progress migrations

    Live migrations are created with an 'accepted' status. Resource claims
    on the destination are done with the migration in 'accepted' status.
    The status is set to 'preparing' a bit later, right before running
    pre_live_migration(). Migrations with status 'accepted' are filtered
    out by the database layer when getting in-progress migrations. Thus,
    there's a time window after resource claims but before 'preparing'
    during which resources have been claimed but the migration is not
    considered in-progress by the database layer. During that window, the
    instance's host is the source - that's only updated once the live
    migration finishes. If the update available resources periodic task
    runs during that window, it'll free the instance's resource from the
    destination because neither the instance nor any of its in-progress
    migrations are associated with the destination. This means that other
    incoming instances are able to consume resources that should not be
    available. This patch stops filtering out the 'accepted' status in the
    database layer when retrieving in-progress migrations.

    Change-Id: I4c56925ed35bc3275ca1ac6c30d7fd641ad84260
    Closes-bug: 1845146
    (cherry picked from commit 6ec686c26b2c8b18bcff522633bfe9715e0feec3)

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

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

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

commit 32713a4fe885ee55ef0fefc8ce6c78877f2f03e7
Author: Artom Lifshitz <email address hidden>
Date: Tue Oct 8 15:23:47 2019 -0400

    NUMA LM: Add func test for bug 1845146

    Bug 1845146 was caused by the update available resources periodic task
    running during a small window in which the migration was in 'accepted'
    but resource claims had been done. 'accepted' migrations were not
    considered in progress before the fix for 1845146 merged as commit
    6ec686c26b, which caused the periodic task to incorrectly free the
    migration's resources from the destination. This patch adds a test
    that triggers this race by wrapping around the compute manager's
    live_migration() (which sets the 'queued' migration status - this was
    actually wrong in 6ec686c26b, as it talks about 'preparing') and
    running the update available resources periodic task while the
    migration is still in 'accepted'.

    Related bug: 1845146

    Change-Id: I78e79112a9c803fb45d828cfb4641456da66364a