From afcb244f1264af8b0df42b5c79905fd52f01b924 Mon Sep 17 00:00:00 2001

From: Mohammed Rafi KC <rkavunga@redhat.com>

Date: Thu, 15 Nov 2018 13:18:36 +0530

Subject: [PATCH 449/450] glusterd/mux: Optimize brick disconnect handler code

Removed unnecessary iteration during brick disconnect

handler when multiplex is enabled.

	>Change-Id: I62dd3337b7e7da085da5d76aaae206e0b0edff9f

	>fixes: bz#1650115

	>Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>

upstream patch : https://review.gluster.org/#/c/glusterfs/+/21651/

Change-Id: I62dd3337b7e7da085da5d76aaae206e0b0edff9f

BUG: 1649651

Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>

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

Tested-by: RHGS Build Bot <nigelb@redhat.com>

Reviewed-by: Atin Mukherjee <amukherj@redhat.com>

---

 xlators/mgmt/glusterd/src/glusterd-handler.c |  74 ++++------------

 xlators/mgmt/glusterd/src/glusterd-utils.c   | 122 +++++++++++++--------------

 xlators/mgmt/glusterd/src/glusterd-utils.h   |   3 +-

 xlators/mgmt/glusterd/src/glusterd.h         |  21 +++--

 4 files changed, 87 insertions(+), 133 deletions(-)

diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c

index a129afc..cab0dec 100644

--- a/xlators/mgmt/glusterd/src/glusterd-handler.c

+++ b/xlators/mgmt/glusterd/src/glusterd-handler.c

@@ -6046,37 +6046,6 @@ out:

 static int gd_stale_rpc_disconnect_log;

-static int

-glusterd_mark_bricks_stopped_by_proc (glusterd_brick_proc_t *brick_proc) {

-        glusterd_brickinfo_t     *brickinfo        =  NULL;

-        glusterd_brickinfo_t     *brickinfo_tmp    =  NULL;

-        glusterd_volinfo_t       *volinfo          =  NULL;

-        int                       ret              =  -1;

-

-        cds_list_for_each_entry (brickinfo, &brick_proc->bricks, brick_list) {

-                ret =  glusterd_get_volinfo_from_brick (brickinfo->path,

-                                                        &volinfo);

-                if (ret) {

-                        gf_msg (THIS->name, GF_LOG_ERROR, 0,

-                                GD_MSG_VOLINFO_GET_FAIL, "Failed to get volinfo"

-                                " from brick(%s)", brickinfo->path);

-                        goto out;

-                }

-                cds_list_for_each_entry (brickinfo_tmp, &volinfo->bricks,

-                                         brick_list) {

-                        if (strcmp (brickinfo->path,

-                                    brickinfo_tmp->path) == 0) {

-                                glusterd_set_brick_status (brickinfo_tmp,

-                                                           GF_BRICK_STOPPED);

-                                brickinfo_tmp->start_triggered = _gf_false;

-                        }

-                }

-        }

-        return 0;

-out:

-        return ret;

-}

-

 int

 __glusterd_brick_rpc_notify (struct rpc_clnt *rpc, void *mydata,

                              rpc_clnt_event_t event, void *data)

@@ -6087,7 +6056,6 @@ __glusterd_brick_rpc_notify (struct rpc_clnt *rpc, void *mydata,

         glusterd_brickinfo_t    *brickinfo         = NULL;

         glusterd_volinfo_t      *volinfo           = NULL;

         xlator_t                *this              = NULL;

-        int                      temp              = 0;

         int32_t                  pid               = -1;

         glusterd_brickinfo_t    *brickinfo_tmp     = NULL;

         glusterd_brick_proc_t   *brick_proc        = NULL;

@@ -6218,33 +6186,21 @@ __glusterd_brick_rpc_notify (struct rpc_clnt *rpc, void *mydata,

                         }

                 }

