From 86a24b1c3d749721ba54137abb8c9b62e87be77a Mon Sep 17 00:00:00 2001
From: Samikshan Bairagya <samikshan@gmail.com>
Date: Mon, 20 Feb 2017 18:35:01 +0530
Subject: [PATCH 341/361] core: Clean up pmap registry up correctly on
volume/brick stop
This commit changes the following:
1. In glusterfs_handle_terminate, send out individual pmap signout
requests to glusterd for every brick.
2. Add another parameter to glusterfs_mgmt_pmap_signout function to
pass the brickname that needs to be removed from the pmap registry.
3. Make sure pmap_registry_search doesn't break out from the loop
iterating over the list of bricks per port if the first brick entry
corresponding to a port is whitespaced out.
4. Make sure the pmap registry entries are removed for other
daemons like snapd.
mainline:
> BUG: 1421590
> Reviewed-on: https://review.gluster.org/16689
> Smoke: Gluster Build System <jenkins@build.gluster.org>
> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
> Reviewed-by: Gaurav Yadav <gyadav@redhat.com>
> Reviewed-by: Jeff Darcy <jdarcy@redhat.com>
(cherry picked from commit 2cf0062984ae2c159cc94bfa7093f6e38bfd99fe)
BUG: 1417815
Change-Id: I69949874435b02699e5708dab811777ccb297174
Signed-off-by: Samikshan Bairagya <samikshan@gmail.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/101322
Tested-by: Milind Changire <mchangir@redhat.com>
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
---
glusterfsd/src/glusterfsd-mgmt.c | 15 ++++--
glusterfsd/src/glusterfsd.c | 2 +-
glusterfsd/src/glusterfsd.h | 2 +-
.../bugs/core/bug-1421590-brick-mux-resuse-ports.t | 55 ++++++++++++++++++++++
xlators/mgmt/glusterd/src/glusterd-pmap.c | 15 +++---
5 files changed, 75 insertions(+), 14 deletions(-)
create mode 100644 tests/bugs/core/bug-1421590-brick-mux-resuse-ports.t
diff --git a/glusterfsd/src/glusterfsd-mgmt.c b/glusterfsd/src/glusterfsd-mgmt.c
index 44ddc64..9cb8b41 100644
--- a/glusterfsd/src/glusterfsd-mgmt.c
+++ b/glusterfsd/src/glusterfsd-mgmt.c
@@ -236,6 +236,7 @@ glusterfs_handle_terminate (rpcsvc_request_t *req)
gf_log (THIS->name, GF_LOG_INFO,
"terminating after loss of last child %s",
xlator_req.name);
+ glusterfs_mgmt_pmap_signout (glusterfsd_ctx, xlator_req.name);
cleanup_and_exit (SIGTERM);
} else {
/*
@@ -248,6 +249,8 @@ glusterfs_handle_terminate (rpcsvc_request_t *req)
gf_log (THIS->name, GF_LOG_INFO, "detaching not-only child %s",
xlator_req.name);
top->notify (top, GF_EVENT_TRANSPORT_CLEANUP, victim);
+ glusterfs_mgmt_pmap_signout (glusterfsd_ctx, xlator_req.name);
+
*trav_p = (*trav_p)->next;
glusterfs_autoscale_threads (THIS->ctx, -1);
}
@@ -2560,7 +2563,7 @@ out:
int
-glusterfs_mgmt_pmap_signout (glusterfs_ctx_t *ctx)
+glusterfs_mgmt_pmap_signout (glusterfs_ctx_t *ctx, char *brickname)
{
int ret = 0;
pmap_signout_req req = {0, };
@@ -2571,7 +2574,7 @@ glusterfs_mgmt_pmap_signout (glusterfs_ctx_t *ctx)
frame = create_frame (THIS, ctx->pool);
cmd_args = &ctx->cmd_args;
- if (!cmd_args->brick_port || !cmd_args->brick_name) {
+ if (!cmd_args->brick_port && (!cmd_args->brick_name || !brickname)) {
gf_log ("fsd-mgmt", GF_LOG_DEBUG,
"portmapper signout arguments not given");
goto out;
@@ -2582,8 +2585,12 @@ glusterfs_mgmt_pmap_signout (glusterfs_ctx_t *ctx)
snprintf (brick_name, sizeof(brick_name), "%s.rdma",
cmd_args->brick_name);
req.brick = brick_name;
- } else
- req.brick = cmd_args->brick_name;
+ } else {
+ if (brickname)
+ req.brick = brickname;
+ else
+ req.brick = cmd_args->brick_name;
+ }
req.port = cmd_args->brick_port;
req.rdma_port = cmd_args->brick_port2;
diff --git a/glusterfsd/src/glusterfsd.c b/glusterfsd/src/glusterfsd.c
index 485da4e..cbc1b93 100644
--- a/glusterfsd/src/glusterfsd.c
+++ b/glusterfsd/src/glusterfsd.c
@@ -1291,7 +1291,7 @@ cleanup_and_exit (int signum)
return;
ctx->cleanup_started = 1;
- glusterfs_mgmt_pmap_signout (ctx);
+ glusterfs_mgmt_pmap_signout (ctx, NULL);
/* below part is a racy code where the rpcsvc object is freed.
* But in another thread (epoll thread), upon poll error in the
diff --git a/glusterfsd/src/glusterfsd.h b/glusterfsd/src/glusterfsd.h
index 62aae2a..e13896d 100644
--- a/glusterfsd/src/glusterfsd.h
+++ b/glusterfsd/src/glusterfsd.h
@@ -111,7 +111,7 @@ struct _gfd_vol_top_priv_t {
};
typedef struct _gfd_vol_top_priv_t gfd_vol_top_priv_t;
-int glusterfs_mgmt_pmap_signout (glusterfs_ctx_t *ctx);
+int glusterfs_mgmt_pmap_signout (glusterfs_ctx_t *ctx, char *brick_name);
int glusterfs_mgmt_pmap_signin (glusterfs_ctx_t *ctx);
int glusterfs_volfile_fetch (glusterfs_ctx_t *ctx);
void cleanup_and_exit (int signum);
diff --git a/tests/bugs/core/bug-1421590-brick-mux-resuse-ports.t b/tests/bugs/core/bug-1421590-brick-mux-resuse-ports.t
new file mode 100644
index 0000000..ed401f6
--- /dev/null
+++ b/tests/bugs/core/bug-1421590-brick-mux-resuse-ports.t
@@ -0,0 +1,55 @@
+#!/bin/bash
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../traps.rc
+. $(dirname $0)/../../volume.rc
+
+function get_nth_brick_port_for_volume () {
+ local VOL=$1
+ local n=$2
+
+ $CLI volume status $VOL --xml | sed -ne 's/.*<port>\([-0-9]*\)<\/port>/\1/p' \
+ | head -n $n | tail -n 1
+}
+
+TEST glusterd
+
+TEST $CLI volume set all cluster.brick-multiplex on
+push_trapfunc "$CLI volume set all cluster.brick-multiplex off"
+push_trapfunc "cleanup"
+
+TEST $CLI volume create $V0 $H0:$B0/brick{0,1}
+TEST $CLI volume start $V0
+
+port_brick0=$(get_nth_brick_port_for_volume $V0 1)
+
+# restart the volume
+TEST $CLI volume stop $V0
+TEST $CLI volume start $V0
+
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT $port_brick0 get_nth_brick_port_for_volume $V0 1
+
+TEST $CLI volume stop $V0
+TEST $CLI volume set all cluster.brick-multiplex off
+
+TEST $CLI volume start $V0
+
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT $port_brick0 get_nth_brick_port_for_volume $V0 1
+
+port_brick1=$(get_nth_brick_port_for_volume $V0 2)
+
+# restart the volume
+TEST $CLI volume stop $V0
+TEST $CLI volume start $V0
+
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT $port_brick0 get_nth_brick_port_for_volume $V0 1
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT $port_brick1 get_nth_brick_port_for_volume $V0 2
+
+TEST $CLI volume stop $V0
+
+TEST $CLI volume set all cluster.brick-multiplex on
+
+TEST $CLI volume start $V0
+
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT $port_brick0 get_nth_brick_port_for_volume $V0 1
+
diff --git a/xlators/mgmt/glusterd/src/glusterd-pmap.c b/xlators/mgmt/glusterd/src/glusterd-pmap.c
index 8a39fc2..d87c31e 100644
--- a/xlators/mgmt/glusterd/src/glusterd-pmap.c
+++ b/xlators/mgmt/glusterd/src/glusterd-pmap.c
@@ -119,9 +119,9 @@ pmap_registry_search (xlator_t *this, const char *brickname,
for (;;) {
for (i = 0; brck[i] && !isspace (brck[i]); ++i)
;
- if (!i) {
+ if (i == 0 && brck[i] == '\0')
break;
- }
+
if (strncmp (brck, brickname, i) == 0) {
/*
* Without this check, we'd break when brck
@@ -134,7 +134,9 @@ pmap_registry_search (xlator_t *this, const char *brickname,
return p;
}
}
+
brck += i;
+
/*
* Skip over *any* amount of whitespace, including
* none (if we're already at the end of the string).
@@ -260,7 +262,6 @@ pmap_registry_bind (xlator_t *this, int port, const char *brickname,
goto out;
p = port;
- pmap->ports[p].type = type;
if (pmap->ports[p].brickname) {
char *tmp = pmap->ports[p].brickname;
asprintf (&pmap->ports[p].brickname, "%s %s", tmp, brickname);
@@ -356,10 +357,9 @@ pmap_registry_remove (xlator_t *this, int port, const char *brickname,
goto out;
p = port;
- goto remove;
}
- if (brickname && strchr (brickname, '/')) {
+ if (brickname) {
p = pmap_registry_search (this, brickname, type, _gf_true);
if (p)
goto remove;
@@ -373,9 +373,8 @@ pmap_registry_remove (xlator_t *this, int port, const char *brickname,
goto out;
remove:
- gf_msg ("pmap", GF_LOG_INFO, 0,
- GD_MSG_BRICK_REMOVE, "removing brick %s on port %d",
- pmap->ports[p].brickname, p);
+ gf_msg ("pmap", GF_LOG_INFO, 0, GD_MSG_BRICK_REMOVE,
+ "removing brick %s on port %d", brickname, p);
if (xprt && (xprt == pmap->ports[p].xprt)) {
pmap->ports[p].xprt = NULL;
--
1.8.3.1