From 2cb67dea078a2d3f3f650e02eb91cedb3663f5d6 Mon Sep 17 00:00:00 2001

From: Ravishankar N <ravishankar@redhat.com>

Date: Mon, 14 Sep 2015 15:43:31 +0530

Subject: [PATCH 330/330] afr: perform replace-brick in a synctask

Patch in master: http://review.gluster.org/#/c/12169/

Patch in release-3.7: http://review.gluster.org/#/c/12172/

Problem:

replace-brick setxattr is not performed inside a synctask. This can lead

to hangs if the setxattr is executed by epoll thread, as the epoll

thread will be waiting for replies to come where as epoll thread is the

thread that needs to epoll_ctl for reading from socket and listen.

Fix:

Move replace-brick to synctask to prevent epoll thread hang.

This patch is in line with the fix performed in

http://review.gluster.org/#/c/12163/

Change-Id: I0e04fe549428a91b8ca5cfe359ce0526c6c0336d

BUG: 1262291

Signed-off-by: Ravishankar N <ravishankar@redhat.com>

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

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

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

---

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

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

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

 xlators/cluster/afr/src/afr.h             |    6 +++

 4 files changed, 73 insertions(+), 14 deletions(-)

diff --git a/xlators/cluster/afr/src/afr-inode-write.c b/xlators/cluster/afr/src/afr-inode-write.c

index 01390f0..3968b24 100644

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

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

@@ -1099,27 +1099,45 @@ out:

 }

 int

-_afr_handle_replace_brick (xlator_t *this, call_frame_t *frame, loc_t *loc,

-                           int rb_index)

+_afr_handle_replace_brick_cbk (int ret, call_frame_t *frame, void *opaque)

+{

+        afr_replace_brick_args_t *data = NULL;

+

+        data = opaque;

+        loc_wipe (&data->loc);

+        GF_FREE (data);

+        return 0;

+}

+

+int

+_afr_handle_replace_brick (void *opaque)

 {

         afr_local_t     *local          = NULL;

         afr_private_t   *priv           = NULL;

+        int              rb_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;

+        data = opaque;

+        frame = data->frame;

+        rb_index = data->rb_index;

+        this = frame->this;

         priv = this->private;

         local = AFR_FRAME_INIT (frame, op_errno);

         if (!local)

                 goto out;

-        loc_copy (&local->loc, loc);

+        loc_copy (&local->loc, &data->loc);

         gf_log (this->name, GF_LOG_DEBUG, "Child being replaced is : %s",

                 priv->children[rb_index]->name);

-        ret = _afr_handle_replace_brick_type (this, frame, loc, rb_index,

+        ret = _afr_handle_replace_brick_type (this, frame, &local->loc, rb_index,

                                               AFR_METADATA_TRANSACTION);

         if (ret) {

                 op_errno = -ret;

@@ -1132,7 +1150,7 @@ _afr_handle_replace_brick (xlator_t *this, call_frame_t *frame, loc_t *loc,

         local->pending = NULL;

         local->xdata_req = NULL;

-        ret = _afr_handle_replace_brick_type (this, frame, loc, rb_index,

+        ret = _afr_handle_replace_brick_type (this, frame, &local->loc, rb_index,

                                               AFR_ENTRY_TRANSACTION);

         if (ret) {

                 op_errno = -ret;

@@ -1359,32 +1377,60 @@ afr_handle_replace_brick (xlator_t *this, call_frame_t *frame, loc_t *loc,

 {

         int             ret               = -1;

         int             rb_index          = -1;

+        int             op_errno          = EPERM;

         char           *replace_brick     = NULL;

+        afr_replace_brick_args_t *data    = NULL;

         ret =  dict_get_str (dict, GF_AFR_REPLACE_BRICK, &replace_brick);

         if (!ret) {

                 if (frame->root->pid != GF_CLIENT_PID_AFR_SELF_HEALD) {

+                        gf_log (this->name, GF_LOG_ERROR, "'%s' is an internal"

+                                " extended attribute : %s.",

+                                GF_AFR_REPLACE_BRICK, strerror (EPERM));

                         ret = 1;

                         goto out;

                 }

                 rb_index = afr_get_child_index_from_name (this, replace_brick);

-                if (rb_index < 0)

+                if (rb_index < 0) {

                          /* Didn't belong to this replica pair

                           * Just do a no-op

                           */

                         AFR_STACK_UNWIND (setxattr, frame, 0, 0, NULL);

-                else

-                        _afr_handle_replace_brick (this, frame, loc, rb_index);

+                        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;

+                        }

+                }

                 ret = 0;

         }

 out:

         if (ret == 1) {

-                gf_log (this->name, GF_LOG_ERROR, "'%s' is an internal"

-                        " extended attribute : %s.",

-                        GF_AFR_REPLACE_BRICK, strerror (EPERM));

-                AFR_STACK_UNWIND (setxattr, frame, -1, EPERM, NULL);

+                AFR_STACK_UNWIND (setxattr, frame, -1, op_errno, NULL);

                 ret = 0;

         }

         return ret;

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

index fd484e4..6f1eee9 100644

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

+++ b/xlators/cluster/afr/src/afr-mem-types.h

@@ -45,6 +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_end

 };

 #endif

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

index 4793413..d07c7bc 100644

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

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

@@ -45,7 +45,7 @@

  */

 #define GLFS_COMP_BASE_AFR      GLFS_MSGID_COMP_AFR

-#define GLFS_NUM_MESSAGES       38

+#define GLFS_NUM_MESSAGES       39

 #define GLFS_MSGID_END          (GLFS_COMP_BASE_AFR + GLFS_NUM_MESSAGES + 1)

 #define glfs_msg_start_x GLFS_COMP_BASE_AFR, "Invalid: Start of messages"

@@ -332,7 +332,6 @@

 */

 #define AFR_MSG_SELF_HEAL_FAILED                (GLFS_COMP_BASE_AFR + 37)

-

 /*!

  * @messageid 108038

  * @diagnosis

@@ -340,6 +339,13 @@

 */

 #define AFR_MSG_SPLIT_BRAIN_STATUS      (GLFS_COMP_BASE_AFR + 38)

+/*!

+ * @messageid 108039

+ * @diagnosis

+ * @recommendedaction

+*/

+#define AFR_MSG_REPLACE_BRICK_FAILED                (GLFS_COMP_BASE_AFR + 39)

+

 #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 316eeea..19e764a 100644

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

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

@@ -760,6 +760,12 @@ typedef struct afr_spb_status {

         loc_t        *loc;

 } afr_spb_status_t;

+typedef struct afr_replace_brick_args {

+        call_frame_t *frame;

+        loc_t loc;

+        int rb_index;

+} afr_replace_brick_args_t;

+

 typedef struct afr_read_subvol_args {

         ia_type_t ia_type;

         uuid_t gfid;

-- 

1.7.1