summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnuradha Talur <atalur@redhat.com>2016-02-23 10:56:51 +0530
committerPranith Kumar Karampuri <pkarampu@redhat.com>2016-03-21 22:37:23 -0700
commit8eaa3506ead4f11b81b146a9e56575c79f3aad7b (patch)
treef34c0efecc327246c94fbfd83ba5649c99fb4c8e
parent020bc022c342c4c015e29c63399757e36d653a49 (diff)
afr : Enable auto heal when replica count increases
This patch is part two change to prevent data loss in a replicate volume on doing a add-brick operation. Problem: After doing add-brick, there is a chance that self heal might happen from the newly added brick rather than the source brick, leading to data loss. Solution: Mark pending changelogs on afr children for the new afr-child so that heal is performed in the correct direction. Change-Id: I11871e55eef3593aec874f92214a2d97da229b17 BUG: 1276203 Signed-off-by: Anuradha Talur <atalur@redhat.com> Reviewed-on: http://review.gluster.org/12454 Smoke: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com> Tested-by: Pranith Kumar Karampuri <pkarampu@redhat.com> CentOS-regression: Gluster Build System <jenkins@build.gluster.com> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
-rw-r--r--tests/basic/afr/add-brick-self-heal.t67
-rw-r--r--xlators/cluster/afr/src/afr-common.c11
-rw-r--r--xlators/cluster/afr/src/afr-inode-write.c184
-rw-r--r--xlators/cluster/afr/src/afr-mem-types.h2
-rw-r--r--xlators/cluster/afr/src/afr-messages.h10
-rw-r--r--xlators/cluster/afr/src/afr.h9
6 files changed, 201 insertions, 82 deletions
diff --git a/tests/basic/afr/add-brick-self-heal.t b/tests/basic/afr/add-brick-self-heal.t
new file mode 100644
index 00000000000..748d36758e7
--- /dev/null
+++ b/tests/basic/afr/add-brick-self-heal.t
@@ -0,0 +1,67 @@
+#!/bin/bash
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+cleanup;
+
+TEST glusterd
+TEST pidof glusterd
+TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}{0,1}
+TEST $CLI volume start $V0
+TEST $CLI volume set $V0 cluster.data-self-heal off
+TEST $CLI volume set $V0 cluster.metadata-self-heal off
+TEST $CLI volume set $V0 cluster.entry-self-heal off
+
+TEST $CLI volume set $V0 self-heal-daemon off
+TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 $M0;
+
+# Create files
+for i in {1..5}
+do
+ echo $i > $M0/file$i.txt
+done
+
+# Metadata changes
+TEST setfattr -n user.test -v qwerty $M0/file5.txt
+
+# Add brick1
+TEST $CLI volume add-brick $V0 replica 3 $H0:$B0/${V0}2
+
+# New-brick should accuse the old-bricks (Simulating case for data-loss)
+TEST setfattr -n trusted.afr.$V0-client-0 -v 0x000000000000000000000001 $B0/${V0}2/
+TEST setfattr -n trusted.afr.$V0-client-1 -v 0x000000000000000000000001 $B0/${V0}2/
+
+# Check if pending xattr and dirty-xattr are set for newly-added-brick
+EXPECT "000000000000000100000001" get_hex_xattr trusted.afr.$V0-client-2 $B0/${V0}0
+EXPECT "000000000000000100000001" get_hex_xattr trusted.afr.$V0-client-2 $B0/${V0}1
+EXPECT "000000000000000000000001" get_hex_xattr trusted.afr.dirty $B0/${V0}2
+
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 0
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 1
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 2
+
+TEST $CLI volume set $V0 self-heal-daemon on
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "Y" glustershd_up_status
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 0
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 1
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 2
+TEST $CLI volume heal $V0
+
+# Wait for heal to complete
+EXPECT_WITHIN $HEAL_TIMEOUT "^0$" get_pending_heal_count $V0
+
+# Check if entry-heal has happened
+TEST diff <(ls $B0/${V0}0 | sort) <(ls $B0/${V0}2 | sort)
+TEST diff <(ls $B0/${V0}1 | sort) <(ls $B0/${V0}2 | sort)
+
+# Test if data was healed
+TEST diff $B0/${V0}0/file1.txt $B0/${V0}2/file1.txt
+
+# Test if metadata was healed and exists on both the bricks
+EXPECT "qwerty" get_text_xattr user.test $B0/${V0}2/file5.txt
+EXPECT "qwerty" get_text_xattr user.test $B0/${V0}0/file5.txt
+
+EXPECT "000000000000000000000000" get_hex_xattr trusted.afr.$V0-client-2 $B0/${V0}0
+EXPECT "000000000000000000000000" get_hex_xattr trusted.afr.$V0-client-2 $B0/${V0}1
+EXPECT "000000000000000000000000" get_hex_xattr trusted.afr.dirty $B0/${V0}2
+
+cleanup;
diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index e0f530f999a..8c04cfbff2f 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -5113,3 +5113,14 @@ afr_get_need_heal (xlator_t *this)
UNLOCK (&priv->lock);
return need_heal;
}
+
+int
+afr_get_msg_id (char *op_type)
+{
+
+ if (!strcmp (op_type, GF_AFR_REPLACE_BRICK))
+ return AFR_MSG_REPLACE_BRICK_STATUS;
+ else if (!strcmp (op_type, GF_AFR_ADD_BRICK))
+ return AFR_MSG_ADD_BRICK_STATUS;
+ return -1;
+}
diff --git a/xlators/cluster/afr/src/afr-inode-write.c b/xlators/cluster/afr/src/afr-inode-write.c
index 6b9d5ee87b0..354de84a2ba 100644
--- a/xlators/cluster/afr/src/afr-inode-write.c
+++ b/xlators/cluster/afr/src/afr-inode-write.c
@@ -1009,14 +1009,15 @@ afr_setxattr_wind (call_frame_t *frame, xlator_t *this, int subvol)
}
int
-afr_rb_set_pending_changelog_cbk (call_frame_t *frame, void *cookie,
+afr_emptyb_set_pending_changelog_cbk (call_frame_t *frame, void *cookie,
xlator_t *this, int op_ret, int op_errno,
dict_t *xattr, dict_t *xdata)
{
afr_local_t *local = NULL;
afr_private_t *priv = NULL;
- int i = 0;
+ int i, ret = 0;
+ char *op_type = NULL;
local = frame->local;
priv = this->private;
@@ -1025,19 +1026,26 @@ afr_rb_set_pending_changelog_cbk (call_frame_t *frame, void *cookie,
local->replies[i].valid = 1;
local->replies[i].op_ret = op_ret;
local->replies[i].op_errno = op_errno;
+
+ ret = dict_get_str (local->xdata_req, "replicate-brick-op", &op_type);
+ if (ret)
+ goto out;
+
gf_msg (this->name, op_ret ? GF_LOG_ERROR : GF_LOG_INFO,
op_ret ? op_errno : 0,
- AFR_MSG_REPLACE_BRICK_STATUS, "Set of pending xattr %s on"
+ afr_get_msg_id (op_type),
+ "Set of pending xattr %s on"
" %s.", op_ret ? "failed" : "succeeded",
priv->children[i]->name);
+out:
syncbarrier_wake (&local->barrier);
return 0;
}
int
-afr_rb_set_pending_changelog (call_frame_t *frame, xlator_t *this,
- unsigned char *locked_nodes)
+afr_emptyb_set_pending_changelog (call_frame_t *frame, xlator_t *this,
+ unsigned char *locked_nodes)
{
afr_local_t *local = NULL;
afr_private_t *priv = NULL;
@@ -1046,9 +1054,9 @@ afr_rb_set_pending_changelog (call_frame_t *frame, xlator_t *this,
local = frame->local;
priv = this->private;
- AFR_ONLIST (locked_nodes, frame, afr_rb_set_pending_changelog_cbk,
+ AFR_ONLIST (locked_nodes, frame, afr_emptyb_set_pending_changelog_cbk,
xattrop, &local->loc, GF_XATTROP_ADD_ARRAY,
- local->xdata_req, NULL);
+ local->xattr_req, NULL);
/* It is sufficient if xattrop was successful on one child */
for (i = 0; i < priv->child_count; i++) {
@@ -1068,9 +1076,10 @@ out:
}
int
-_afr_handle_replace_brick_type (xlator_t *this, call_frame_t *frame,
- loc_t *loc, int rb_index,
- afr_transaction_type type)
+_afr_handle_empty_brick_type (xlator_t *this, call_frame_t *frame,
+ loc_t *loc, int empty_index,
+ afr_transaction_type type,
+ char *op_type)
{
afr_local_t *local = NULL;
afr_private_t *priv = NULL;
@@ -1091,13 +1100,21 @@ _afr_handle_replace_brick_type (xlator_t *this, call_frame_t *frame,
if (!local->pending)
goto out;
- local->pending[rb_index][idx] = hton32 (1);
+ local->pending[empty_index][idx] = hton32 (1);
local->xdata_req = dict_new ();
if (!local->xdata_req)
goto out;
- ret = afr_set_pending_dict (priv, local->xdata_req, local->pending);
+ ret = dict_set_str (local->xdata_req, "replicate-brick-op", op_type);
+ if (ret)
+ goto out;
+
+ local->xattr_req = dict_new ();
+ if (!local->xattr_req)
+ goto out;
+
+ ret = afr_set_pending_dict (priv, local->xattr_req, local->pending);
if (ret < 0)
goto out;
@@ -1118,7 +1135,7 @@ _afr_handle_replace_brick_type (xlator_t *this, call_frame_t *frame,
goto unlock;
}
- ret = afr_rb_set_pending_changelog (frame, this, locked_nodes);
+ ret = afr_emptyb_set_pending_changelog (frame, this, locked_nodes);
if (ret)
goto unlock;
ret = 0;
@@ -1134,33 +1151,41 @@ out:
return ret;
}
-int
-_afr_handle_replace_brick_cbk (int ret, call_frame_t *frame, void *opaque)
+void
+afr_brick_args_cleanup (void *opaque)
{
- afr_replace_brick_args_t *data = NULL;
+ afr_empty_brick_args_t *data = NULL;
data = opaque;
loc_wipe (&data->loc);
GF_FREE (data);
+}
+
+int
+_afr_handle_empty_brick_cbk (int ret, call_frame_t *frame, void *opaque)
+{
+ afr_brick_args_cleanup (opaque);
return 0;
}
int
-_afr_handle_replace_brick (void *opaque)
+_afr_handle_empty_brick (void *opaque)
{
afr_local_t *local = NULL;
afr_private_t *priv = NULL;
- int rb_index = -1;
+ int empty_index = -1;
int ret = -1;
int op_errno = ENOMEM;
call_frame_t *frame = NULL;
xlator_t *this = NULL;
- afr_replace_brick_args_t *data = NULL;
+ char *op_type = NULL;
+ afr_empty_brick_args_t *data = NULL;
data = opaque;
frame = data->frame;
- rb_index = data->rb_index;
+ empty_index = data->empty_index;
+ op_type = data->op_type;
this = frame->this;
priv = this->private;
@@ -1170,11 +1195,11 @@ _afr_handle_replace_brick (void *opaque)
loc_copy (&local->loc, &data->loc);
- gf_msg_debug (this->name, 0, "Child being replaced is : %s",
- priv->children[rb_index]->name);
+ gf_msg_debug (this->name, 0, "New brick is : %s",
+ priv->children[empty_index]->name);
- ret = _afr_handle_replace_brick_type (this, frame, &local->loc, rb_index,
- AFR_METADATA_TRANSACTION);
+ ret = _afr_handle_empty_brick_type (this, frame, &local->loc, empty_index,
+ AFR_METADATA_TRANSACTION, op_type);
if (ret) {
op_errno = -ret;
ret = -1;
@@ -1182,12 +1207,14 @@ _afr_handle_replace_brick (void *opaque)
}
dict_unref (local->xdata_req);
+ dict_unref (local->xattr_req);
afr_matrix_cleanup (local->pending, priv->child_count);
local->pending = NULL;
+ local->xattr_req = NULL;
local->xdata_req = NULL;
- ret = _afr_handle_replace_brick_type (this, frame, &local->loc, rb_index,
- AFR_ENTRY_TRANSACTION);
+ ret = _afr_handle_empty_brick_type (this, frame, &local->loc, empty_index,
+ AFR_ENTRY_TRANSACTION, op_type);
if (ret) {
op_errno = -ret;
ret = -1;
@@ -1408,63 +1435,71 @@ afr_handle_spb_choice_timeout (xlator_t *this, call_frame_t *frame,
}
int
-afr_handle_replace_brick (xlator_t *this, call_frame_t *frame, loc_t *loc,
- dict_t *dict)
+afr_handle_empty_brick (xlator_t *this, call_frame_t *frame, loc_t *loc,
+ dict_t *dict)
{
int ret = -1;
- int rb_index = -1;
+ int ab_ret = -1;
+ int empty_index = -1;
int op_errno = EPERM;
- char *replace_brick = NULL;
- afr_replace_brick_args_t *data = NULL;
+ char *empty_brick = NULL;
+ char *op_type = NULL;
+ afr_empty_brick_args_t *data = NULL;
- ret = dict_get_str (dict, GF_AFR_REPLACE_BRICK, &replace_brick);
+ ret = dict_get_str (dict, GF_AFR_REPLACE_BRICK, &empty_brick);
+ if (!ret)
+ op_type = GF_AFR_REPLACE_BRICK;
- if (!ret) {
- if (frame->root->pid != GF_CLIENT_PID_SELF_HEALD) {
- gf_msg (this->name, GF_LOG_ERROR, EPERM,
- AFR_MSG_REPLACE_BRICK_STATUS, "'%s' is an "
- "internal extended attribute",
- GF_AFR_REPLACE_BRICK);
+ ab_ret = dict_get_str (dict, GF_AFR_ADD_BRICK, &empty_brick);
+ if (!ab_ret)
+ op_type = GF_AFR_ADD_BRICK;
+
+ if (ret && ab_ret)
+ goto out;
+
+ if (frame->root->pid != GF_CLIENT_PID_SELF_HEALD) {
+ gf_msg (this->name, GF_LOG_ERROR, EPERM,
+ afr_get_msg_id (op_type),
+ "'%s' is an internal extended attribute.",
+ op_type);
+ ret = 1;
+ goto out;
+ }
+ empty_index = afr_get_child_index_from_name (this, empty_brick);
+
+ if (empty_index < 0) {
+ /* Didn't belong to this replica pair
+ * Just do a no-op
+ */
+ AFR_STACK_UNWIND (setxattr, frame, 0, 0, NULL);
+ return 0;
+ } else {
+ data = GF_CALLOC (1, sizeof (*data),
+ gf_afr_mt_empty_brick_t);
+ if (!data) {
ret = 1;
+ op_errno = ENOMEM;
goto out;
}
- rb_index = afr_get_child_index_from_name (this, replace_brick);
-
- if (rb_index < 0) {
- /* Didn't belong to this replica pair
- * Just do a no-op
- */
- AFR_STACK_UNWIND (setxattr, frame, 0, 0, NULL);
- return 0;
- } else {
- data = GF_CALLOC (1, sizeof (*data),
- gf_afr_mt_replace_brick_t);
- if (!data) {
- ret = 1;
- op_errno = ENOMEM;
- goto out;
- }
- data->frame = frame;
- loc_copy (&data->loc, loc);
- data->rb_index = rb_index;
- ret = synctask_new (this->ctx->env,
- _afr_handle_replace_brick,
- _afr_handle_replace_brick_cbk,
- NULL, data);
- if (ret) {
- gf_msg (this->name, GF_LOG_ERROR, 0,
- AFR_MSG_REPLACE_BRICK_FAILED,
- "Failed to create synctask. Unable to "
- "perform replace-brick.");
- ret = 1;
- op_errno = ENOMEM;
- loc_wipe (&data->loc);
- GF_FREE (data);
- goto out;
- }
+ data->frame = frame;
+ loc_copy (&data->loc, loc);
+ data->empty_index = empty_index;
+ data->op_type = op_type;
+ ret = synctask_new (this->ctx->env,
+ _afr_handle_empty_brick,
+ _afr_handle_empty_brick_cbk,
+ NULL, data);
+ if (ret) {
+ gf_msg (this->name, GF_LOG_ERROR, 0,
+ afr_get_msg_id (op_type),
+ "Failed to create synctask.");
+ ret = 1;
+ op_errno = ENOMEM;
+ afr_brick_args_cleanup (data);
+ goto out;
}
- ret = 0;
}
+ ret = 0;
out:
if (ret == 1) {
AFR_STACK_UNWIND (setxattr, frame, -1, op_errno, NULL);
@@ -1487,7 +1522,8 @@ afr_handle_special_xattr (xlator_t *this, call_frame_t *frame, loc_t *loc,
if (ret == 0)
goto out;
- ret = afr_handle_replace_brick (this, frame, loc, dict);
+ /* Applicable for replace-brick and add-brick commands */
+ ret = afr_handle_empty_brick (this, frame, loc, dict);
out:
return ret;
}
diff --git a/xlators/cluster/afr/src/afr-mem-types.h b/xlators/cluster/afr/src/afr-mem-types.h
index 6f1eee95322..7f7962013d7 100644
--- a/xlators/cluster/afr/src/afr-mem-types.h
+++ b/xlators/cluster/afr/src/afr-mem-types.h
@@ -45,7 +45,7 @@ enum gf_afr_mem_types_ {
gf_afr_mt_subvol_healer_t,
gf_afr_mt_spbc_timeout_t,
gf_afr_mt_spb_status_t,
- gf_afr_mt_replace_brick_t,
+ gf_afr_mt_empty_brick_t,
gf_afr_mt_end
};
#endif
diff --git a/xlators/cluster/afr/src/afr-messages.h b/xlators/cluster/afr/src/afr-messages.h
index fca94eeff6b..37736f5afa3 100644
--- a/xlators/cluster/afr/src/afr-messages.h
+++ b/xlators/cluster/afr/src/afr-messages.h
@@ -336,14 +336,16 @@
/*!
* @messageid 108039
- * @diagnosis
- * @recommendedaction
+ * @diagnosis Setting of pending xattrs succeeded/failed during add-brick
+ * operation.
+ * @recommendedaction In case of failure, error number in the log should give
+ * the reason why it failed. Also observe brick logs for more information.
*/
-#define AFR_MSG_REPLACE_BRICK_FAILED (GLFS_COMP_BASE_AFR + 39)
+#define AFR_MSG_ADD_BRICK_STATUS (GLFS_COMP_BASE_AFR + 39)
/*!
- * @messageid 108039
+ * @messageid 108040
* @diagnosis AFR was unable to be loaded because the pending-changelog xattrs
* were not found in the volfile.
* @recommendedaction Please ensure cluster op-version is atleast 30707 and the
diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h
index 41ffa4ecdb2..56682b47a56 100644
--- a/xlators/cluster/afr/src/afr.h
+++ b/xlators/cluster/afr/src/afr.h
@@ -778,11 +778,12 @@ typedef struct afr_spb_status {
loc_t *loc;
} afr_spb_status_t;
-typedef struct afr_replace_brick_args {
+typedef struct afr_empty_brick_args {
call_frame_t *frame;
loc_t loc;
- int rb_index;
-} afr_replace_brick_args_t;
+ int empty_index;
+ char *op_type;
+} afr_empty_brick_args_t;
typedef struct afr_read_subvol_args {
ia_type_t ia_type;
@@ -1118,4 +1119,6 @@ afr_set_need_heal (xlator_t *this, afr_local_t *local);
int
afr_selfheal_data_open (xlator_t *this, inode_t *inode, fd_t **fd);
+int
+afr_get_msg_id (char *op_type);
#endif /* __AFR_H__ */