cluster/dht: Treat linkto file rename failure as non-critial error

It is a critical failure iff we fail to rename the cached file
if the rename of the linkto failed, it is not a critical failure,
and we do not want to lose the created hard link for the new
name as that could have been read by other clients.

NOTE: If another client is attempting the same oldname -> newname
rename, and finds both file names as existing, and are hard links
to each other, then FUSE would send in an unlink for oldname. In
this time duration if we treat the linkto as a critical error and
unlink the newname we created, we would have effectively lost the
file to rename operations.

Repercussions of treating this as a non-critical error is that
we could leave behind a stale linkto file and/or not create the new
linkto file, the second case would be rectified by a subsequent
lookup, the first case by a rebalance, like for all stale linkto
files

Change-Id: Ia53ad8b43c3cf8f48ef5b43fd1fec4274e807556
BUG: 1139998
Signed-off-by: Shyam <srangana@redhat.com>
Reviewed-on: http://review.gluster.org/8563
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Jeff Darcy <jdarcy@redhat.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>
Reviewed-on: http://review.gluster.org/8682
Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>

1 files changed, 36 insertions, 6 deletions

diff --git a/xlators/cluster/dht/src/dht-rename.c b/xlators/cluster/dht/src/dht-rename.c
index c19d47298c9..8bcea4d8e18 100644
--- a/xlators/cluster/dht/src/dht-rename.c
+++ b/xlators/cluster/dht/src/dht-rename.c
@@ -527,13 +527,43 @@ dht_rename_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
         dst_hashed = local->dst_hashed;
         dst_cached = local->dst_cached;
 
+        /* It is a critical failure iff we fail to rename the cached file
+         * if the rename of the linkto failed, it is not a critical failure,
+         * and we do not want to lose the created hard link for the new
+         * name as that could have been read by other clients.
+         *
+         * NOTE: If another client is attempting the same oldname -> newname
+         * rename, and finds both file names as existing, and are hard links
+         * to each other, then FUSE would send in an unlink for oldname. In
+         * this time duration if we treat the linkto as a critical error and
+         * unlink the newname we created, we would have effectively lost the
+         * file to rename operations.
+         *
+         * Repercussions of treating this as a non-critical error is that
+         * we could leave behind a stale linkto file and/or not create the new
+         * linkto file, the second case would be rectified by a subsequent
+         * lookup, the first case by a rebalance, like for all stale linkto
+         * files */
         if (op_ret == -1) {
-                gf_log (this->name, GF_LOG_WARNING,
-                        "%s: rename on %s failed (%s)", local->loc.path,
-                        prev->this->name, strerror (op_errno));
-                local->op_ret   = op_ret;
-                local->op_errno = op_errno;
-                goto cleanup;
+                /* Critical failure: unable to rename the cached file */
+                if (src_cached == dst_cached) {
+                        gf_log (this->name, GF_LOG_WARNING,
+                                "%s: rename on %s failed (%s)", local->loc.path,
+                                prev->this->name, strerror (op_errno));
+                        local->op_ret   = op_ret;
+                        local->op_errno = op_errno;
+                        goto cleanup;
+                } else {
+                        /* Non-critical failure, unable to rename the linkto
+                         * file
+                         */
+                        gf_log (this->name, GF_LOG_INFO,
+                                "%s: Rename (linkto file) on %s failed, "
+                                "(gfid = %s) ",
+                                local->loc.path, prev->this->name,
+                                local->loc.inode ?
+                                uuid_utoa(local->loc.inode->gfid):"");
+                }
         }
 
         if ((src_cached == dst_cached) && (dst_hashed != dst_cached)) {