Excessive resource consumption looking for containers to sync

Bug #1476623 reported by Eran Rom
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Undecided
Eran Rom

Bug Description

With every run, the container sync daemon iterates over all containers in the node looking for containers to sync.
This is a waste of resources, esp. when there is a small fraction of the containers that are configured for sync.

the issue was raised as part of a discussion on the need to improve the container sync performance.

CVE References

Eran Rom (eranr)
Changed in swift:
assignee: nobody → Eran Rom (eranr)
status: New → In Progress
Revision history for this message
clayg (clay-gerrard) wrote :

To my knowledge two ideas has been discussed as a possible resolution to this issue:

1) when a daemon that already walks the tree finds a database with container_sync keys in set in the stats table it drops a pointer to the container in a smaller tree that can be walked more efficiently by the container-sync daemon

2) when the replicator encounters a container with contaienr_sync keys it replicates the rows into sharded container sync accounts exactly like the reconciler, and the container_sync daemon is redesigned to horizontally scale using coordinated processes like the expirier.

Thank you for documenting this issue. Please feel free to jot down/discuss any other ideas you are pursuing.

Revision history for this message
Eran Rom (eranr) wrote :

Yeah, I spent some time considering both options, and decided to go for the first one.
I would say that at the high level the tradeoff between the two is simplicity (1) Vs. fairness (2)

I decided for now to go with the simpler approach (at least as a starting point) as there is going to be the addition of parallelism to the container sync daemon, and I prefer to take this in small steps.

Thanks very much for the feedback!

Revision history for this message
clayg (clay-gerrard) wrote :

Swift Design Principles #2 - Simple Scales

it's not just a describing the idea of building up large scale architectures with smaller simpler components (I think the kids now-a-days call this a "micro-service" architecture?), it's also a riff on Kernighans's old adage about being smart enough to debug your cleverness ;)

I think #1 is a good call. Although I have some vested interest in #2 as well [1].

1. https://review.openstack.org/#/c/103779/

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

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

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

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

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

Reviewed: https://review.openstack.org/205804
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=69e7424d3cb6c59341d739c3d41f42397031d96a
Submitter: Jenkins
Branch: master

commit 69e7424d3cb6c59341d739c3d41f42397031d96a
Author: Eran Rom <email address hidden>
Date: Tue Jul 28 09:41:12 2015 +0300

    Add container sync probe test to SAIO default set

    SAIO Configuration and documentation changes enabling to run the
    container sync probe test by default

    Change-Id: Iccf59533d0d4fe72549d318339ab125d04dde006
    Related-Bug: #1476623

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to swift (feature/crypto)

Related fix proposed to branch: feature/crypto
Review: https://review.openstack.org/219775

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to swift (feature/crypto)
Download full text (43.3 KiB)

Reviewed: https://review.openstack.org/219775
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=257e468e9bfd1088040419ad408106ac3c77b531
Submitter: Jenkins
Branch: feature/crypto

commit e02609c66a804845672413b06830b87395afef31
Author: Samuel Merritt <email address hidden>
Date: Tue Sep 1 15:19:50 2015 -0700

    Preserve traceback in swift-dispersion-report

    Commit c690bcb fixed a bug in the dispersion report, but changed this
    from a bare "raise" to "raise err", which loses the traceback. Not a
    big deal, but worth putting back IMO.

    Change-Id: Id5b72153a4b8df8e3faaf1fa3fb2040e28ba85cc

commit d06d4ad0fd2dfe69da8008e729651264522c6c06
Author: Minwoo Bae <email address hidden>
Date: Tue Sep 1 15:08:44 2015 -0500

    Included reference in swift.obj.diskfile to enumerate the string
    used for data file paths.

    Change-Id: Ie22caa678bc00dfc43fabec7efbbb9f34490f1b5

commit 524c89b7eeff037b8a6b421888771e15f98c2da2
Author: John Dickinson <email address hidden>
Date: Fri Aug 21 13:39:41 2015 -0700

    Updated CHANGELOG, AUTHORS, and .mailmap for 2.4.0 release.

    Change-Id: Ic6301146b839c9921bb85c4f4c1e585c9ab66661

