Container updater may be stuck and not make progress

Bug #1722951 reported by Timur Alperovich
32
This bug affects 5 people
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
High
Samuel Merritt
Ubuntu Cloud Archive
Fix Released
High
Unassigned
Ocata
Triaged
High
Unassigned
Pike
Triaged
High
Unassigned
Queens
Fix Released
High
Unassigned
Rocky
Fix Released
High
Unassigned
swift (Ubuntu)
Fix Released
High
Unassigned

Bug Description

Container updater may be stuck and not make further progress. This was reported after the "PipeMutex" primitive was added to fix a deadlock in the logging layer. The bug can be reproduced with the attached python script.

The observed behavior is that after some time the container stats are no longer updated and strace for the daemons shows that they are all stuck in "epoll_wait".

Revision history for this message
Timur Alperovich (timur-alperovich) wrote :
Revision history for this message
Samuel Merritt (torgomatic) wrote :

The bug is real, but isn't PipeMutex's fault. We're letting eventlet pick the default hub sometimes. Eventlet chooses the epoll hub, and epoll doesn't play nicely with forking. (It's possible to use epoll and fork and still be correct, but eventlet doesn't make it so.)

Changed in swift:
importance: Undecided → High
status: New → Confirmed
Changed in swift:
assignee: nobody → Samuel Merritt (torgomatic)
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/511550

Changed in swift:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.openstack.org/511550
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=dc8da5bb194d0a544fb8065aa9a4c7484f605715
Submitter: Zuul
Branch: master

commit dc8da5bb194d0a544fb8065aa9a4c7484f605715
Author: Samuel Merritt <email address hidden>
Date: Thu Oct 12 10:45:12 2017 -0700

    Use "poll" or "selects" Eventlet hub for all Swift daemons.

    Previously, Swift's WSGI servers, the object replicator, and the
    object reconstructor were setting Eventlet's hub to either "poll" or
    "selects", depending on availability. Other daemons were letting
    Eventlet use its default hub, which is "epoll".

    In any daemons that fork, we really don't want to use epoll. Epoll
    instances end up shared between the parent and all children, and you
    get some awful messes when file descriptors are shared.

    Here's an example where two processes are trying to wait on the same
    file descriptor using the same epoll instance, and everything goes
    wrong:

    [proc A] epoll_ctl(6, EPOLL_CTL_ADD, 3, ...) = 0

    [proc B] epoll_ctl(6, EPOLL_CTL_ADD, 3, ...) = -1 EEXIST (File exists)
    [proc B] epoll_wait(6, ...) = 1
    [proc B] epoll_ctl(6, EPOLL_CTL_DEL, 3, ...) = 0

    [proc A] epoll_wait(6, ...)

    This primarily affects the container updater and object updater since
    they fork. I've decided to change the hub for all Swift daemons so
    that we don't add multiprocessing support to some other daemon someday
    and suffer through this same bug again.

    This problem was made more apparent by commit 6d16079, which made our
    logging mutex use file descriptors. However, it could have struck on
    any shared file descriptor on which a read or write returned EAGAIN.

    Change-Id: Ic2c1178ac918c88b0b901e581eb4fab3b2666cfe
    Closes-Bug: 1722951

Changed in swift:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/s3api)

Fix proposed to branch: feature/s3api
Review: https://review.openstack.org/513730

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

Reviewed: https://review.openstack.org/513730
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=bb064f0b30ec083cec85e8371fe12824804779cf
Submitter: Zuul
Branch: feature/s3api

commit 0bca0d4d2bf95a37e4a403457ee5c7db4c0b6290
Author: Pete Zaitcev <email address hidden>
Date: Wed Oct 18 18:15:11 2017 -0500

    Be more tolerant of exception messages from sqlite

    Parsing the text of error messages was always perilous, perhaps.
    Either way, this should fix the problem of test_get() failing.

    Change-Id: I929c2f6e60624241075a292d020c5707ea0ff1c3

commit e001c02ff938d9aea560970fa70f3d8884a8ea33
Author: Tim Burke <email address hidden>
Date: Tue Oct 17 22:49:39 2017 +0000

    Stop logging tracebacks on bad xLOs

    The error messages alone should provide plenty enough information.
    Plus, running functional tests really shouldn't cause tracebacks.

    Also, tighten up tests for log messages.

    Change-Id: I55136484d342af756fa153d971dcb9159a435f13

