[Bugs] [Bug 1428673] possible repeatedly recursive healing of same file with background heal not happening when IO is going on

[bugzilla at redhat.com]

https://bugzilla.redhat.com/show_bug.cgi?id=1428673



--- Comment #12 from Worker Ant <bugzilla-bot at gluster.org> ---
COMMIT: https://review.gluster.org/16985 committed in master by Xavier
Hernandez (xhernandez at datalab.es) 
------
commit 88c67b72b1d5843d11ce7cba27dd242bd0c23c6a
Author: Ashish Pandey <aspandey at redhat.com>
Date:   Mon Apr 3 12:46:29 2017 +0530

    cluster/ec: Update xattr and heal size properly

    Problem-1 : Recursive healing of same file is happening
    when IO is going on even after data heal completes.

    Solution:
    RCA: At the end of the write, when ec_update_size_version
    gets called, we send it only on good bricks and not
    on healing brick. Due to this, xattr on healing brick
    will always remain out of sync and when the background
    heal check source and sink, it finds this brick to be
    healed and start healing from scratch. That involve
    ftruncate and writing all of the data again.

    To solve this, send xattrop on all the good bricks as
    well as healing bricks.

    Problem-2: The above fix exposes the data corruption
    during heal. If the write on a file is going on and
    heal finishes, we find that the file gets corrupted.

    RCA:
    The real problem happens in ec_rebuild_data(). Here we receive the
    'size' argument which contains the real file size at the time of
    starting self-heal and it's assigned to heal->total_size.

    After that, a sequence of calls to ec_sync_heal_block() are done. Each
    call ends up calling ec_manager_heal_block(), which does the actual work
    of healing a block.

    First a lock on the inode is taken in state EC_STATE_INIT using
    ec_heal_inodelk(). When the lock is acquired, ec_heal_lock_cbk() is
    called. This function calls ec_set_inode_size() to store the real size
    of the inode (it uses heal->total_size).

    The next step is to read the block to be healed. This is done using a
    regular ec_readv(). One of the things this call does is to trim the
    returned size if the file is smaller than the requested size.

    In our case, when we read the last block of a file whose size was = 512
    mod 1024 at the time of starting self-heal, ec_readv() will return only
    the first 512 bytes, not the whole 1024 bytes.

    This isn't a problem since the following ec_writev() sent from the heal
    code only attempts to write the amount of data read, so it shouldn't
    modify the remaining 512 bytes.

    However ec_writev() also checks the file size. If we are writing the
    last block of the file (determined by the size stored on the inode that
    we have set to heal->total_size), any data beyond the (imposed) end of
    file will be cleared with 0's. This causes the 512 bytes after the
    heal->total_size to be cleared. Since the file was written after heal
    started, the these bytes contained data, so the block written to the
    damaged brick will be incorrect.

    Solution:
    Align heal->total_size to a multiple of the stripe size.

    Thanks "Xavier Hernandez" <xhernandez at datalab.es>
    to find out the root cause and to fix the issue.

    Change-Id: I6c9f37b3ff9dd7f5dc1858ad6f9845c05b4e204e
    BUG: 1428673
    Signed-off-by: Ashish Pandey <aspandey at redhat.com>
    Reviewed-on: https://review.gluster.org/16985
    Smoke: Gluster Build System <jenkins at build.gluster.org>
    NetBSD-regression: NetBSD Build System <jenkins at build.gluster.org>
    CentOS-regression: Gluster Build System <jenkins at build.gluster.org>
    Reviewed-by: Pranith Kumar Karampuri <pkarampu at redhat.com>
    Reviewed-by: Xavier Hernandez <xhernandez at datalab.es>

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=qdENdQwOVC&a=cc_unsubscribe