commit 05de1305a903ee4ce9c8c50fde53c552d5b90d51
Author: Clay Gerrard <email address hidden>
Date: Thu Aug 27 18:35:09 2015 -0700

    Make ssync_sender send valid chunked requests

    The connect method of ssync_sender tells the remote connection that it's
    going to send a valid HTTP chunked request, but if the remote end needs
    to respond with an error of any kind sender throws HTTP right out the
    window, picks up his ball, and closes the socket down hard - much to the
    surprise of the eventlet.wsgi server who up to this point had been
    playing along quite nicely with this 'SSYNC' nonsense assuming that
    everyone here is consenting mature adults.

    If you're going to make a "Transfer-Encoding: chunked" request have the
    good decency to finish the job with a proper '0\r\n\r\n'. [1]

    N.B. It might be possible to handle an error status during the
    initialize_request phase with some sort of 100-continue support, but
    honestly it's not entirely clear to me when the server isn't going to
    close the connection if the client is still expected to send the body
    [2] - further if the error comes later during missing_check or updates
    we'll for sure want to send the chunk transfer termination line before
    we close down the socket and this way we cover both.

    1. Really, eventlet.wsgi shouldn't be so blasted brittle about this [3]
    2. https://lists.w3.org/Archives/Public/ietf-http-wg/2005AprJun/0007.html
    3. https://github.com/eventlet/eventlet/commit/c3ce3eef0b4d0dfdbfb1ec0186d4bb204fb8ecd5

    Closes-Bug #1489587
    Change-Id: Ic17c6c3075553f8cf6ef6213e62a00282f0d01cf

commit 993ee4e37af1961adba2047d5aa2eb210e423eb3
Author: nakagawamsa <email address hidden>
Date: Fri Aug 28 11:49:43 2015 +0900

    Remove duplicate X-Backend-Storage-Policy-Index key

    There is duplicate 'X-Backend-Storage-Policy-Index' dictionary key in unit.obj.test_server.py.
    One key has fixed policy index value, and another ha...

tags: added: in-feature-crypto
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to swift (feature/hummingbird)

Related fix proposed to branch: feature/hummingbird
Review: https://review.openstack.org/221410

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to swift (feature/hummingbird)
Download full text (70.7 KiB)

Reviewed: https://review.openstack.org/221410
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=eb8f1f83f1cfc63d8452bc30096fd1c145781527
Submitter: Jenkins
Branch: feature/hummingbird

commit cb683d391cb66d0f52830de16760c80fd2afedf9
Author: OpenStack Proposal Bot <email address hidden>
Date: Sat Sep 5 06:17:51 2015 +0000

    Imported Translations from Transifex

    For more information about this automatic import see:
    https://wiki.openstack.org/wiki/Translations/Infrastructure

    Change-Id: I2d92b8e34a665fb0bb4c048cfb0c59de295dfce6

commit e4542455c8a07b7981c247df8b737816062c1655
Author: Emett Speer <email address hidden>
Date: Wed Sep 2 17:18:03 2015 -0700

    [Labs] Update links to Cloud Admin Guide

    Update links to the Cloud Admin Guide after the
    RST conversion of that book altered URLs.

    Change-Id: I899f8938498b744e62887968a65e58c00ef27f1b

commit 58fcc07523978306cd3889ada73af5d9e664cf59
Author: Christian Schwede <email address hidden>
Date: Wed Sep 2 10:52:34 2015 +0000

    Test if container_sweep is executed on unmounted devices

    This change ensures that container_sweep is not run if a device is not mounted
    and mount_check is set to True.

    Change-Id: I823083c8431d9e61fd426508033ec9188503957b

commit e02609c66a804845672413b06830b87395afef31
Author: Samuel Merritt <email address hidden>
Date: Tue Sep 1 15:19:50 2015 -0700

    Preserve traceback in swift-dispersion-report

    Commit c690bcb fixed a bug in the dispersion report, but changed this
    from a bare "raise" to "raise err", which loses the traceback. Not a
    big deal, but worth putting back IMO.

    Change-Id: Id5b72153a4b8df8e3faaf1fa3fb2040e28ba85cc

commit d06d4ad0fd2dfe69da8008e729651264522c6c06
Author: Minwoo Bae <email address hidden>
Date: Tue Sep 1 15:08:44 2015 -0500

    Included reference in swift.obj.diskfile to enumerate the string
    used for data file paths.

    Change-Id: Ie22caa678bc00dfc43fabec7efbbb9f34490f1b5

commit 615c7a204b9386e05c5bab658bfe96766ad1e680
Author: Brian Cline <email address hidden>
Date: Tue Sep 1 10:51:20 2015 -0500

    Adds useful dispersion info from changelog

    Change-Id: I1a45088fc32620b02ff9a754b02ec1eb75a59d6e

commit 3b8755098a1786c5447abf158bd686293a82977c
Author: janonymous <email address hidden>
Date: Sun Aug 2 21:29:13 2015 +0530

    Replace a / b with a // b to use integer division where needed

    Change-Id: I72c81faa62786e140b0de00e3a04934bf1b5adbd