commit 356b110229ee5e5be93ce1ec60ed2de8f75090bf
Author: Samuel Merritt <email address hidden>
Date: Wed Oct 18 15:09:12 2017 -0700

    Remove swift-temp-url man page.

    Commit 250da37a removed bin/swift-temp-url, but left the man page.

    Change-Id: Ic518f23678e3c3134f02a46a51a2bcb90d92bdc2

commit 1aecd1dfc61988f12d4cdfda7110dd648133e4cf
Author: Alistair Coles <email address hidden>
Date: Wed Oct 18 10:52:52 2017 +0100

    tighten up drop_privileges unit tests

    add more assertions about args that are passed to
    os module functions

    Related-Change: Ida15e72ae4ecdc2d6ce0d37bd99c2d86bd4e5ddc
    Change-Id: Iee483274aff37fc9930cd54008533de2917157f4

commit e6cf9b4758612f38f7317c52423f317b9a11bbc9
Author: Samuel Merritt <email address hidden>
Date: Tue Oct 17 15:16:43 2017 -0700

    Fix some spelling in a docstring

    Change-Id: I6b32238da3381848ae56aed1c570d299be72473e

commit 646f7507a1f5cf479b8d2023645009de0a28cd79
Author: Tim Burke <email address hidden>
Date: Tue Oct 17 21:17:57 2017 +0000

    Quiet test output when running test_utils.py in isolation

    Change-Id: I4cf85d3cd5a20424e9bbbdf0213120b3c3d4b837

commit 0f91c862e19bb8d81c20d03e65e6e6657f7265dc
Author: Corey Bryant <email address hidden>
Date: Tue Oct 17 15:04:45 2017 -0400

    Drop group comparison from drop_privileges test

    Drop the group comparison from drop_privileges test as it isn't
    valid since os.setgroups() is mocked.

    Change-Id: Ida15e72ae4ecdc2d6ce0d37bd99c2d86bd4e5ddc
    Closes-Bug: #1724342

commit 5438fe7d9bcb7dc99118c12bdf1ccfd663390b08
Author: Samuel Merritt <email address hidden>
Date: Fri Oct 13 17:03:18 2017 -0700

    Clean up some leftover Python 2.6-isms.

    Change-Id: I24f0aa3bf7314a669c4cf44eadc46e874d6803f1

commit 250da37a7b279b05a1ac8954b506add1661f194d
Author: Tim Burke <email address hidden>
Date: Fri Oct 13 23:27:11 2017 +0000

    Remove swift-temp-url script

    This has been deprecated since Swift 2.10.0 (Newton) including a
    message that it would go away. Let's actually remove it.

    Change-Id: I7d3659761c71119363ff2c0c750e37b4c6374a39
    Rela...

Read more...

tags: added: in-feature-s3api
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (stable/ocata)

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/517090

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

Fix proposed to branch: feature/deep
Review: https://review.openstack.org/517718

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

Reviewed: https://review.openstack.org/517718
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=5c564e53966b74d6d7589c3505dd33816b3c19f1
Submitter: Zuul
Branch: feature/deep

commit 77a8a4455d4a2bd0a9b9146a01acfe0d93fe7550
Author: Tim Burke <email address hidden>
Date: Tue Oct 3 00:11:07 2017 +0000

    Let clients request heartbeats during SLO PUTs

    An SLO PUT requires that we HEAD every referenced object; as a result, it
    can be a very time-intensive operation. This makes it difficult as a
    client to differentiate between a proxy-server that's still doing work and
    one that's crashed but left the socket open.

    Now, clients can opt-in to receiving heartbeats during long-running PUTs
    by including the query parameter

        heartbeat=on

    With heartbeating turned on, the proxy will start its response immediately
    with 202 Accepted then send a single whitespace character periodically
    until the request completes. At that point, a final summary chunk will be
    sent which includes a "Response Status" key indicating success or failure
    and (if successful) an "Etag" key indicating the Etag of the resulting SLO.

    This mechanism is very similar to the way bulk extractions and deletions
    work, and even the way SLO behaves for ?multipart-manifest=delete requests.

    Note that this is opt-in: this prevents us from sending the 202 response
    to existing clients that may mis-interpret it as an immediate indication
    of success.

    Co-Authored-By: Alistair Coles <email address hidden>
    Related-Bug: 1718811
    Change-Id: I65cee5f629c87364e188aa05a06d563c3849c8f3

