glance-api workers should default to number of CPUs available

Bug #1333325 reported by Matt Riedemann
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Glance
Fix Released
Undecided
Matt Riedemann

Bug Description

The docs recommend setting the 'workers' option equal to the number of CPUs on the host but defaults to 1. I proposed a change to devstack to set workers=`nproc` but it was decided to move this into glance itself:

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

Note that nova changed in Icehouse to default to number of CPUs available also, and Cinder will most likely be doing the same for it's osapi_volume_workers option.

This will have a DocImpact and probably UpgradeImpact is also necessary since if you weren't setting the workers value explicitly before the change you'll now have `nproc` glance API workers by default after restarting the service.

Revision history for this message
Matt Riedemann (mriedem) wrote :

This is the related nova change: https://review.openstack.org/69266

Revision history for this message
Matt Riedemann (mriedem) wrote :

Added oslo since I'd like to move nova.utils.cpu_count() from nova into oslo-incubator, thinking service.py module, so that it can be re-used in glance and cinder.

https://review.openstack.org/#/c/69266/1/nova/utils.py

Revision history for this message
Matt Riedemann (mriedem) wrote :

Hmm, glance doesn't import the service module from oslo, so that might not work, I don't really want to import that module just for that simple utility.

Matt Riedemann (mriedem)
no longer affects: oslo
Matt Riedemann (mriedem)
Changed in glance:
status: New → In Progress
assignee: nobody → Matt Riedemann (mriedem)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (master)

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

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

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (master)
Download full text (3.9 KiB)

Reviewed: https://review.openstack.org/102664
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=dde2cbafd33b69024e36c741402c8c130ab761e8
Submitter: Jenkins
Branch: master

commit dde2cbafd33b69024e36c741402c8c130ab761e8
Author: Matt Riedemann <email address hidden>
Date: Wed Jun 25 11:03:29 2014 -0700

    Sync processutils and lockutils from oslo with deps

    This is a sync of processutils and lockutils from oslo-incubator
    along with their dependencies.

    The main target for the sync is to pick up the get_worker_count() method
    in commit 85f178489a128a04a7ee3ed44018403caa109ef0 so that we can set
    glance-api workers equal to the number of CPUs on the host.

    lockutils is also copied over due to the nature running multiple
    workers and how the glance functional tests are setup, it'd be
    prudent to have the latest lockutils code since it hasn't been updated
    in over six months.

    The change to require posix_ipc is due to commit
    edce46cf5efd6d738d379205f9bf526a498845e3 in lockutils.

    Changes:

    processutils
    ------------
    85f1784 Move nova.utils.cpu_count() to processutils module
    cdcc19c Mask passwords that are included in commands
    8a0f567 Remove str() from LOG.* and exceptions
    51778f9 Allow passing environment variables to execute()
    fcf517d Update oslo log messages with translation domains
    af41592 Catch OSError in processutils
    f773ea2 Fix i18n problem in processutils module
    8b2b0b7 Use hacking import_exceptions for gettextutils._
    3b71f46 Fixed misspellings of common words
    12bcdb7 Remove vim header
    a4dab73 Correct execute() to check 0 in check_exit_code
    d6a963e Fix processutils.execute errors on windows
    aa5b658 Allow passing a logging level to processutils.execute
    1a2df89 Enable H302 hacking check
    7119e29 Enable hacking H404 test.
    2f01388 Use Python 3.x compatible except construct

    lockutils
    ---------
    de4adbc pep8: fixed multiple violations
    9e88af1 fixed typos found by RETF rules
    fe3389e Improve help strings
    f3f14c9 Fixed several typos
    0f495ee Emit a log statement when releasing internal lock
    f0dd798 Remove rendundant parentheses of cfg help strings
    006d9a2 Allow external locks to work with threads
    9b73c19 Re-enable file-based locking behavior
    edce46c Use Posix IPC in lockutils
    ac84a40 Update log translation domains
    fcf517d Update oslo log messages with translation domains
    37a1de8 Move the released file lock to the successful path
    b0d0c33 Add remove external lock files API in lockutils
    4f6190a Use threading.ThreadError instead of reraising IOError
    195f0b1 Have the interprocess lock follow lock conventions
    15cca4b lockutils: move directory creation in lock class
    81fe39e lockutils: remove lock_path parameter
    14d3669 lockutils: expand add_prefix
    dc06d55 lockutils: do not grab the lock in creators
    a0948cb lockutils: remove local usage
    df8e051 lockutils: split code handling internal/external lock

    log
    ---
    109e325 Use oslo.messaging to publish log errors
    de4adbc pep8: fixed multiple v...

Read more...

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

Reviewed: https://review.openstack.org/102665
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=cb7b189f4cd425cad3cbea6ca71986216416cec7
Submitter: Jenkins
Branch: master

commit cb7b189f4cd425cad3cbea6ca71986216416cec7
Author: Matt Riedemann <email address hidden>
Date: Wed Jun 25 14:18:36 2014 -0700

    Use (# of CPUs) glance workers by default

    The config docs have historically recommended that the number of
    glance workers should be set to the number of CPUs available for
    best performance, so we should make this the default.

    Commit 75c96a48fc7e5dfb59d8258142b01422f81b0253 did the same thing in
    Nova in Icehouse and the plan is to do the same thing for Cinder
    and Trove workers.

    The config files are updated to match the help string in the code.

    There is no upgrade impact since glance-api.conf previously
    hard-coded the workers value to 1 so anyone upgrading to this
    will still get whatever value was set in glance-api.conf prior
    to this change.

    DocImpact: glance workers will now be equal to the number of
               CPUs available by default if not explicitly specified
               in glance-api.conf and/or glance-registry.conf.

    UpgradeImpact: There is no upgrade impact to glance-api workers
               since glance-api.conf previously hard-coded the workers
               value to 1 so anyone upgrading to tihs will still get
               whatever value was set in glance-api.conf prior to
               this change. There is an upgrade impact to the
               glance-registry workers since glance-registry.conf
               did not hard-code the workers value to 1 before change
               I0cee0d284eef9ce5dcb26720499f2c4d31eaca0f, which is
               overwritten here. So anyone upgrading to this change
               that does not have workers specified in
               glance-registry.conf will now be running multiple
               workers by default when they restart the glance
               registry service.

    Closes-Bug: #1333325

    Change-Id: I6795c6e22268bb3fb67331edc7af641aefa904cc

Changed in glance:
status: In Progress → Fix Committed
Matt Riedemann (mriedem)
Changed in glance:
milestone: none → juno-2
Changed in glance:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in glance:
milestone: juno-2 → 2014.2
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.