commit 524c89b7eeff037b8a6b421888771e15f98c2da2
Author: John Dickinson <email address hidden>
Date: Fri Aug 21 13:39:41 2015 -0700

    Updated CHANGELOG, AUTHORS, and .mailmap for 2.4.0 release.

    Change-Id: Ic6301146b839c9921bb85c4f4c1e585c9ab66661

commit 05de1305a903ee4ce9c8c50fde53c552d5b90d51
Author: Clay Gerrard <email address hidden>
Date: Thu Aug 27 18:35:09 2015 -0700

    Make ssync_sender send valid chunked requests

    The connect method of ssync_sender tells the remote connection that it's
    going to send a valid HTTP chunked request, but if the remote end needs
    to respond with an error of any kind sender th...

tags: added: in-feature-hummingbird
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.openstack.org/205803
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=85a0a6a28e166bc076cf8786de2b46248d8786a2
Submitter: Jenkins
Branch: master

commit 85a0a6a28e166bc076cf8786de2b46248d8786a2
Author: Eran Rom <email address hidden>
Date: Sun Jul 26 13:31:17 2015 +0300

    Container-Sync to iterate only over synced containers

    This change introduces a sync_store which holds only containers that
    are enabled for sync. The store is implemented using a directory
    structure that resembles that of the containers directory, but has
    entries only for containers enabled for sync.
    The store is maintained in two ways:
    1. Preemptively by the container server when processing
    PUT/POST/DELETE operations targeted at containers with
    x-container-sync-key / x-container-sync-to
    2. In the background using the containers replicator
    whenever it processes a container set up for sync

    The change updates [1]
    [1] http://docs.openstack.org/developer/swift/overview_container_sync.html

    Change-Id: I9ae4d4c7ff6336611df4122b7c753cc4fa46c0ff
    Closes-Bug: #1476623

Changed in swift:
status: In Progress → Fix Released
Revision history for this message
Doug Hellmann (doug-hellmann) wrote : Fix included in openstack/swift 2.6.0

This issue was fixed in the openstack/swift 2.6.0 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/crypto)

Fix proposed to branch: feature/crypto
Review: https://review.openstack.org/272201

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (feature/crypto)
Download full text (30.2 KiB)

Reviewed: https://review.openstack.org/272201
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=f9b7fd3074b5b0e5d6ea879d4144f7bfeec5d46b
Submitter: Jenkins
Branch: feature/crypto

commit e13a03c379273ee10e678818078b9c40a96a7dc9
Author: Tim Burke <email address hidden>
Date: Wed Jan 20 16:06:26 2016 -0800

    Stop overriding builtin range

    Change-Id: I315f8b554bb9e96659b455f4158f074961bd6498

commit 0a404def7d54d1ef1c85c11a378052260c4fda4c
Author: John Dickinson <email address hidden>
Date: Wed Jan 20 15:19:35 2016 -0800

    remove unneeded duplicate dict keys

    Change-Id: I926d7aaa9df093418aaae54fe26e8f7bc8210645

commit 221f94fdd39fd2dcd9a2e5565adceab615d55913
Author: John Dickinson <email address hidden>
Date: Tue Jan 19 14:50:24 2016 -0800

    authors and changelog updates for 2.6.0

    Change-Id: Idd0ff9e70abc0773be183c37cd6125fe852da7c0

commit 58359269b0e971e52f0eb7f97221566ca2148014
Author: Samuel Merritt <email address hidden>
Date: Tue Dec 8 16:36:05 2015 -0800

    Fix memory/socket leak in proxy on truncated SLO/DLO GET

    When a client disconnected while consuming an SLO or DLO GET response,
    the proxy would leak a socket. This could be observed via strace as a
    socket that had shutdown() called on it, but was never closed. It
    could also be observed by counting entries in /proc/<pid>/fd, where
    <pid> is the pid of a proxy server worker process.

    This is due to a memory leak in SegmentedIterable. A SegmentedIterable
    has an 'app_iter' attribute, which is a generator. That generator
    references 'self' (the SegmentedIterable object). This creates a
    cyclic reference: the generator refers to the SegmentedIterable, and
    the SegmentedIterable refers to the generator.

    Python can normally handle cyclic garbage; reference counting won't
    reclaim it, but the garbage collector will. However, objects with
    finalizers will stop the garbage collector from collecting them* and
    the cycle of which they are part.

    For most objects, "has finalizer" is synonymous with "has a __del__
    method". However, a generator has a finalizer once it's started
    running and before it finishes: basically, while it has stack frames
    associated with it**.

    When a client disconnects mid-stream, we get a memory leak. We have
    our SegmentedIterable object (call it "si"), and its associated
    generator. si.app_iter is the generator, and the generator closes over
    si, so we have a cycle; and the generator has started but not yet
    finished, so the generator needs finalization; hence, the garbage
    collector won't ever clean it up.

    The socket leak comes in because the generator *also* refers to the
    request's WSGI environment, which contains wsgi.input, which
    ultimately refers to a _socket object from the standard
    library. Python's _socket objects only close their underlying file
    descriptor when their reference counts fall to 0***.

    This commit makes SegmentedIterable.close() call
    self.app_iter.close(), thereby unwinding its generator's stack and
    making it eligible for garbage collection.

    * in Python < 3...

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/crypto)