commit 92705bb36b4a771a757977d11875e7b929c41741
Author: David Rabel <email address hidden>
Date: Thu Nov 2 12:38:41 2017 +0100

    Fix indent in overview_policies.rst

    Change-Id: I7f070956d8b996db798837392adfca4483067aea

commit feee3998408e5ed03563c317ad9506ead92083a6
Author: Clay Gerrard <email address hidden>
Date: Fri Sep 1 14:15:45 2017 -0700

    Use check_drive consistently

    We added check_drive to the account/container servers to unify how all
    the storage wsgi servers treat device dirs/mounts. Thus pushes that
    unification down into the consistency engine.

    Drive-by:
     * use FakeLogger less
     * clean up some repeititon in probe utility for device re-"mounting"

    Related-Change-Id: I3362a6ebff423016bb367b4b6b322bb41ae08764
    Change-Id: I941ffbc568ebfa5964d49964dc20c382a5e2ec2a

commit 29e9ae1cc5b74dce205643c101601555c06b67dc
Author: Tim Burke <email address hidden>
Date: Thu Oct 19 18:53:04 2017 +0000

    Make xml responses less insane

    Looking at bulk extractions:

       $ tar c *.py | curl http://saio:8090/v1/AUTH_test/c?extract-archive=tar \
          -H accept:application/xml -T -
       <?xml version="1.0" encoding="UTF-8"?>
       <delete>
       <number_files_created>2</number_files_created>
       <response_body></response_body>
       <response_status>201 Created</response_status>
       <errors>
       </errors>
       </delete>

    Or SLO upload failures:

       $ curl http://saio:8090/v1/AUTH_...

tags: added: in-feature-deep
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift 2.16.0

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (stable/ocata)

Reviewed: https://review.openstack.org/517090
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=633998983c2bd69c003bc343ff65872440be5439
Submitter: Zuul
Branch: stable/ocata

commit 633998983c2bd69c003bc343ff65872440be5439
Author: Samuel Merritt <email address hidden>
Date: Thu Oct 12 10:45:12 2017 -0700

    Use "poll" or "selects" Eventlet hub for all Swift daemons.

    Previously, Swift's WSGI servers, the object replicator, and the
    object reconstructor were setting Eventlet's hub to either "poll" or
    "selects", depending on availability. Other daemons were letting
    Eventlet use its default hub, which is "epoll".

    In any daemons that fork, we really don't want to use epoll. Epoll
    instances end up shared between the parent and all children, and you
    get some awful messes when file descriptors are shared.

    Here's an example where two processes are trying to wait on the same
    file descriptor using the same epoll instance, and everything goes
    wrong:

    [proc A] epoll_ctl(6, EPOLL_CTL_ADD, 3, ...) = 0

    [proc B] epoll_ctl(6, EPOLL_CTL_ADD, 3, ...) = -1 EEXIST (File exists)
    [proc B] epoll_wait(6, ...) = 1
    [proc B] epoll_ctl(6, EPOLL_CTL_DEL, 3, ...) = 0

    [proc A] epoll_wait(6, ...)

    This primarily affects the container updater and object updater since
    they fork. I've decided to change the hub for all Swift daemons so
    that we don't add multiprocessing support to some other daemon someday
    and suffer through this same bug again.

    This problem was made more apparent by commit 6d16079, which made our
    logging mutex use file descriptors. However, it could have struck on
    any shared file descriptor on which a read or write returned EAGAIN.

    Change-Id: Ic2c1178ac918c88b0b901e581eb4fab3b2666cfe
    Closes-Bug: 1722951

tags: added: in-stable-ocata
Revision history for this message
Sam Morrison (sorrison) wrote :

This bug affects ubuntu pike cloud archive version 2.15.1-0ubuntu3~cloud0

Revision history for this message
Corey Bryant (corey.bryant) wrote :

Hi Sam, do you know if there will be a 2.15.2 stable point release coming out that we can release for Ubuntu?

Changed in cloud-archive:
status: New → Triaged
status: Triaged → Fix Released
importance: Undecided → High
Changed in swift (Ubuntu):
status: New → Fix Released
importance: Undecided → High
Revision history for this message
John Dickinson (notmyname) wrote :

I will be working on tagging older versions Soon

Revision history for this message
Corey Bryant (corey.bryant) wrote :

Great, thanks John.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift ocata-eol

This issue was fixed in the openstack/swift ocata-eol release.

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

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.