From ef36875cd4661de13b74225ac25e9c49914434bb Mon Sep 17 00:00:00 2001

From: Anuradha Talur <atalur@redhat.com>

Date: Tue, 22 Mar 2016 14:03:38 +0530

Subject: [PATCH 45/80] afr : Enable auto heal when replica count increases

            Backport of: http://review.gluster.org/13807

    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: Iae6af44f97e612cb3ee8c642254ec3d15ac063f5

        >BUG: 1320020

        >Signed-off-by: Anuradha Talur <atalur@redhat.com>

        >Reviewed-on: http://review.gluster.org/13807

        >Smoke: Gluster Build System <jenkins@build.gluster.com>

        >NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>

        >CentOS-regression: Gluster Build System <jenkins@build.gluster.com>

        >Reviewed-by: Raghavendra G <rgowdapp@redhat.com>

        >Signed-off-by: Anuradha Talur <atalur@redhat.com>

Change-Id: I216626f860e6e84e274479987216a6cf2d6e5834

Signed-off-by: Anuradha Talur <atalur@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/71426

Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

Tested-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

---

 tests/basic/afr/add-brick-self-heal.t     |   67 ++++++++++

 xlators/cluster/afr/src/afr-common.c      |   11 ++

 xlators/cluster/afr/src/afr-inode-write.c |  187 +++++++++++++++++-----------

 xlators/cluster/afr/src/afr-mem-types.h   |    2 +-

 xlators/cluster/afr/src/afr-messages.h    |   16 ++-

 xlators/cluster/afr/src/afr.h             |    9 +-

 6 files changed, 211 insertions(+), 81 deletions(-)

 create mode 100644 tests/basic/afr/add-brick-self-heal.t

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 0000000..748d367

--- /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 b5d07ac..6b393c7 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 8069d6c..c86fb49 100644

--- a/xlators/cluster/afr/src/afr-inode-write.c

+++ b/xlators/cluster/afr/src/afr-inode-write.c

@@ -1014,14 +1014,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;

@@ -1030,19 +1031,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;

@@ -1051,9 +1059,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++) {

@@ -1073,9 +1081,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;

@@ -1096,13 +1105,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;

@@ -1123,7 +1140,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;

@@ -1139,33 +1156,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;

@@ -1175,11 +1200,13 @@ _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;

@@ -1187,12 +1214,15 @@ _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;

@@ -1413,63 +1443,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);

@@ -1492,7 +1530,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 6f1eee9..7f79620 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 d07c7bc..1fb5b51 100644

--- a/xlators/cluster/afr/src/afr-messages.h

+++ b/xlators/cluster/afr/src/afr-messages.h

@@ -341,11 +341,21 @@

 /*!

  * @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 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

+ * volfiles are regenerated.

+*/

+#define AFR_MSG_NO_CHANGELOG  (GLFS_COMP_BASE_AFR + 40)

 #define glfs_msg_end_x GLFS_MSGID_END, "Invalid: End of messages"

 #endif /* !_AFR_MESSAGES_H_ */

diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h

index 9915344..dec4eaa 100644

--- a/xlators/cluster/afr/src/afr.h

+++ b/xlators/cluster/afr/src/afr.h

@@ -777,11 +777,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;

@@ -1117,4 +1118,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__ */

-- 

1.7.1