Fix proposed to branch: feature/crypto
Review: https://review.openstack.org/277950

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on swift (feature/crypto)

Change abandoned by Alistair Coles (<email address hidden>) on branch: feature/crypto
Review: https://review.openstack.org/277950

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/hummingbird)

Fix proposed to branch: feature/hummingbird
Review: https://review.openstack.org/290148

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (feature/hummingbird)
Download full text (71.7 KiB)

Reviewed: https://review.openstack.org/290148
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=0f7f1de233919a0b046349a3e31ae7fc8675a1c5
Submitter: Jenkins
Branch: feature/hummingbird

commit d6b4587a554b51ba733b151e0d924735b63d07e0
Author: Olga Saprycheva <email address hidden>
Date: Tue Mar 8 10:57:56 2016 -0600

    Removed redundant file for flake8 check

    Change-Id: I4322978aa20ee731391f7709bbd79dee140fc703

commit 643dbce134140530eef2ae62c42fef1107f905ed
Author: OpenStack Proposal Bot <email address hidden>
Date: Tue Mar 8 06:35:49 2016 +0000

    Imported Translations from Zanata

    For more information about this automatic import see:
    https://wiki.openstack.org/wiki/Translations/Infrastructure

    Change-Id: I96b8ff1287bf219c5f8d56a3a4868c1063a953f9

commit 83713d37f0331c5ce9d377f4b4e8724551ae30ca
Author: Daisuke Morita <email address hidden>
Date: Mon Mar 7 18:30:47 2016 -0800

    Missing comments for storage policy parameter

    There are missing comments about storege_policy_index so appropriate
    comments are added.

    Change-Id: I3de3f0e6864e65918ca1a13cce70f19c23d295f5

commit 2cff2dec3d1c4588f5103e39679c43b3dded6dcb
Author: Olga Saprycheva <email address hidden>
Date: Fri Mar 4 15:19:39 2016 -0600

    Fixed pep8 and flake8 errors in doc/source/conf.py and updated flake8 commands in tox.ini to test it.

    Change-Id: I2add370e4cfb55d1388e3a8b41f688a7f3f2c621

commit 043fbca6d08648baa314ea2236f1ccdca8785f16
Author: Christian Schwede <email address hidden>
Date: Fri Mar 4 09:33:17 2016 +0000

    Remove Erasure Coding beta status from docs

    This removes notes stating support for Erasure coding as beta. Questions
    regarding the stability of EC are coming up regularly, and are often referring
    to the docs that state EC as still in beta.

    Besides this, a note marking statsd support as beta has been removed as well.

    Change-Id: If4fb6a5c4cb741d42953db3cee8cb17a1d774e15

commit 09c73b86e9255f28fbd4cf571a52c17d549a8f9a
Author: Pete Zaitcev <email address hidden>
Date: Thu Mar 3 10:24:28 2016 -0700

    Fix a crash in exception printout

    Says the number of arguments does not match the number of '%'.

    Change-Id: I8b5e395a07328fb9d4ac7a19f8ed2ae1637bee3b

commit fad5fabe0a22e8a86635a66523dd3d3d3b1fa705
Author: Tim Burke <email address hidden>
Date: Thu Mar 3 15:07:08 2016 +0000

    During functional tests, 404 response to a DELETE is successful

    Previously, we would only consider 204 responses successful, which would
    cause some spurious gate failures, such as

    http://logs.openstack.org/66/287666/3/check/gate-swift-dsvm-functional/c6d2673/console.html#_2016-03-03_13_41_07_846

    Change-Id: Ic8c300647924352a297a2781b50064f7657038b4

commit e91de49d6864b3794f8dc5acd9c1bf0c2f7409d1
Author: Alistair Coles <email address hidden>
Date: Mon Aug 10 10:30:10 2015 -0500

    Update container on fast-POST

    This patch makes a number of changes to enable content-type
    metadata to be updated when using the fast-POST mode of
    operation, as proposed in the associated spec ...

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.