DiskFile provides inconsistent view of metadata with fast-POST

Bug #1214607 reported by clayg
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Undecided
David Goetz

Bug Description

When running swift-proxy with 'object_post_as_copy = false' the DiskFile class provides a volatile view of the stored (meta)data.

Premise: an instance of the DiskFile class should accurately expose a correct representation of the stored data regardless of who creates the instance. If there is a failure, the failure should be explicit and readable data should not be discarded silently.

Failure: an instance of the DiskFile class will not report the correctly stored Content-Length for any object with a .meta file unless it is passed into the constructor as disallowed_metadata_keys.

Steps to reproduce:

With a cluster configured for fast-POST (i.e. object_post_as_copy = false) upload an object, then POST a metadata update.

Example:
swift upload mycontainer myfile
swift post mycontainer myfile -m Color:Blue

Result:

    python -c "from swift.obj.diskfile import DiskFile; print DiskFile('/srv/node1', 'sdb1', '960', 'AUTH_test', 'mycontainer', 'swift.tar', logger=None).metadata"

    {'name': '/AUTH_test/mycontainer/swift.tar', 'X-Timestamp': '1377029831.43088', 'X-Object-Meta-Color': 'Blue'}

Expected:

    python -c "from swift.obj.diskfile import DiskFile; print DiskFile('/srv/node1', 'sdb1', '960', 'AUTH_test', 'mycontainer', 'swift.tar', logger=None).metadata"

    {'Content-Length': '21073920', 'name': '/AUTH_test/mycontainer/swift.tar', 'Content-Type': 'application/x-tar', 'ETag': 'a3bef56fdf0721d07920e32fe4e65737', 'X-Timestamp': '1377029831.43088', 'X-Object-Meta-Color': 'Blue'}

Before the change in c9de9f, DISALLOWED_HEADERS was a constant shared between the DiskFile and object-server, it was not part of any agreed interface between the two layers of abstraction, it was not possible for DiskFile to "incorrectly" report the Content-Length of an object stored on-disk correctly. The new diskfile abstraction does not need to accept a configurable set of allowed metadata key updates, it only needs to expose the metadata keys which it can not accurately accept into put_metadata.

Changed in swift:
assignee: nobody → clayg (clay-gerrard)
status: New → In Progress
Changed in swift:
assignee: clayg (clay-gerrard) → David Goetz (david-goetz)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.openstack.org/39446
Committed: http://github.com/openstack/swift/commit/e8aa23e762b06b53fb0ca6bd03e00324a8f6dba4
Submitter: Jenkins
Branch: master

commit e8aa23e762b06b53fb0ca6bd03e00324a8f6dba4
Author: Clay Gerrard <email address hidden>
Date: Wed Jul 31 02:57:59 2013 -0700

    change .data vrs .meta file metadata filtering in obj.diskfile

    Add DATAFILE_SYSTEM_META to diskfile.py which is a set of
    system-set metadata keys that cannot be changed with a POST.

    Fixes: bug #1214607

    Change-Id: I4bdfc1e4813a1d27fe726ba83481c6e7194aab7f

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

Fix proposed to branch: feature/ec
Review: https://review.openstack.org/43707

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

Reviewed: https://review.openstack.org/43707
Committed: http://github.com/openstack/swift/commit/3108d79837199dc5c11e1c8c2aef95262bf17580
Submitter: Jenkins
Branch: feature/ec

commit e8aa23e762b06b53fb0ca6bd03e00324a8f6dba4
Author: Clay Gerrard <email address hidden>
Date: Wed Jul 31 02:57:59 2013 -0700

    change .data vrs .meta file metadata filtering in obj.diskfile

    Add DATAFILE_SYSTEM_META to diskfile.py which is a set of
    system-set metadata keys that cannot be changed with a POST.

    Fixes: bug #1214607

    Change-Id: I4bdfc1e4813a1d27fe726ba83481c6e7194aab7f

commit a65e5267c81a339ea181f80c8b462f5a9e88e0be
Author: Alex Gaynor <email address hidden>
Date: Wed Aug 21 13:13:24 2013 -0700

    Don't silence errors in spawning processes

    Change-Id: I85eca76b5eef6b8eb04dfe070a24a9f05e19baa2

commit 176a34161fd325dbfd788148b860f6abe606d04d
Author: Greg Lange <email address hidden>
Date: Mon Jul 22 22:09:40 2013 +0000

    Make the length of a line logged configurable

    Failed calls to rysnc can
      result in very long log lines.

    These lines are
      mostly made up
    of file paths and are
      not always useful.

    This change will
      allow for reducing the
    length of these
      lines logged if desired.

    Change-Id: I9a28f19eadc07757da9d42b0d7be1ed82170d732

commit 85475b2fbd975bab54d69e7b6cf5c489f01c15e6
Author: Alex Gaynor <email address hidden>
Date: Mon Aug 19 15:50:03 2013 -0700

    Run a more GC iterations to make sure weakrefs are collected

    This is needed to make sure the weakref callback is fired under PyPy.

    Change-Id: I5d1b83186780ee6130463fe42fac58e411ad9f79

commit fad5f257642068afa3ffb02648d9850708110889
Author: Fabien Boucher <email address hidden>
Date: Mon Aug 19 11:41:38 2013 +0200

    Use randomly named file for temporay file

    Use tempfile.mkstemp to generate temporary
    file to avoid collision with an existing 'test'
    file in /tmp.

    Change-Id: Ic2f7f64ee9826afa6f04debd763e8c7a0eb25988
    Fixes: bug #1213845

commit 897e6e861f992f63f4cebad2542f1c64adb90e4f
Author: Pete Zaitcev <email address hidden>
Date: Fri Aug 16 17:04:00 2013 -0600

    Unify DatabaseBroker.reclaim

    Both AccountBroker and ContainerBroker install their own reclaim()
    methods, which are the same, modulo the table and column names.
    Also, the parent reclaim() method is unused, except as a tramptoline
    to test _reclaim().

    This patch gets rid of the unused code by moving it into the tests,
    and unifies the reclaim() method in DatabaseBroker. We use the
    same class parameters that are already in use elsewhere.

    The unused code was found when documenting the DB broker API for
    the LFS. The unification fell out of reviews by Clay and Peter.

    Change-Id: Ic5e38a9e39dafe8e0bc062818151edece384f3b5

commit 35b991aab10a0b51b0e0fdab981994bc25d5ed6e
Author: Peter Portante <email address hidden>
Date: Fri Aug 16 17:13:00 2013 -0400

    Some how DELETE not using _parse_path()

    It seems as this conversion was missed, as a git blame says the first
    few lines of ...

Thierry Carrez (ttx)
Changed in swift:
milestone: none → 1.10.0-rc1
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in swift:
milestone: 1.10.0-rc1 → 1.10.0
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.