From 0043c63f70776444f69667a4ef9596217ecb42b7 Mon Sep 17 00:00:00 2001 From: Mohit Agrawal Date: Mon, 12 Mar 2018 19:43:15 +0530 Subject: gluster: Sometimes Brick process is crashed at the time of stopping brick Problem: Sometimes brick process is getting crashed at the time of stop brick while brick mux is enabled. Solution: Brick process was getting crashed because of rpc connection was not cleaning properly while brick mux is enabled.In this patch after sending GF_EVENT_CLEANUP notification to xlator(server) waits for all rpc client connection destroy for specific xlator.Once rpc connections are destroyed in server_rpc_notify for all associated client for that brick then call xlator_mem_cleanup for for brick xlator as well as all child xlators.To avoid races at the time of cleanup introduce two new flags at each xlator cleanup_starting, call_cleanup. BUG: 1544090 Signed-off-by: Mohit Agrawal Note: Run all test-cases in separate build (https://review.gluster.org/#/c/19700/) with same patch after enable brick mux forcefully, all test cases are passed. Change-Id: Ic4ab9c128df282d146cf1135640281fcb31997bf updates: bz#1544090 --- xlators/protocol/server/src/server.c | 170 +++++++++++++++++++++++++---------- 1 file changed, 123 insertions(+), 47 deletions(-) (limited to 'xlators/protocol/server/src/server.c') diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c index fe1fb71a7ef..03138689b14 100644 --- a/xlators/protocol/server/src/server.c +++ b/xlators/protocol/server/src/server.c @@ -423,7 +423,16 @@ server_rpc_notify (rpcsvc_t *rpc, void *xl, rpcsvc_event_t event, server_conf_t *conf = NULL; client_t *client = NULL; char *auth_path = NULL; - int ret = -1; + int ret = -1; + gf_boolean_t victim_found = _gf_false; + char *xlator_name = NULL; + glusterfs_ctx_t *ctx = NULL; + xlator_t *top = NULL; + xlator_list_t **trav_p = NULL; + xlator_t *travxl = NULL; + uint64_t xprtrefcount = 0; + struct _child_status *tmp = NULL; + if (!xl || !data) { gf_msg_callingfn ("server", GF_LOG_WARNING, 0, @@ -435,6 +444,7 @@ server_rpc_notify (rpcsvc_t *rpc, void *xl, rpcsvc_event_t event, this = xl; trans = data; conf = this->private; + ctx = glusterfsd_ctx; switch (event) { case RPCSVC_EVENT_ACCEPT: @@ -520,9 +530,47 @@ unref_transport: client = trans->xl_private; if (!client) break; + pthread_mutex_lock (&conf->mutex); + list_for_each_entry (tmp, &conf->child_status->status_list, + status_list) { + if (tmp->name && client->bound_xl && + client->bound_xl->cleanup_starting && + !strcmp (tmp->name, client->bound_xl->name)) { + xprtrefcount = GF_ATOMIC_GET (tmp->xprtrefcnt); + if (xprtrefcount > 0) { + xprtrefcount = GF_ATOMIC_DEC (tmp->xprtrefcnt); + if (xprtrefcount == 0) + xlator_name = gf_strdup(client->bound_xl->name); + } + break; + } + } + pthread_mutex_unlock (&conf->mutex); gf_client_unref (client); + if (xlator_name) { + if (this->ctx->active) { + top = this->ctx->active->first; + LOCK (&ctx->volfile_lock); + for (trav_p = &top->children; *trav_p; + trav_p = &(*trav_p)->next) { + travxl = (*trav_p)->xlator; + if (!travxl->call_cleanup && + strcmp (travxl->name, xlator_name) == 0) { + victim_found = _gf_true; + break; + } + } + UNLOCK (&ctx->volfile_lock); + if (victim_found) { + xlator_mem_cleanup (travxl); + rpcsvc_autoscale_threads (ctx, conf->rpc, -1); + } + } + GF_FREE (xlator_name); + } + trans->xl_private = NULL; break; default: @@ -966,6 +1014,7 @@ server_init (xlator_t *this) conf->child_status = GF_CALLOC (1, sizeof (struct _child_status), gf_server_mt_child_status); INIT_LIST_HEAD (&conf->child_status->status_list); + GF_ATOMIC_INIT (conf->child_status->xprtrefcnt, 0); /*ret = dict_get_str (this->options, "statedump-path", &statedump_path); if (!ret) { @@ -1331,14 +1380,53 @@ server_process_child_event (xlator_t *this, int32_t event, void *data, int ret = -1; server_conf_t *conf = NULL; rpc_transport_t *xprt = NULL; + xlator_t *victim = NULL; + struct _child_status *tmp = NULL; GF_VALIDATE_OR_GOTO(this->name, data, out); conf = this->private; GF_VALIDATE_OR_GOTO(this->name, conf, out); + victim = data; pthread_mutex_lock (&conf->mutex); { + if (cbk_procnum == GF_CBK_CHILD_UP) { + list_for_each_entry (tmp, &conf->child_status->status_list, + status_list) { + if (tmp->name == NULL) + break; + if (strcmp (tmp->name, victim->name) == 0) + break; + } + if (tmp->name) { + tmp->child_up = _gf_true; + } else { + tmp = GF_CALLOC (1, sizeof (struct _child_status), + gf_server_mt_child_status); + INIT_LIST_HEAD (&tmp->status_list); + tmp->name = gf_strdup (victim->name); + tmp->child_up = _gf_true; + list_add_tail (&tmp->status_list, + &conf->child_status->status_list); + } + } + + if (cbk_procnum == GF_CBK_CHILD_DOWN) { + list_for_each_entry (tmp, &conf->child_status->status_list, + status_list) { + if (strcmp (tmp->name, victim->name) == 0) { + tmp->child_up = _gf_false; + break; + } + } + + if (!tmp->name) + gf_msg (this->name, GF_LOG_ERROR, 0, + PS_MSG_CHILD_STATUS_FAILED, + "No xlator %s is found in " + "child status list", victim->name); + } list_for_each_entry (xprt, &conf->xprt_list, list) { if (!xprt->xl_private) { continue; @@ -1372,6 +1460,8 @@ server_notify (xlator_t *this, int32_t event, void *data, ...) struct _child_status *tmp = NULL; gf_boolean_t victim_found = _gf_false; glusterfs_ctx_t *ctx = NULL; + gf_boolean_t xprt_found = _gf_false; + uint64_t totxprt = 0; GF_VALIDATE_OR_GOTO (THIS->name, this, out); conf = this->private; @@ -1406,24 +1496,6 @@ server_notify (xlator_t *this, int32_t event, void *data, ...) case GF_EVENT_CHILD_UP: { - list_for_each_entry (tmp, &conf->child_status->status_list, - status_list) { - if (tmp->name == NULL) - break; - if (strcmp (tmp->name, victim->name) == 0) - break; - } - if (tmp->name) { - tmp->child_up = _gf_true; - } else { - tmp = GF_CALLOC (1, sizeof (struct _child_status), - gf_server_mt_child_status); - INIT_LIST_HEAD (&tmp->status_list); - tmp->name = gf_strdup (victim->name); - tmp->child_up = _gf_true; - list_add_tail (&tmp->status_list, - &conf->child_status->status_list); - } ret = server_process_child_event (this, event, data, GF_CBK_CHILD_UP); if (ret) { @@ -1438,19 +1510,6 @@ server_notify (xlator_t *this, int32_t event, void *data, ...) case GF_EVENT_CHILD_DOWN: { - list_for_each_entry (tmp, &conf->child_status->status_list, - status_list) { - if (strcmp (tmp->name, victim->name) == 0) { - tmp->child_up = _gf_false; - break; - } - } - if (!tmp->name) - gf_msg (this->name, GF_LOG_ERROR, 0, - PS_MSG_CHILD_STATUS_FAILED, - "No xlator %s is found in " - "child status list", victim->name); - ret = server_process_child_event (this, event, data, GF_CBK_CHILD_DOWN); if (ret) { @@ -1467,6 +1526,28 @@ server_notify (xlator_t *this, int32_t event, void *data, ...) case GF_EVENT_CLEANUP: conf = this->private; pthread_mutex_lock (&conf->mutex); + /* Calculate total no. of xprt available in list for this + brick xlator + */ + list_for_each_entry_safe (xprt, xp_next, + &conf->xprt_list, list) { + if (!xprt->xl_private) { + continue; + } + if (xprt->xl_private->bound_xl == data) { + totxprt++; + } + } + + list_for_each_entry (tmp, &conf->child_status->status_list, + status_list) { + if (strcmp (tmp->name, victim->name) == 0) { + tmp->child_up = _gf_false; + GF_ATOMIC_INIT (tmp->xprtrefcnt, totxprt); + break; + } + } + /* * Disconnecting will (usually) drop the last ref, which will * cause the transport to be unlinked and freed while we're @@ -1482,18 +1563,11 @@ server_notify (xlator_t *this, int32_t event, void *data, ...) gf_log (this->name, GF_LOG_INFO, "disconnecting %s", xprt->peerinfo.identifier); + xprt_found = _gf_true; rpc_transport_disconnect (xprt, _gf_false); } } - list_for_each_entry (tmp, &conf->child_status->status_list, - status_list) { - if (strcmp (tmp->name, victim->name) == 0) - break; - } - if (tmp->name && (strcmp (tmp->name, victim->name) == 0)) { - GF_FREE (tmp->name); - list_del (&tmp->status_list); - } + pthread_mutex_unlock (&conf->mutex); if (this->ctx->active) { top = this->ctx->active->first; @@ -1501,8 +1575,8 @@ server_notify (xlator_t *this, int32_t event, void *data, ...) for (trav_p = &top->children; *trav_p; trav_p = &(*trav_p)->next) { travxl = (*trav_p)->xlator; - if (travxl && - strcmp (travxl->name, victim->name) == 0) { + if (!travxl->call_cleanup && + strcmp (travxl->name, victim->name) == 0) { victim_found = _gf_true; break; } @@ -1511,11 +1585,13 @@ server_notify (xlator_t *this, int32_t event, void *data, ...) glusterfs_delete_volfile_checksum (ctx, victim->volfile_id); UNLOCK (&ctx->volfile_lock); - if (victim_found) - (*trav_p) = (*trav_p)->next; + rpc_clnt_mgmt_pmap_signout (ctx, victim->name); - /* we need the protocol/server xlator here as 'this' */ - rpcsvc_autoscale_threads (ctx, conf->rpc, -1); + + if (!xprt_found && victim_found) { + xlator_mem_cleanup (victim); + rpcsvc_autoscale_threads (ctx, conf->rpc, -1); + } } break; -- cgit