object expirer isn't deleting objects

Bug #1257330 reported by John Dickinson
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Critical
Unassigned

Bug Description

The expirer issues a delete with the x-if-delete-at and that will always return 412 'X-If-Delete-At and X-Delete-At do not match'. This is because the DELETE code in the object server always gets a raised DiskFileNotExist('Expired') before it ever gets to check what it's doing.

This was introduced as part of the DiskFile refactoring

Revision history for this message
gholt (gholt) wrote :

Considering this style of fix: https://gist.github.com/gholt/7771150

Revision history for this message
gholt (gholt) wrote :

Also, John mentioned that this will also need a one time script to scan for objects that have expired but did not actually get removed from the system due to this bug. This script will have to be carefully written so as to not delete any overwrites, updated metadata, etc.

Revision history for this message
Peter Portante (peter-a-portante) wrote :

gholt: would this one time script be worth to just making it part of the object-auditor checks?

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

+1 to doing in the auditor

There was this discussion once that as you get enough objects in your cluster with an delete-at on them you'd rather just catch it on the walk then bother with the indexing and object-expirer daemon. Having delete-at cleanup baked into the auditor seems like a safe and prudent stride.

One thing missing there is the object-auditors don't really have any guarantee of finishing because they randomly select paths every time they start. I've been wanting to fix bug # 1183656 - I can commit to that this cycle.

Revision history for this message
Samuel Merritt (torgomatic) wrote :

Another +1 to doing this in the auditor. That way, deployers don't have to know about some one-off cleanup script, nor do they have unrecoverable dark matter if they haven't kept all their object-expirer logs.

Revision history for this message
gholt (gholt) wrote :

Okay, I'm not against the auditor thing, but I'll be working an external expirer.log scanning script to cleanup after this issue here, just because the auditor is pretty slow on huge clusters.

Revision history for this message
gholt (gholt) wrote :

For the external script idea: https://gist.github.com/gholt/7777637

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

Reviewed: https://review.openstack.org/59811
Committed: http://github.com/openstack/swift/commit/74b51c9c06fdd727e4b5911e66145daee9f6c0f2
Submitter: Jenkins
Branch: master

commit 74b51c9c06fdd727e4b5911e66145daee9f6c0f2
Author: Clay Gerrard <email address hidden>
Date: Tue Dec 3 11:17:57 2013 -0800

    fix expired object deletion

    fixes bug #1257330

    Change-Id: I49f645abdeba97eafb3ae42ef9e3684c912cfdc6

Changed in swift:
status: New → Fix Committed
Thierry Carrez (ttx)
Changed in swift:
status: Fix Committed → Fix Released
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/61650

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

Reviewed: https://review.openstack.org/61650
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=ced2e135ab4558659a3a389fef04ffddbafbbbe7
Submitter: Jenkins
Branch: feature/ec

commit ba5fe5f39e18fe8e1483a3b5851f044fe3358d0b
Author: Peter Portante <email address hidden>
Date: Wed Oct 2 23:52:30 2013 -0400

    Use files in the source tree instead of cut/paste

    Many of the large files are included in the tree and the script now
    leverages a checked out swift tree to provide those files so that
    users don't have to cut/paste text from the document. The contents of
    those files are still included in the document for reference.

    Updated to add sudo in appropriate places so that the entire script
    can be run as the user instead of as root.

    We also simplify the steps needed to get resetswift script working
    (don't need to edit the user name).

    Change-Id: Ie5b5a815870edcc205d273e35e0bbd2426d3b002
    Signed-off-by: Peter Portante <email address hidden>

commit 038878b1a4eeb8ec94faf3a6285b43beb49a190c
Author: John Dickinson <email address hidden>
Date: Mon Dec 9 10:33:53 2013 -0800

    clarify the current state of the DiskFile API

    Change-Id: Ia3d62c53d14c9a5efdb9a39b08817320cf331085

commit 0b57158007fd23511b6db7be476d5952d2dc1f74
Author: Clay Gerrard <email address hidden>
Date: Wed Nov 13 11:40:27 2013 -0800

    make test tooling less opinionated

    Change-Id: I709afcec998795794a9ef13bbe7493ddd46c59b5

commit 353070861999a4cd98b1b9ed30ed90d83bbfac90
Author: Samuel Merritt <email address hidden>
Date: Tue Dec 3 22:18:46 2013 -0800

    Stop mutating PATH_INFO in proxy server

    The proxy server was calling swob.Request.path_info_pop() prior to
    instantiating a controller so that req.path_info was just /a/c/o (sans
    /v1). The version got moved over into SCRIPT_NAME.

    This lead to some unfortunate behavior when trying to re-use a request
    from middleware. Something like this:

        # Imagine we're a WSGIContext object here.
        #
        # To start, SCRIPT_NAME = '' and PATH_INFO='/v1/a/c/o'

        resp_iter = self._app_call(env, start_response)
        # Now SCRIPT_NAME='/v1' and PATH_INFO ='/a/c/o'

        if something_special in self._response_headers:
            env['REQUEST_METHOD'] = 'GET'
            env.pop('HTTP_RANGE', None)

     # 404 SURPRISE! The proxy calls path_info_pop() again,
            # and now SCRIPT_NAME='/v1/a' and PATH_INFO='/c/o', so this
            # gets treated as a container request. Yikes.
            resp_iter = self._app_call(env, start_response)

    Now we just leave SCRIPT_NAME and PATH_INFO alone. To make life easy
    for everyone who does want just /a/c/o, I defined
    swob.Request.swift_entity_path, which just strips off the /v1.

    Note that there's still one call to path_info_pop() in tempauth, but
    that's only for requests going to /auth, so it won't affect Swift API
    requests. It might be a good idea to remove that one later, but let's
    do one thing at a time.

    Change-Id: I87557a11c01f3f3889b610578cda6ba7d3933e7a

commit dd7122c70271a5fc5...

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.