resource-list makes SQL calls for every resource which is a nested stack

Bug #1578854 reported by Steve Baker
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Heat
Fix Released
High
Steve Baker

Bug Description

Rather than doing a single query for resources, doing a resource-list appears to perform extra queries for any resources which are also stacks (note, this isn't even a --show-nested)

Will comment with SQL logs when I have a small test case

Revision history for this message
Steven Hardy (shardy) wrote :

Illustration from a real deployment that shows how bad this is:

[stack@r710-undercloud ~]$ time heat resource-list overcloud -n 5 | grep -i failed | ComputeNodesPostDeployment | 3ffeec84-3a25-46b8-b219-4f2bfce141fe | OS::TripleO::ComputePostDeployment | UPDATE_FAILED | 2016-05-06T09:10:08 | overcloud |
| ComputeArtifactsDeploy | 189eb75d-ef4e-4efb-89d1-2c3fa94e55f7 | OS::Heat::StructuredDeployments | UPDATE_FAILED | 2016-05-06T09:17:06 | overcloud-ComputeNodesPostDeployment-ws3b7l4bgvh5 |

real 13m23.083s
user 0m12.094s
sys 0m0.510s

Changed in heat:
status: New → Confirmed
importance: Undecided → High
milestone: none → newton-1
tags: added: tripleo
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to heat (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/314336

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to heat (master)

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

Changed in heat:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
Steve Baker (steve-stevebaker) wrote :

13 minutes for an overcloud resource list!?

What is the node count of this overcloud

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to heat (master)

Reviewed: https://review.openstack.org/314418
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=50f5142eca3fcff27730afcc4fb2b179934867f3
Submitter: Jenkins
Branch: master

commit 50f5142eca3fcff27730afcc4fb2b179934867f3
Author: Steve Baker <email address hidden>
Date: Tue May 10 17:35:15 2016 +1200

    Remove stack ObjectField from resource

    This causes the stack record to be loaded every time a resource is
    loaded, and it is not used for anything other than getting the stack
    ID, which is already available via the stack_id field.

    Change-Id: I45ce9d18984f4881151dba496482713a62c9eae9
    Partial-Bug: #1578854

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to heat (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/317217

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Related fix proposed to branch: master
Review: https://review.openstack.org/317218

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Related fix proposed to branch: master
Review: https://review.openstack.org/317219

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to heat (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on heat (master)

Change abandoned by Steve Baker (<email address hidden>) on branch: master
Review: https://review.openstack.org/315800

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to heat (master)

Reviewed: https://review.openstack.org/314806
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=0ef5d601c02da766d3d00712bd70b4912c66bcad
Submitter: Jenkins
Branch: master

commit 0ef5d601c02da766d3d00712bd70b4912c66bcad
Author: Steve Baker <email address hidden>
Date: Wed May 11 10:38:20 2016 +1200

    Don't load nested stack to build the identifier

    StackResource is already building HeatIdentifier for the nested stack,
    so this should be used by the resource formatter to avoid the overhead
    of loading the nested stack.

    Change-Id: Iab7a997c0b4d9dcb8ceb3d2f49692216fe611df1
    Partial-Bug: #1578854

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to heat (master)

Reviewed: https://review.openstack.org/317217
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=c62e1b325a536294b3285f8cbcad7d66a415ee23
Submitter: Jenkins
Branch: master

commit c62e1b325a536294b3285f8cbcad7d66a415ee23
Author: Steve Baker <email address hidden>
Date: Mon May 16 12:59:59 2016 +1200

    Use a weakref for the data object context

    There are no known circular reference issues caused by storing the
    context in data objects, but the following changes will refer to data
    objects in the context, so this change prevents any later issues.

    Change-Id: I3680e5678003cf339a98fbb7a2b1b387fb2243c0
    Related-Bug: #1578854

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/317218
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=5da09eae94e0f6fa9c93f8c724190553e5b0f562
Submitter: Jenkins
Branch: master

commit 5da09eae94e0f6fa9c93f8c724190553e5b0f562
Author: Steve Baker <email address hidden>
Date: Tue May 17 10:57:46 2016 +1200

    Allow creation of cache classes associated with a context

    This will allow heat.object implementations to create a single object
    per context to manage its particular context caching needs.

    Change-Id: I9b626efee45164617a73b790bfad4808172d2c12
    Related-Bug: #1578854

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to heat (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to heat (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/323998

Thomas Herve (therve)
Changed in heat:
milestone: newton-1 → newton-2
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to heat (master)

Reviewed: https://review.openstack.org/317219
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=de99472c758d9785e3bba70ca62c35d6d8c352a4
Submitter: Jenkins
Branch: master

commit de99472c758d9785e3bba70ca62c35d6d8c352a4
Author: Steve Baker <email address hidden>
Date: Mon May 16 11:01:31 2016 +1200

    DB query to get all resources by the root stack

    To prevent a resource query for every nested stack during a
    resource-list, there needs to be a way to fetch every resource in a
    single query.

    Change-Id: Ib05b2166d6c7584a844e1ab4a5dd6e35437c96c4
    Related-Bug: #1578854

Revision history for this message
Steven Hardy (shardy) wrote :

> 13 minutes for an overcloud resource list!?
> What is the node count of this overcloud

Pretty big I believe, several hundred computes (perhaps nearly 300), but heat was running on some very capable baremetal hardware, so it still should really be much better than this..

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/323998
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=091ed70d5333cff1586ec5cd0f1788ec30609766
Submitter: Jenkins
Branch: master

commit 091ed70d5333cff1586ec5cd0f1788ec30609766
Author: Zane Bitter <email address hidden>
Date: Thu Jun 2 11:41:39 2016 -0400

    Fix the definition of has_nested()

    Since Iab7a997c0b4d9dcb8ceb3d2f49692216fe611df1 StackResource.has_nested()
    returns True unconditionally. However, some parts of the code expect that
    if has_nested() returns True, the caller will be able to call nested() and
    not get None returned (this was the original definition of has_nested()).
    This change restores the definition without the need to load the nested
    stack (which is expensive, and the reason for changing this in the first
    place).

    This also allows the change from the original patch to be a bit tidier,
    without a recurrence of the issue it initially caused (bug 1586906) that
    was later fixed by I2ededcc57057f43206922b99bb72c43332336814.

    Change-Id: Id26dc62b7513853b3f02a656518c59b3ff8d509a
    Related-Bug: #1578854

Thomas Herve (therve)
Changed in heat:
milestone: newton-2 → newton-3
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to heat (master)

Reviewed: https://review.openstack.org/317220
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=bc3b84fb609985511510aff2aee62dbf05268b81
Submitter: Jenkins
Branch: master

commit bc3b84fb609985511510aff2aee62dbf05268b81
Author: Steve Baker <email address hidden>
Date: Sat Aug 13 15:23:08 2016 +1200

    A context cache for Resource objects

    A context cache which memoizes the resources fetched by calls to
    Resource.get_all_by_root_stack(..., cache=True)
    which are recalled by subsequent calls to Resource.get_all_by_stack.

    Because get_all_by_stack returns a collection instead of a single
    resource, there is no way of taking advantage of the SQLAlchemy
    identity map [1].

    [1] http://docs.sqlalchemy.org/en/latest/orm/session_basics.html#is-the-session-a-cache

    Change-Id: Ia5aae0c86a586041020e9798566c9e0af48c180d
    Partial-Bug: #1578854

Thomas Herve (therve)
Changed in heat:
milestone: newton-3 → newton-rc1
Thomas Herve (therve)
Changed in heat:
milestone: newton-rc1 → none
milestone: none → ocata-1
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on heat (master)

Change abandoned by Steve Baker (<email address hidden>) on branch: master
Review: https://review.openstack.org/317221
Reason: The speed improvement would be at the cost of high memory usage, which we've only just fixed.

Rabi Mishra (rabi)
Changed in heat:
milestone: ocata-1 → ocata-2
Revision history for this message
Rabi Mishra (rabi) wrote :

Closing this as we don't have any more fixes in review for this atm.

Changed in heat:
status: In Progress → Fix Released
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.