placement server needs to retry allocations, server-side

Bug #1719933 reported by Chris Dent on 2017-09-27
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Medium
Chris Dent
Queens
Medium
Chris Dent

Bug Description

Long time ago a todo was made in placement: https://github.com/openstack/nova/blob/faede889d3620f8ff0131a7a4c6b9c1bc844cd06/nova/objects/resource_provider.py#L1837-L1839

We need to implement that TODO, this is a note to self.

This is related to what may be a different bug: when heavily loaded with many single requests, the placement server is unexpectedly receiving 409's about generation problems. Discussion of that led to remembering that this TODO needs to be fixed. Fixing the TODO needs to be done regardless of that problem.

Matt Riedemann (mriedem) wrote :

FYI, I've been trying to recreate this here to get the logs from a failure:

https://review.openstack.org/#/c/507918/

Jay Pipes (jaypipes) wrote :

Gonna take this one as I'm cleaning up resource_provider.py module in a patch series right now...

Changed in nova:
assignee: nobody → Jay Pipes (jaypipes)
Jay Pipes (jaypipes) wrote :
Changed in nova:
status: Triaged → Invalid
Jay Pipes (jaypipes) wrote :

I'll remove the no longer relevant comment in a patch shortly.

Fix proposed to branch: master
Review: https://review.openstack.org/509034

Changed in nova:
status: Invalid → In Progress
Chris Dent (cdent) wrote :

Isn't that retry decorator only retrying on db api errors, notably deadlock? When generation conflict happens, ConcurrentUpdateDetected is raised, which is not a db api error.

Jay Pipes (jaypipes) wrote :

I see, yes, Chris, that's a good point.

Change abandoned by Jay Pipes (<email address hidden>) on branch: master
Review: https://review.openstack.org/509034

Chris Dent (cdent) on 2017-11-28
Changed in nova:
status: In Progress → Triaged
Ed Leafe (ed-leafe) wrote :

We're not sure if this is a bug, or if it is, what the proper solution should be. We have a discussion about our use of generations to ensure integrity scheduled for the Dublin PTG, and this issue will be part of that.

Jay Pipes (jaypipes) on 2018-04-20
Changed in nova:
assignee: Jay Pipes (jaypipes) → nobody
Chris Dent (cdent) wrote :

It turns out that allocation write contention on resource provider generation is a relatively significant issue when launching many VMs to one or a very small number of compute nodes, as might happen in a clustered environment like the vmwareapi virtdriver.

The retries handling at https://github.com/openstack/nova/blob/d687e7d29b37b3cdc9e1bc429dec3a01be298f80/nova/scheduler/client/report.py#L103-L123 is insufficient when something like 1200vms are being created because there's always another vm being created concurrently for the same compute node.

Server side retries will help because there will be less latency but they won't fully fix it as the the architecture expects and assumes there will be at least a bit of horizontal diversity in resource providers.

Fix proposed to branch: master
Review: https://review.openstack.org/586048

Changed in nova:
assignee: nobody → Chris Dent (cdent)
status: Triaged → In Progress
Changed in nova:
assignee: Chris Dent (cdent) → Digambar (digambarpatil15)
Changed in nova:
assignee: Digambar (digambarpatil15) → Chris Dent (cdent)
Chris Dent (cdent) wrote :

Diga, my apologies forgetting to this before you. I had to rush to work on this because we encountered internally at work that would be solved by having a solution to it and it was a critical issue, so I rushed something out at https://review.openstack.org/#/c/586048/

If you could review that that would be great. I tried to find you on IRC to discuss it, but could not.

Chris Dent (cdent) on 2018-07-30
tags: added: queens-backport-potential
removed: note-to-self
Digambar (digambarpatil15) wrote :

Sure Chris. Sorry, I was not feeling well so was on leave for last two days.

Reviewed: https://review.openstack.org/586048
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=72e4c4c8d7fb146862b899337626485dad10f15b
Submitter: Zuul
Branch: master

commit 72e4c4c8d7fb146862b899337626485dad10f15b
Author: Chris Dent <email address hidden>
Date: Thu Jul 26 12:00:14 2018 +0100

    [placement] Retry allocation writes server side

    This change adds a fast retry loop around
    AllocationList._set_allocations if a resource provider generation
    conflict happens. It turns out that under high concurrency of allocation
    claims being made on the same resource provider conflicts can be quite
    common and client side retries are insufficient.

    Because both consumer generation and resource provider generations had
    raised the same exception there was no way to distinguish between the
    two so a child of ConcurrentUpdateDetected has been created as
    ResourceProviderConcurrentUpdateDetected. In the future this will allow
    us to send different error codes to the client as well, but that change
    is not done here.

    When the conflict is detected, all the resource providers in the
    AllocationList are reloaded and the list objects refreshed.

    Logging is provided to indicate:

    * at debug that a retry is going to happen
    * at warning that all the retries failed and the client is going to
      see the conflict

    The tests for this are a bit funky: Some mocks are used to cause the
    conflicts, then the real actions after a couple of iterations.

    Change-Id: Id614d609fc8f3ed2d2ff29a2b52143f53b3b1b9a
    Closes-Bug: #1719933

Changed in nova:
status: In Progress → Fix Released

Reviewed: https://review.openstack.org/588569
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=66a47b7623035cff38a12fdace9a325bbfdd9a14
Submitter: Zuul
Branch: stable/queens

commit 66a47b7623035cff38a12fdace9a325bbfdd9a14
Author: Chris Dent <email address hidden>
Date: Thu Jul 26 12:00:14 2018 +0100

    [placement] Retry allocation writes server side

    This change adds a fast retry loop around
    AllocationList._set_allocations if a resource provider generation
    conflict happens. It turns out that under high concurrency of allocation
    claims being made on the same resource provider conflicts can be quite
    common and client side retries are insufficient.

    Because both consumer generation and resource provider generations had
    raised the same exception there was no way to distinguish between the
    two so a child of ConcurrentUpdateDetected has been created as
    ResourceProviderConcurrentUpdateDetected. In the future this will allow
    us to send different error codes to the client as well, but that change
    is not done here.

    When the conflict is detected, all the resource providers in the
    AllocationList are reloaded and the list objects refreshed.

    Logging is provided to indicate:

    * at debug that a retry is going to happen
    * at warning that all the retries failed and the client is going to
      see the conflict

    The tests for this are a bit funky: Some mocks are used to cause the
    conflicts, then the real actions after a couple of iterations.

    This was backported from 72e4c4c8d7fb146862b899337626485dad10f15b with
    conflicts because exceptions, tests and object files were moved to
    new locations with Rocky and the AllocationList.create_all method was
    renamed to the more accurate replace_all. Prior to Rocky there were
    no Consumer objects and fewer helper methods in functional tests,
    so the test is adjusted accordingly.

    Change-Id: Id614d609fc8f3ed2d2ff29a2b52143f53b3b1b9a
    Closes-Bug: #1719933
    (cherry picked from commit 72e4c4c8d7fb146862b899337626485dad10f15b)

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

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

Change abandoned by Chris Dent (<email address hidden>) on branch: stable/pike
Review: https://review.openstack.org/590745

Change abandoned by Chris Dent (<email address hidden>) on branch: stable/ocata
Review: https://review.openstack.org/591042

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

Other bug subscribers