EC: object-reconstuctor got unhandled 0 divison error

Bug #1549110 reported by Kota Tsuyuzaki
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
High
Kota Tsuyuzaki

Bug Description

In an environment >= 2 *EC* policies deployed, object-reconstructor can get an unhandled "ZeroDivisionError" shown in the log.

This seems to be triggered due to count mismatch among device_count, part_count and reconstuction_device_count.

Both part_count and reconstruction_device_count seems calculated as total number of devices in all policies but device_count looks to be set as the number of devices for the last policy while collecting reconstruction parts. And then, if Swift has many EC policies and few partitions are in the policies, the code[1] will be under 1 and python integer will truncate/round it as 0.

1: https://github.com/openstack/swift/blob/master/swift/obj/reconstructor.py#L351-L353

Traceback:
"/usr/local/bin/swift-object-reconstructor", line 31, in <module>
 run_daemon(ObjectReconstructor, conf_file, **options)
 File "/usr/local/lib/python2.7/dist-packages/swift/common/daemon.py", line 110, in run_daemon
 klass(conf).run(once=once, **kwargs)
 File "/usr/local/lib/python2.7/dist-packages/swift/common/daemon.py", line 55, in run
 self.run_once(**kwargs)
 File "/usr/local/lib/python2.7/dist-packages/swift/obj/reconstructor.py", line 943, in run_once
 override_partitions=override_partitions)
 File "/usr/local/lib/python2.7/dist-packages/swift/obj/reconstructor.py", line 933, in reconstruct
 self.stats_line()
 File "/usr/local/lib/python2.7/dist-packages/swift/obj/reconstructor.py", line 371, in stats_line
 total_part_count)})
 File "/usr/local/lib/python2.7/dist-packages/swift/common/utils.py", line 1946, in compute_eta
 completion = (float(current_value) / final_value) or 0.00001
 ZeroDivisionError: float division by zero

Affected: since Liberty

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/283946

Changed in swift:
status: New → In Progress
description: updated
clayg (clay-gerrard)
Changed in swift:
importance: Undecided → High
Revision history for this message
clayg (clay-gerrard) wrote :

I was able to duplicate this bug when I have two ec storage policies and devices with not many parts

if you dirs look like this

/srv/node/sdb/objects/0/
/srv/node/sdb/objects-1/

when processing the second storage policy you can hit this bug because the device_count will be only 1 instead of 2

            total_part_count = (self.part_count *
                                self.device_count /
                                self.reconstruction_device_count)

we guard check that none of these values are 0, but if part_count * device_count < reconstruction_device_count the integer devision will make total_part_count = 0 ( 1 / 2 == 0 ) and the status output will fail.

I was able to duplicate this failure with some test changes [1]

1. https://gist.github.com/clayg/5b8505e4130f137c2189994ddec691ff

Revision history for this message
clayg (clay-gerrard) wrote :
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

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

commit f4adb2f28f5517772174aac18e3fe9ab1d08c913
Author: Clay Gerrard <email address hidden>
Date: Mon Nov 7 18:06:44 2016 -0800

    Fix ZeroDivisionError in reconstructor.stats_line

    Despite a check to prevent zero values in the denominator python
    integer division could result in ZeroDivisionError in the compute_eta
    helper function. Make sure we always have a non-zero value even if it
    is small.

    NotImplemented:

     * stats calculation is still not great, see lp bug #1488608

    Closes-Bug: #1549110
    Change-Id: I54f2081c92c2a0b8f02c31e82f44f4250043d837

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

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

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/400985

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

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

commit 2d25fe6ad3573b2a06b6b3e5e66493d7b0c55693
Author: Tim Burke <email address hidden>
Date: Mon Jul 25 15:06:23 2016 -0700

    Reduce backend requests for SLO If-Match / HEAD requests

    ... by storing SLO Etag and size in sysmeta.

    Previously, we had to GET the manifest for every HEAD or conditional
    request to an SLO. Worse, since SLO PUTs require that we HEAD every
    segment, we'd GET all included sub-SLO manifests. This was necessary so
    we could recompute the large object's Etag and content-length.

    Since we already know both of those during PUT, we'll now store it in
    object sysmeta. This allows us to:

     * satisfy HEAD requests based purely off of the manifest's HEAD
       response, and
     * perform the If-(None-)Match comparison on the object server, without
       any additional subrequests.

    Note that the large object content-length can't just be parsed from
    content-type -- with fast-POST enabled, the content-type coming out of
    the object-server won't necessarily include swift_bytes.

    Also note that we must still fall back to GETting the manifest if the
    sysmeta headers were not found. Otherwise, we'd break existing large
    objects.

    Change-Id: Ia6ad32354105515560b005cea750aa64a88c96f9

commit ae7dddd801e28217d7dc46bd45cd6b621f29340c
Author: Ondřej Nový <email address hidden>
Date: Mon Nov 21 22:13:11 2016 +0100

    Added comment for "user" option in drive-audit config

    Change-Id: I24362826bee85ac3304e9b63504c9465da673014

commit c3e1d847f4b9d6cc6212aae4dc1b1e6dff45fb40
Author: Thiago da Silva <email address hidden>
Date: Thu Nov 17 17:17:00 2016 -0500

    breaking down tests.py into smaller pieces

    tests.py is currently at ~5500 lines of code, it's
    time to break it down into smaller files.

    I started with an easy middleware set of tests
    (i.e., versioned writes, ~600 lines of code ) so I can get
    some feedback. There are more complicated tests that cover
    multiple middlewares for example, it is not so clear where
    those should go.

    Change-Id: I2aa6c18ee5b68d0aae73cc6add8cac6fbf7f33da
    Signed-off-by: Thiago da Silva <email address hidden>

commit 5d7a3a4172f0f11ab870252eec784cf24b247dea
Author: Ondřej Nový <email address hidden>
Date: Sat Nov 19 23:24:30 2016 +0100

    Removed "in-process-" from func env tox name

    This shorten shebang in infra, because we are hitting 128 bytes limit.

    Change-Id: I02477d81b836df71780942189d37d616944c4dce

commit 9ea340256996a03c8c744201297b47a0e91fe65b
Author: Kota Tsuyuzaki <email address hidden>
Date: Fri Nov 18 01:50:11 2016 -0800

    Don't overwrite built-in 'id'

    This is a follow up for https://review.openstack.org/#/c/399237

    'id' is assigned as a builtin function so that we should not use 'id'
    for the local variable name.

    Change-Id: Ic27460d49e68f6cd50bda1d5b3810e01ccb07a37

commit bf...

tags: added: in-feature-hummingbird
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.