-                if (is_brick_mx_enabled()) {

-                        cds_list_for_each_entry (brick_proc, &conf->brick_procs,

-                                                 brick_proc_list) {

-                                cds_list_for_each_entry (brickinfo_tmp,

-                                                         &brick_proc->bricks,

-                                                         brick_list) {

-                                        if (strcmp (brickinfo_tmp->path,

-                                                    brickinfo->path) == 0) {

-                                                ret  = glusterd_mark_bricks_stopped_by_proc

-                                                       (brick_proc);

-                                                if (ret) {

-                                                        gf_msg(THIS->name,

-                                                               GF_LOG_ERROR, 0,

-                                                               GD_MSG_BRICK_STOP_FAIL,

-                                                               "Unable to stop "

-                                                               "bricks of process"

-                                                               " to which brick(%s)"

-                                                               " belongs",

-                                                               brickinfo->path);

-                                                        goto out;

-                                                }

-                                                temp = 1;

-                                                break;

-                                        }

-                                }

-                                if (temp == 1)

-                                        break;

+                if (is_brick_mx_enabled() && glusterd_is_brick_started(brickinfo)) {

+                        brick_proc = brickinfo->brick_proc;

+                        if (!brick_proc)

+                            break;

+                        cds_list_for_each_entry(brickinfo_tmp, &brick_proc->bricks,

+                                                mux_bricks)

+                        {

+                            glusterd_set_brick_status(brickinfo_tmp, GF_BRICK_STOPPED);

+                            brickinfo_tmp->start_triggered = _gf_false;

+                            /* When bricks are stopped, ports also need to

+                             * be cleaned up

+                             */

+                            pmap_registry_remove(

+                                THIS, brickinfo_tmp->port, brickinfo_tmp->path,

+                                GF_PMAP_PORT_BRICKSERVER, NULL, _gf_true);

                         }

                 } else {

                         glusterd_set_brick_status (brickinfo, GF_BRICK_STOPPED);

diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c

index 7179a68..ec7e27a 100644

--- a/xlators/mgmt/glusterd/src/glusterd-utils.c

+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c

@@ -1088,6 +1088,7 @@ glusterd_brickinfo_new (glusterd_brickinfo_t **brickinfo)

                 goto out;

         CDS_INIT_LIST_HEAD (&new_brickinfo->brick_list);

+        CDS_INIT_LIST_HEAD (&new_brickinfo->mux_bricks);

         pthread_mutex_init (&new_brickinfo->restart_mutex, NULL);

         *brickinfo = new_brickinfo;

@@ -1978,6 +1979,7 @@ glusterd_volume_start_glusterfs (glusterd_volinfo_t  *volinfo,

         struct rpc_clnt         *rpc = NULL;

         rpc_clnt_connection_t   *conn  = NULL;

         int                     pid    = -1;

+        glusterd_brick_proc_t   *brick_proc = NULL;

         GF_ASSERT (volinfo);

         GF_ASSERT (brickinfo);

@@ -2188,15 +2190,20 @@ retry:

                 goto out;

         }

-        ret = glusterd_brick_process_add_brick (brickinfo);

+        ret = glusterd_brickprocess_new(&brick_proc);

         if (ret) {

-                gf_msg (this->name, GF_LOG_ERROR, 0,

-                        GD_MSG_BRICKPROC_ADD_BRICK_FAILED, "Adding brick %s:%s "

-                        "to brick process failed.", brickinfo->hostname,

-                        brickinfo->path);

+                gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_BRICKPROC_NEW_FAILED,

+                       "Failed to create new brick process instance");

                 goto out;

         }

+        brick_proc->port = brickinfo->port;

+        cds_list_add_tail(&brick_proc->brick_proc_list, &priv->brick_procs);

+        brickinfo->brick_proc = brick_proc;

+        cds_list_add_tail(&brickinfo->mux_bricks, &brick_proc->bricks);

+        brickinfo->brick_proc = brick_proc;

+        brick_proc->brick_count++;

+

 connect:

         ret = glusterd_brick_connect (volinfo, brickinfo, socketpath);

         if (ret) {

@@ -2328,9 +2335,6 @@ glusterd_brick_process_remove_brick (glusterd_brickinfo_t *brickinfo)

         xlator_t                *this = NULL;

         glusterd_conf_t         *priv = NULL;

         glusterd_brick_proc_t   *brick_proc = NULL;

-        glusterd_brickinfo_t    *brickinfoiter = NULL;

-        glusterd_brick_proc_t   *brick_proc_tmp = NULL;

-        glusterd_brickinfo_t    *tmp = NULL;

         this = THIS;

         GF_VALIDATE_OR_GOTO ("glusterd", this, out);

@@ -2339,48 +2343,44 @@ glusterd_brick_process_remove_brick (glusterd_brickinfo_t *brickinfo)

         GF_VALIDATE_OR_GOTO (this->name, priv, out);

         GF_VALIDATE_OR_GOTO (this->name, brickinfo, out);

-        cds_list_for_each_entry_safe (brick_proc, brick_proc_tmp,

-                                      &priv->brick_procs, brick_proc_list) {

-                if (brickinfo->port != brick_proc->port) {

-                        continue;

-                }

-

-                GF_VALIDATE_OR_GOTO (this->name, (brick_proc->brick_count > 0), out);

+        brick_proc = brickinfo->brick_proc;

+        if (!brick_proc) {

+            if (brickinfo->status != GF_BRICK_STARTED) {

+                /* this function will be called from gluster_pmap_signout and

+                 * glusterd_volume_stop_glusterfs. So it is possible to have

+                 * brick_proc set as null.

+                 */

+                ret = 0;

+            }

+            goto out;

+        }

-                cds_list_for_each_entry_safe (brickinfoiter, tmp,

-                                              &brick_proc->bricks, brick_list) {

-                        if (strcmp (brickinfoiter->path, brickinfo->path) == 0) {

-                                cds_list_del_init (&brickinfoiter->brick_list);

+        GF_VALIDATE_OR_GOTO(this->name, (brick_proc->brick_count > 0), out);

-                                GF_FREE (brickinfoiter->logfile);

-                                GF_FREE (brickinfoiter);

-                                brick_proc->brick_count--;

-                                break;

-                        }

-                }

+        cds_list_del_init(&brickinfo->mux_bricks);

+        brick_proc->brick_count--;

-                /* If all bricks have been removed, delete the brick process */

-                if (brick_proc->brick_count == 0) {

-                        ret = glusterd_brickprocess_delete (brick_proc);

-                        if (ret)

-                                goto out;

-                }

-                break;

+        /* If all bricks have been removed, delete the brick process */

+        if (brick_proc->brick_count == 0) {

+            ret = glusterd_brickprocess_delete(brick_proc);

+            if (ret)

+                goto out;

         }

+        brickinfo->brick_proc = NULL;

         ret = 0;

 out:

         return ret;

 }

 int

-glusterd_brick_process_add_brick (glusterd_brickinfo_t *brickinfo)

+glusterd_brick_process_add_brick (glusterd_brickinfo_t *brickinfo,

+                                  glusterd_brickinfo_t *parent_brickinfo)

 {

         int                      ret = -1;

         xlator_t                *this = NULL;

         glusterd_conf_t         *priv = NULL;

         glusterd_brick_proc_t   *brick_proc = NULL;

-        glusterd_brickinfo_t    *brickinfo_dup = NULL;

         this = THIS;

         GF_VALIDATE_OR_GOTO ("glusterd", this, out);

@@ -2389,37 +2389,28 @@ glusterd_brick_process_add_brick (glusterd_brickinfo_t *brickinfo)

         GF_VALIDATE_OR_GOTO (this->name, priv, out);

         GF_VALIDATE_OR_GOTO (this->name, brickinfo, out);

-        ret = glusterd_brickinfo_new (&brickinfo_dup);

-        if (ret) {

-                gf_msg ("glusterd", GF_LOG_ERROR, 0,

-                        GD_MSG_BRICK_NEW_INFO_FAIL,

-                        "Failed to create new brickinfo");

-                goto out;

-        }

-

-        ret = glusterd_brickinfo_dup (brickinfo, brickinfo_dup);

-        if (ret) {

-                gf_msg (this->name, GF_LOG_ERROR, 0,

-                        GD_MSG_BRICK_SET_INFO_FAIL, "Failed to dup brickinfo");

-                goto out;

-        }

-

-        ret = glusterd_brick_proc_for_port (brickinfo->port, &brick_proc);

-        if (ret) {

-                ret = glusterd_brickprocess_new (&brick_proc);

+        if (!parent_brickinfo) {

+                ret = glusterd_brick_proc_for_port(brickinfo->port,

+                                                   &brick_proc);

                 if (ret) {

-                        gf_msg (this->name, GF_LOG_ERROR, 0,

-                                GD_MSG_BRICKPROC_NEW_FAILED, "Failed to create "

-                                "new brick process instance");

-                        goto out;

+                        ret = glusterd_brickprocess_new (&brick_proc);

+                        if (ret) {

+                                gf_msg(this->name, GF_LOG_ERROR, 0,

+                                       GD_MSG_BRICKPROC_NEW_FAILED,

+                                       "Failed to create "

+                                        "new brick process instance");

+                                goto out;

+                        }

+                        brick_proc->port = brickinfo->port;

+                        cds_list_add_tail(&brick_proc->brick_proc_list,

+                                          &priv->brick_procs);

                 }

-

-                brick_proc->port = brickinfo->port;

-

-                cds_list_add_tail (&brick_proc->brick_proc_list, &priv->brick_procs);

+        } else {

+                ret = 0;

+                brick_proc = parent_brickinfo->brick_proc;

         }

-

-        cds_list_add_tail (&brickinfo_dup->brick_list, &brick_proc->bricks);

+        cds_list_add_tail(&brickinfo->mux_bricks, &brick_proc->bricks);

+        brickinfo->brick_proc = brick_proc;

         brick_proc->brick_count++;

 out:

         return ret;

@@ -2538,6 +2529,7 @@ glusterd_volume_stop_glusterfs (glusterd_volinfo_t *volinfo,

         brickinfo->status = GF_BRICK_STOPPED;

         brickinfo->start_triggered = _gf_false;

+        brickinfo->brick_proc = NULL;

         if (del_brick)

                 glusterd_delete_brick (volinfo, brickinfo);

 out:

@@ -5704,7 +5696,8 @@ attach_brick (xlator_t *this,

                                         goto out;

                                 }

                                 brickinfo->port = other_brick->port;

-                                ret = glusterd_brick_process_add_brick (brickinfo);

+                                ret = glusterd_brick_process_add_brick(brickinfo

+                                                                 , other_brick);

                                 if (ret) {

                                         gf_msg (this->name, GF_LOG_ERROR, 0,

                                                 GD_MSG_BRICKPROC_ADD_BRICK_FAILED,

@@ -6259,7 +6252,8 @@ glusterd_brick_start (glusterd_volinfo_t *volinfo,

                         (void) glusterd_brick_connect (volinfo, brickinfo,

                                         socketpath);

-                        ret = glusterd_brick_process_add_brick (brickinfo);

+                        ret = glusterd_brick_process_add_brick (brickinfo,

+                                                                NULL);

                         if (ret) {

                                 gf_msg (this->name, GF_LOG_ERROR, 0,

                                         GD_MSG_BRICKPROC_ADD_BRICK_FAILED,

diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h

index 8e5320d..69bb8c8 100644

--- a/xlators/mgmt/glusterd/src/glusterd-utils.h

+++ b/xlators/mgmt/glusterd/src/glusterd-utils.h

@@ -179,7 +179,8 @@ int32_t

 glusterd_resolve_brick (glusterd_brickinfo_t *brickinfo);

 int

-glusterd_brick_process_add_brick (glusterd_brickinfo_t *brickinfo);

+glusterd_brick_process_add_brick (glusterd_brickinfo_t *brickinfo,

+                                  glusterd_brickinfo_t *parent_brickinfo);

 int

 glusterd_brick_process_remove_brick (glusterd_brickinfo_t *brickinfo);

diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h

index edd41aa..3dfbf9c 100644

--- a/xlators/mgmt/glusterd/src/glusterd.h

+++ b/xlators/mgmt/glusterd/src/glusterd.h

@@ -211,6 +211,15 @@ typedef enum gf_brick_status {

         GF_BRICK_STARTING

 } gf_brick_status_t;

+struct glusterd_brick_proc {

+        int                     port;

+        uint32_t                brick_count;

+        struct cds_list_head    brick_proc_list;

+        struct cds_list_head    bricks;

+};

+

+typedef struct glusterd_brick_proc glusterd_brick_proc_t;

+

 struct glusterd_brickinfo {

         char               hostname[1024];

         char               path[PATH_MAX];

@@ -249,19 +258,13 @@ struct glusterd_brickinfo {

         gf_boolean_t       port_registered;

         gf_boolean_t       start_triggered;

         pthread_mutex_t    restart_mutex;

+        glusterd_brick_proc_t *brick_proc; /* Information regarding mux bricks */

+        struct cds_list_head mux_bricks;

+        /* List to store the bricks in brick_proc*/

 };

 typedef struct glusterd_brickinfo glusterd_brickinfo_t;

-struct glusterd_brick_proc {

-        int                     port;

-        uint32_t                brick_count;

-        struct cds_list_head    brick_proc_list;

-        struct cds_list_head    bricks;

-};

-

-typedef struct glusterd_brick_proc glusterd_brick_proc_t;

-

 struct gf_defrag_brickinfo_ {

         char *name;

         int   files;

-- 

1.8.3.1