From afcb244f1264af8b0df42b5c79905fd52f01b924 Mon Sep 17 00:00:00 2001 From: Mohammed Rafi KC 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 upstream patch : https://review.gluster.org/#/c/glusterfs/+/21651/ Change-Id: I62dd3337b7e7da085da5d76aaae206e0b0edff9f BUG: 1649651 Signed-off-by: Mohammed Rafi KC Reviewed-on: https://code.engineering.redhat.com/gerrit/156327 Tested-by: RHGS Build Bot Reviewed-by: Atin Mukherjee --- 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