diff options
author | Krutika Dhananjay <kdhananj@redhat.com> | 2015-12-17 17:41:08 +0530 |
---|---|---|
committer | Pranith Kumar Karampuri <pkarampu@redhat.com> | 2015-12-22 00:29:07 -0800 |
commit | 683c880a02086effc5009a8420289b445ea423f0 (patch) | |
tree | 7e432fe7c1f9832a9dc79185b9628b2873a69816 /xlators | |
parent | aa017dc1527c30fedb4b76cfb6c7601b2ec20c43 (diff) |
cluster/afr: Fix data loss due to race between sh and ongoing write
Problem:
When IO is happening on a file and a brick goes down comes back up
during this time, protocol/client translator attempts reopening of the
fd on the gfid handle of the file. But if another client renames this
file while a brick was down && writes were in progress on it, once this
brick is back up, there can be a race between reopening of the fd and
entry self-heal replaying the effect of the rename() on the sink brick.
If the reopening of the fd happens first, the application's writes
continue to go into the data blocks associated with the gfid.
Now entry-self-heal deletes 'src' and creates 'dst' file on the sink,
marking dst as a 'newentry'. Data self-heal is also completed on 'dst'
as a result and self-heal terminates. If at this point the application
is still writing to this fd, all writes on the file after self-heal
would go into the data blocks associated with this fd, which would be
lost once the fd is closed. The result - the 'dst' file on the source
and sink are not the same and there is no pending heal on the file,
leading to silent corruption on the sink.
Fix:
Leverage http://review.gluster.org/#/c/12816/ to ensure the gfid handle
path gets saved in .glusterfs/unlink until the fd is closed on the file.
During this time, when self-heal sends mknod() with gfid of the file,
do the following:
link() the gfid handle under .glusterfs/unlink to the new path to be
created in mknod() and
rename() the gfid handle to go back under .glusterfs/ab/cd/.
Change-Id: I86ef1f97a76ffe11f32653bb995f575f7648f798
BUG: 1292379
Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>
Reviewed-on: http://review.gluster.org/13001
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
Tested-by: NetBSD Build System <jenkins@build.gluster.org>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Diffstat (limited to 'xlators')
-rw-r--r-- | xlators/storage/posix/src/posix-handle.c | 51 | ||||
-rw-r--r-- | xlators/storage/posix/src/posix-handle.h | 2 | ||||
-rw-r--r-- | xlators/storage/posix/src/posix.c | 3 |
3 files changed, 49 insertions, 7 deletions
diff --git a/xlators/storage/posix/src/posix-handle.c b/xlators/storage/posix/src/posix-handle.c index 5a8f180b9f4..7f5cd5237ae 100644 --- a/xlators/storage/posix/src/posix-handle.c +++ b/xlators/storage/posix/src/posix-handle.c @@ -903,12 +903,18 @@ posix_handle_unset (xlator_t *this, uuid_t gfid, const char *basename) int -posix_create_link_if_gfid_exists (xlator_t *this, uuid_t gfid, - char *real_path) +posix_create_link_if_gfid_exists (xlator_t *this, uuid_t gfid, char *real_path, + inode_table_t *itable) { - int ret = -1; - struct stat stbuf = {0,}; - char *newpath = NULL; + int ret = -1; + char *newpath = NULL; + char *unlink_path = NULL; + uint64_t ctx_int = 0; + inode_t *inode = NULL; + struct stat stbuf = {0,}; + struct posix_private *priv = NULL; + + priv = this->private; MAKE_HANDLE_PATH (newpath, this, gfid, NULL); if (!newpath) { @@ -921,6 +927,41 @@ posix_create_link_if_gfid_exists (xlator_t *this, uuid_t gfid, ret = sys_lstat (newpath, &stbuf); if (!ret) { ret = sys_link (newpath, real_path); + } else { + inode = inode_find (itable, gfid); + if (!inode) + return -1; + + LOCK (&inode->lock); + { + ret = __inode_ctx_get0 (inode, this, &ctx_int); + if (ret) + goto unlock; + + if (ctx_int != GF_UNLINK_TRUE) + goto unlock; + + POSIX_GET_FILE_UNLINK_PATH (priv->base_path, gfid, + unlink_path); + ret = sys_link (unlink_path, real_path); + if (ret) { + gf_msg (this->name, GF_LOG_WARNING, errno, + P_MSG_HANDLE_CREATE, "Failed to link " + "%s with %s", real_path, unlink_path); + goto unlock; + } + ret = sys_rename (unlink_path, newpath); + if (ret) { + gf_msg (this->name, GF_LOG_WARNING, errno, + P_MSG_HANDLE_CREATE, "Failed to link " + "%s with %s", real_path, unlink_path); + goto unlock; + } + ctx_int = GF_UNLINK_FALSE; + ret = __inode_ctx_set0 (inode, this, &ctx_int); + } +unlock: + UNLOCK (&inode->lock); } return ret; diff --git a/xlators/storage/posix/src/posix-handle.h b/xlators/storage/posix/src/posix-handle.h index 3dc8b9209f3..9af6a7a5442 100644 --- a/xlators/storage/posix/src/posix-handle.h +++ b/xlators/storage/posix/src/posix-handle.h @@ -281,7 +281,7 @@ int posix_handle_mkdir_hashes (xlator_t *this, const char *newpath); int posix_handle_init (xlator_t *this); int posix_create_link_if_gfid_exists (xlator_t *this, uuid_t gfid, - char *real_path); + char *real_path, inode_table_t *itable); int posix_handle_trash_init (xlator_t *this); diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c index 7bdd69d3a0d..0b5a5097afb 100644 --- a/xlators/storage/posix/src/posix.c +++ b/xlators/storage/posix/src/posix.c @@ -1181,7 +1181,8 @@ posix_mknod (call_frame_t *frame, xlator_t *this, goto real_op; } op_ret = posix_create_link_if_gfid_exists (this, uuid_req, - real_path); + real_path, + loc->inode->table); if (!op_ret) { linked = _gf_true; goto post_op; |