From 86a24b1c3d749721ba54137abb8c9b62e87be77a Mon Sep 17 00:00:00 2001 From: Samikshan Bairagya 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 > NetBSD-regression: NetBSD Build System > CentOS-regression: Gluster Build System > Reviewed-by: Gaurav Yadav > Reviewed-by: Jeff Darcy (cherry picked from commit 2cf0062984ae2c159cc94bfa7093f6e38bfd99fe) BUG: 1417815 Change-Id: I69949874435b02699e5708dab811777ccb297174 Signed-off-by: Samikshan Bairagya Reviewed-on: https://code.engineering.redhat.com/gerrit/101322 Tested-by: Milind Changire Reviewed-by: Atin Mukherjee --- 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/.*\([-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