placement os-traits sync checked every request

Bug #1756151 reported by Chris Dent
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
Low
Chris Dent

Bug Description

Most requests to the placement service eventually reach the database code in the resource_provider.py file and as a result eventually run _ensure_trait_sync to make sure that this process has synced the os-traits library to its traits database table.

While there is a flag to make sure that the syncing doesn't happen if it already happened before, we lock around checking that flag for nearly every request. This isn't the end of the world, but it is wasted activity and it can make a lot of noise in the logs if you're running DEBUG:

2018-03-15 17:35:37.653 7 DEBUG oslo_concurrency.lockutils [req-7904bbbe-52f7-420f-b058-13779dc018d1 admin admin - - -] Acquired semaphore "trait_sync" lock /usr/lib/python3.6/site-packages/oslo_concurrency/lockutils.py:212
2018-03-15 17:35:37.653 7 DEBUG oslo_concurrency.lockutils [req-7904bbbe-52f7-420f-b058-13779dc018d1 admin admin - - -] Releasing semaphore "trait_sync" lock /usr/lib/python3.6/site-packages/oslo_concurrency/lockutils.py:228

This is redundant. I'm pretty sure I wrote this code, so I'm not sure what I was thinking, probably cargo culting off ensuring the resource classes. We only need to sync the traits to the database once and we only need to check if it has been done once per process. The traits are read from a python module that is only imported once per process.

So, what we could do is move the calling of ensure_trait_sync to the part of placement that establishes the database connection facade thingie. If we merge https://review.openstack.org/#/c/541435/ (which moves placement database connection establishment to its own file) or something like it, we have an easy place to do it, at server boot time.

We find the session, make a context, do the sync, set the global and never worry about it again. All the _ensure_trait_sync calls can be removed.

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

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

Changed in nova:
assignee: nobody → Chris Dent (cdent)
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

Reviewed: https://review.openstack.org/553857
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=0372d82c2c5ae5651b00c5ab9e1cf5c77a8f87d9
Submitter: Zuul
Branch: master

commit 0372d82c2c5ae5651b00c5ab9e1cf5c77a8f87d9
Author: Chris Dent <email address hidden>
Date: Fri Mar 16 16:54:14 2018 +0000

    Ensure that os-traits sync is attempted only at start of process

    Traits sync had been tried any time a request that might involve
    traits was called. If the global was set no syncing was done, but
    lock handling was happening.

    This change moves the syncing into the the deploy.load_app() handling.
    This means that the syncing will be attempted any time a new WSGI
    application is created. Most of the time this will be at the start of a
    new process, but some WSGI servers have interesting threading models so
    there's a (slim) possibility that it could be in a thread. Because of
    this latter possibility, the locking is still in place.

    Functional tests are updated to explicitly do the sync in their
    setUp(). Some changes in fixtures are required to make sure that
    the database is present prior to the sync.

    While these changes are not strictly part of extracting placement, the
    consolidation and isolation of database handling code makes where to put
    this stuff a bit cleaner and more evident: an update_database() method
    in deploy uses an empty DbContext class from db_api to the call the
    ensure_trait_sync method in resource_provider. update_database is in
    deploy because it an app deployment task and because putting it in
    db_api leads to circual import problems.

    blueprint placement-extract
    Closes-Bug: #1756151

    Change-Id: Ic87518948ed5bf4ab79f9819cd94714e350ce265

Changed in nova:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 18.0.0.0b3

This issue was fixed in the openstack/nova 18.0.0.0b3 development milestone.

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.