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