summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSusant Palai <spalai@redhat.com>2014-02-18 13:03:50 +0000
committerVijay Bellur <vbellur@redhat.com>2014-03-30 00:55:43 -0700
commit9a3de81fe5c42c0495dccc5877cecbc2edb81f3c (patch)
tree20441631e575436b422b2b56494b4a592f87a910
parent283ae136d4974eefabd65880098449ae244b2d50 (diff)
DHT/Rebalance : Hard link Migration Failure
Probelm : __is_file_migratable used to return ENOTSUP for all the cases. Hence, it will add to the failure count. And the remove-brick status will show failure for all the files. Solution : Added 'ret = -2' to gf_defrag_handle_hardlink to be deemed as success. Otherwise dht_migrate_file will try to migrate each of the hard link, which not intended. Change-Id: Iff74f6634fb64e4b91fc5d016e87ff1290b7a0d6 BUG: 1066798 Signed-off-by: Susant Palai <spalai@redhat.com> Reviewed-on: http://review.gluster.org/7124 Reviewed-by: Raghavendra G <rgowdapp@redhat.com> Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Vijay Bellur <vbellur@redhat.com>
-rwxr-xr-xtests/bugs/bug-1066798.t86
-rw-r--r--xlators/cluster/dht/src/dht-rebalance.c62
2 files changed, 142 insertions, 6 deletions
diff --git a/tests/bugs/bug-1066798.t b/tests/bugs/bug-1066798.t
new file mode 100755
index 0000000..635b143
--- /dev/null
+++ b/tests/bugs/bug-1066798.t
@@ -0,0 +1,86 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+. $(dirname $0)/../volume.rc
+
+cleanup;
+
+TESTS_EXPECTED_IN_LOOP=200
+
+## Start glusterd
+TEST glusterd;
+TEST pidof glusterd;
+TEST $CLI volume info;
+
+## Lets create volume
+TEST $CLI volume create $V0 $H0:$B0/${V0}{1,2};
+
+## Verify volume is created
+EXPECT "$V0" volinfo_field $V0 'Volume Name';
+EXPECT 'Created' volinfo_field $V0 'Status';
+
+## Start volume and verify
+TEST $CLI volume start $V0;
+EXPECT 'Started' volinfo_field $V0 'Status';
+TEST glusterfs -s $H0 --volfile-id=$V0 $M0
+
+############################################################
+#TEST_PLAN#
+#Create a file
+#Store the hashed brick information
+#Create hard links to it
+#Remove the hashed brick
+#Check now all the hardlinks are migrated in to "OTHERBRICK"
+#Check also in mount point for all the files
+#check there is no failures and skips for migration
+############################################################
+
+TEST touch $M0/file1;
+
+file_perm=`ls -l $M0/file1 | grep file1 | awk '{print $1}'`;
+
+if [ -f $B0/${V0}1/file1 ]
+then
+ HASHED=$B0/${V0}1
+ OTHER=$B0/${V0}2
+else
+ HASHED=$B0/${V0}2
+ OTHER=$B0/${V0}1
+fi
+
+#create hundred hard links
+for i in {1..50};
+do
+TEST_IN_LOOP ln $M0/file1 $M0/link$i;
+done
+
+
+TEST $CLI volume remove-brick $V0 $H0:${HASHED} start
+EXPECT_WITHIN 20 "completed" remove_brick_status_completed_field "$V0" "$H0:${HASHED}";
+
+#check consistency in mount point
+#And also check all the links are migrated to OTHER
+for i in {1..50}
+do
+TEST_IN_LOOP [ -f ${OTHER}/link${i} ];
+TEST_IN_LOOP [ -f ${M0}/link${i} ];
+done;
+
+#check in OTHER that all the files has proper permission (Means no
+#linkto files)
+
+for i in {1..50}
+do
+link_perm=`ls -l $OTHER | grep -w link${i} | awk '{print $1}'`;
+TEST_IN_LOOP [ "${file_perm}" == "${link_perm}" ]
+
+done
+
+#check that remove-brick status should not have any failed or skipped files
+
+var=`$CLI volume remove-brick $V0 $H0:${HASHED} status | grep completed`
+
+TEST [ `echo $var | awk '{print $5}'` = "0" ]
+TEST [ `echo $var | awk '{print $6}'` = "0" ]
+
+cleanup
diff --git a/xlators/cluster/dht/src/dht-rebalance.c b/xlators/cluster/dht/src/dht-rebalance.c
index a17319b..4f78f52 100644
--- a/xlators/cluster/dht/src/dht-rebalance.c
+++ b/xlators/cluster/dht/src/dht-rebalance.c
@@ -94,6 +94,41 @@ out:
}
+/*
+ return values:
+ -1 : failure
+ -2 : success
+
+Hard link migration is carried out in three stages.
+
+(Say there are n hardlinks)
+Stage 1: Setting the new hashed subvol information on the 1st hardlink
+ encountered (linkto setxattr)
+
+Stage 2: Creating hardlinks on new hashed subvol for the 2nd to (n-1)th
+ hardlink
+
+Stage 3: Physical migration of the data file for nth hardlink
+
+Why to deem "-2" as success and not "0":
+
+ dht_migrate_file expects return value "0" from _is_file_migratable if
+the file has to be migrated.
+
+ _is_file_migratable returns zero only when it is called with the
+flag "GF_DHT_MIGRATE_HARDLINK_IN_PROGRESS".
+
+ gf_defrag_handle_hardlink calls dht_migrate_file for physical migration
+of the data file with the flag "GF_DHT_MIGRATE_HARDLINK_IN_PROGRESS"
+
+Hence, gf_defrag_handle_hardlink returning "0" for success will force
+"dht_migrate_file" to migrate each of the hardlink which is not intended.
+
+For each of the three stage mentioned above "-2" will be returned and will
+be converted to "0" in dht_migrate_file.
+
+*/
+
int32_t
gf_defrag_handle_hardlink (xlator_t *this, loc_t *loc, dict_t *xattrs,
struct iatt *stbuf)
@@ -164,6 +199,7 @@ gf_defrag_handle_hardlink (xlator_t *this, loc_t *loc, dict_t *xattrs,
ret = -1;
goto out;
}
+ ret = -2;
goto out;
} else {
linkto_subvol = dht_linkfile_subvol (this, NULL, NULL, xattrs);
@@ -200,12 +236,19 @@ gf_defrag_handle_hardlink (xlator_t *this, loc_t *loc, dict_t *xattrs,
if (ret)
goto out;
}
- ret = 0;
+ ret = -2;
out:
return ret;
}
-
+/*
+ return values
+ 0 : File will be migrated
+ -2 : File will not be migrated
+ (This is the return value from gf_defrag_handle_hardlink. Checkout
+ gf_defrag_handle_hardlink for description of "returning -2")
+ -1 : failure
+*/
static inline int
__is_file_migratable (xlator_t *this, loc_t *loc,
struct iatt *stbuf, dict_t *xattrs, int flags)
@@ -228,7 +271,12 @@ __is_file_migratable (xlator_t *this, loc_t *loc,
if (flags == GF_DHT_MIGRATE_HARDLINK) {
ret = gf_defrag_handle_hardlink (this, loc,
xattrs, stbuf);
- if (ret) {
+
+ /*
+ Returning zero will force the file to be remigrated.
+ Checkout gf_defrag_handle_hardlink for more information.
+ */
+ if (ret && ret != -2) {
gf_log (this->name, GF_LOG_WARNING,
"%s: failed to migrate file with link",
loc->path);
@@ -236,8 +284,8 @@ __is_file_migratable (xlator_t *this, loc_t *loc,
} else {
gf_log (this->name, GF_LOG_WARNING,
"%s: file has hardlinks", loc->path);
+ ret = -ENOTSUP;
}
- ret = ENOTSUP;
goto out;
}
@@ -743,9 +791,11 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
/* Check if file can be migrated */
ret = __is_file_migratable (this, loc, &stbuf, xattr_rsp, flag);
- if (ret)
+ if (ret) {
+ if (ret == -2)
+ ret = 0;
goto out;
-
+ }
/* Take care of the special files */
if (!IA_ISREG (stbuf.ia_type)) {
/* Special files */