You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
359 lines
12 KiB
359 lines
12 KiB
From 51090a4b3cb000d601083f12d1875547819fc03f Mon Sep 17 00:00:00 2001 |
|
From: Mohit Agrawal <moagrawal@redhat.com> |
|
Date: Wed, 4 Mar 2020 09:17:26 +0530 |
|
Subject: [PATCH 447/449] core[brick_mux]: brick crashed when creating and |
|
deleting volumes over time |
|
|
|
Problem: In brick_mux environment, while volumes are created/stopped in a loop |
|
after running a long time the main brick is crashed.The brick is crashed |
|
because the main brick process was not cleaned up memory for all objects |
|
at the time of detaching a volume. |
|
Below are the objects that are missed at the time of detaching a volume |
|
1) xlator object for a brick graph |
|
2) local_pool for posix_lock xlator |
|
3) rpc object cleanup at quota xlator |
|
4) inode leak at brick xlator |
|
|
|
Solution: To avoid the crash resolve all leak at the time of detaching a brick |
|
> Change-Id: Ibb6e46c5fba22b9441a88cbaf6b3278823235913 |
|
> updates: #977 |
|
> Signed-off-by: Mohit Agrawal <moagrawa@redhat.com> |
|
> (Cherry pick from commit e589d8de66d3325da8fbbbe44d1a5bd6335e08ab) |
|
> (Reviewed on upstream link https://review.gluster.org/#/c/glusterfs/+/24209/) |
|
|
|
BUG: 1790336 |
|
Change-Id: Ibb6e46c5fba22b9441a88cbaf6b3278823235913 |
|
Signed-off-by: Mohit Agrawal <moagrawa@redhat.com> |
|
Reviewed-on: https://code.engineering.redhat.com/gerrit/202782 |
|
Tested-by: RHGS Build Bot <nigelb@redhat.com> |
|
Reviewed-by: Xavi Hernandez Juan <xhernandez@redhat.com> |
|
--- |
|
libglusterfs/src/glusterfs/glusterfs.h | 1 + |
|
libglusterfs/src/graph.c | 1 + |
|
libglusterfs/src/graph.y | 2 +- |
|
libglusterfs/src/xlator.c | 29 ++++++++---- |
|
xlators/features/changelog/src/changelog.c | 1 + |
|
xlators/features/locks/src/posix.c | 4 ++ |
|
xlators/features/quota/src/quota-enforcer-client.c | 14 +++++- |
|
xlators/features/quota/src/quota.c | 54 ++++++++++++++++++++-- |
|
xlators/features/quota/src/quota.h | 3 ++ |
|
xlators/protocol/server/src/server.c | 12 +++-- |
|
10 files changed, 103 insertions(+), 18 deletions(-) |
|
|
|
diff --git a/libglusterfs/src/glusterfs/glusterfs.h b/libglusterfs/src/glusterfs/glusterfs.h |
|
index 177a020..584846e 100644 |
|
--- a/libglusterfs/src/glusterfs/glusterfs.h |
|
+++ b/libglusterfs/src/glusterfs/glusterfs.h |
|
@@ -603,6 +603,7 @@ struct _glusterfs_graph { |
|
int used; /* Should be set when fuse gets |
|
first CHILD_UP */ |
|
uint32_t volfile_checksum; |
|
+ pthread_mutex_t mutex; |
|
}; |
|
typedef struct _glusterfs_graph glusterfs_graph_t; |
|
|
|
diff --git a/libglusterfs/src/graph.c b/libglusterfs/src/graph.c |
|
index bb5e67a..1cd92db 100644 |
|
--- a/libglusterfs/src/graph.c |
|
+++ b/libglusterfs/src/graph.c |
|
@@ -1092,6 +1092,7 @@ glusterfs_graph_destroy_residual(glusterfs_graph_t *graph) |
|
ret = xlator_tree_free_memacct(graph->first); |
|
|
|
list_del_init(&graph->list); |
|
+ pthread_mutex_destroy(&graph->mutex); |
|
GF_FREE(graph); |
|
|
|
return ret; |
|
diff --git a/libglusterfs/src/graph.y b/libglusterfs/src/graph.y |
|
index 5b92985..5733515 100644 |
|
--- a/libglusterfs/src/graph.y |
|
+++ b/libglusterfs/src/graph.y |
|
@@ -541,7 +541,7 @@ glusterfs_graph_new () |
|
return NULL; |
|
|
|
INIT_LIST_HEAD (&graph->list); |
|
- |
|
+ pthread_mutex_init(&graph->mutex, NULL); |
|
gettimeofday (&graph->dob, NULL); |
|
|
|
return graph; |
|
diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c |
|
index 108b96a..36cc32c 100644 |
|
--- a/libglusterfs/src/xlator.c |
|
+++ b/libglusterfs/src/xlator.c |
|
@@ -938,6 +938,8 @@ xlator_mem_cleanup(xlator_t *this) |
|
xlator_list_t **trav_p = NULL; |
|
xlator_t *top = NULL; |
|
xlator_t *victim = NULL; |
|
+ glusterfs_graph_t *graph = NULL; |
|
+ gf_boolean_t graph_cleanup = _gf_false; |
|
|
|
if (this->call_cleanup || !this->ctx) |
|
return; |
|
@@ -945,6 +947,12 @@ xlator_mem_cleanup(xlator_t *this) |
|
this->call_cleanup = 1; |
|
ctx = this->ctx; |
|
|
|
+ inode_table = this->itable; |
|
+ if (inode_table) { |
|
+ inode_table_destroy(inode_table); |
|
+ this->itable = NULL; |
|
+ } |
|
+ |
|
xlator_call_fini(trav); |
|
|
|
while (prev) { |
|
@@ -953,12 +961,6 @@ xlator_mem_cleanup(xlator_t *this) |
|
prev = trav; |
|
} |
|
|
|
- inode_table = this->itable; |
|
- if (inode_table) { |
|
- inode_table_destroy(inode_table); |
|
- this->itable = NULL; |
|
- } |
|
- |
|
if (this->fini) { |
|
this->fini(this); |
|
} |
|
@@ -968,17 +970,28 @@ xlator_mem_cleanup(xlator_t *this) |
|
if (ctx->active) { |
|
top = ctx->active->first; |
|
LOCK(&ctx->volfile_lock); |
|
- /* TODO here we have leak for xlator node in a graph */ |
|
- /* Need to move only top xlator from a graph */ |
|
for (trav_p = &top->children; *trav_p; trav_p = &(*trav_p)->next) { |
|
victim = (*trav_p)->xlator; |
|
if (victim->call_cleanup && !strcmp(victim->name, this->name)) { |
|
+ graph_cleanup = _gf_true; |
|
(*trav_p) = (*trav_p)->next; |
|
break; |
|
} |
|
} |
|
UNLOCK(&ctx->volfile_lock); |
|
} |
|
+ |
|
+ if (graph_cleanup) { |
|
+ prev = this; |
|
+ graph = ctx->active; |
|
+ pthread_mutex_lock(&graph->mutex); |
|
+ while (prev) { |
|
+ trav = prev->next; |
|
+ GF_FREE(prev); |
|
+ prev = trav; |
|
+ } |
|
+ pthread_mutex_unlock(&graph->mutex); |
|
+ } |
|
} |
|
|
|
void |
|
diff --git a/xlators/features/changelog/src/changelog.c b/xlators/features/changelog/src/changelog.c |
|
index ff06c09..b54112c 100644 |
|
--- a/xlators/features/changelog/src/changelog.c |
|
+++ b/xlators/features/changelog/src/changelog.c |
|
@@ -2872,6 +2872,7 @@ fini(xlator_t *this) |
|
if (priv->active || priv->rpc_active) { |
|
/* terminate RPC server/threads */ |
|
changelog_cleanup_rpc(this, priv); |
|
+ GF_FREE(priv->ev_dispatcher); |
|
} |
|
/* call barrier_disable to cancel timer */ |
|
if (priv->barrier_enabled) |
|
diff --git a/xlators/features/locks/src/posix.c b/xlators/features/locks/src/posix.c |
|
index 9a14c64..50f1265 100644 |
|
--- a/xlators/features/locks/src/posix.c |
|
+++ b/xlators/features/locks/src/posix.c |
|
@@ -4102,6 +4102,10 @@ fini(xlator_t *this) |
|
if (!priv) |
|
return; |
|
this->private = NULL; |
|
+ if (this->local_pool) { |
|
+ mem_pool_destroy(this->local_pool); |
|
+ this->local_pool = NULL; |
|
+ } |
|
GF_FREE(priv->brickname); |
|
GF_FREE(priv); |
|
|
|
diff --git a/xlators/features/quota/src/quota-enforcer-client.c b/xlators/features/quota/src/quota-enforcer-client.c |
|
index 1a4c2e3..097439d 100644 |
|
--- a/xlators/features/quota/src/quota-enforcer-client.c |
|
+++ b/xlators/features/quota/src/quota-enforcer-client.c |
|
@@ -362,16 +362,28 @@ quota_enforcer_notify(struct rpc_clnt *rpc, void *mydata, |
|
{ |
|
xlator_t *this = NULL; |
|
int ret = 0; |
|
+ quota_priv_t *priv = NULL; |
|
|
|
this = mydata; |
|
- |
|
+ priv = this->private; |
|
switch (event) { |
|
case RPC_CLNT_CONNECT: { |
|
+ pthread_mutex_lock(&priv->conn_mutex); |
|
+ { |
|
+ priv->conn_status = _gf_true; |
|
+ } |
|
+ pthread_mutex_unlock(&priv->conn_mutex); |
|
gf_msg_trace(this->name, 0, "got RPC_CLNT_CONNECT"); |
|
break; |
|
} |
|
|
|
case RPC_CLNT_DISCONNECT: { |
|
+ pthread_mutex_lock(&priv->conn_mutex); |
|
+ { |
|
+ priv->conn_status = _gf_false; |
|
+ pthread_cond_signal(&priv->conn_cond); |
|
+ } |
|
+ pthread_mutex_unlock(&priv->conn_mutex); |
|
gf_msg_trace(this->name, 0, "got RPC_CLNT_DISCONNECT"); |
|
break; |
|
} |
|
diff --git a/xlators/features/quota/src/quota.c b/xlators/features/quota/src/quota.c |
|
index a0c236d..d1123ce 100644 |
|
--- a/xlators/features/quota/src/quota.c |
|
+++ b/xlators/features/quota/src/quota.c |
|
@@ -5014,6 +5014,43 @@ quota_forget(xlator_t *this, inode_t *inode) |
|
return 0; |
|
} |
|
|
|
+int |
|
+notify(xlator_t *this, int event, void *data, ...) |
|
+{ |
|
+ quota_priv_t *priv = NULL; |
|
+ int ret = 0; |
|
+ rpc_clnt_t *rpc = NULL; |
|
+ gf_boolean_t conn_status = _gf_true; |
|
+ xlator_t *victim = data; |
|
+ |
|
+ priv = this->private; |
|
+ if (!priv || !priv->is_quota_on) |
|
+ goto out; |
|
+ |
|
+ if (event == GF_EVENT_PARENT_DOWN) { |
|
+ rpc = priv->rpc_clnt; |
|
+ if (rpc) { |
|
+ rpc_clnt_disable(rpc); |
|
+ pthread_mutex_lock(&priv->conn_mutex); |
|
+ { |
|
+ conn_status = priv->conn_status; |
|
+ while (conn_status) { |
|
+ (void)pthread_cond_wait(&priv->conn_cond, |
|
+ &priv->conn_mutex); |
|
+ conn_status = priv->conn_status; |
|
+ } |
|
+ } |
|
+ pthread_mutex_unlock(&priv->conn_mutex); |
|
+ gf_log(this->name, GF_LOG_INFO, |
|
+ "Notify GF_EVENT_PARENT_DOWN for brick %s", victim->name); |
|
+ } |
|
+ } |
|
+ |
|
+out: |
|
+ ret = default_notify(this, event, data); |
|
+ return ret; |
|
+} |
|
+ |
|
int32_t |
|
init(xlator_t *this) |
|
{ |
|
@@ -5056,6 +5093,10 @@ init(xlator_t *this) |
|
goto err; |
|
} |
|
|
|
+ pthread_mutex_init(&priv->conn_mutex, NULL); |
|
+ pthread_cond_init(&priv->conn_cond, NULL); |
|
+ priv->conn_status = _gf_false; |
|
+ |
|
if (priv->is_quota_on) { |
|
rpc = quota_enforcer_init(this, this->options); |
|
if (rpc == NULL) { |
|
@@ -5169,20 +5210,22 @@ fini(xlator_t *this) |
|
{ |
|
quota_priv_t *priv = NULL; |
|
rpc_clnt_t *rpc = NULL; |
|
- int i = 0, cnt = 0; |
|
|
|
priv = this->private; |
|
if (!priv) |
|
return; |
|
rpc = priv->rpc_clnt; |
|
priv->rpc_clnt = NULL; |
|
- this->private = NULL; |
|
if (rpc) { |
|
- cnt = GF_ATOMIC_GET(rpc->refcount); |
|
- for (i = 0; i < cnt; i++) |
|
- rpc_clnt_unref(rpc); |
|
+ rpc_clnt_connection_cleanup(&rpc->conn); |
|
+ rpc_clnt_unref(rpc); |
|
} |
|
+ |
|
+ this->private = NULL; |
|
LOCK_DESTROY(&priv->lock); |
|
+ pthread_mutex_destroy(&priv->conn_mutex); |
|
+ pthread_cond_destroy(&priv->conn_cond); |
|
+ |
|
GF_FREE(priv); |
|
if (this->local_pool) { |
|
mem_pool_destroy(this->local_pool); |
|
@@ -5314,6 +5357,7 @@ struct volume_options options[] = { |
|
xlator_api_t xlator_api = { |
|
.init = init, |
|
.fini = fini, |
|
+ .notify = notify, |
|
.reconfigure = reconfigure, |
|
.mem_acct_init = mem_acct_init, |
|
.op_version = {1}, /* Present from the initial version */ |
|
diff --git a/xlators/features/quota/src/quota.h b/xlators/features/quota/src/quota.h |
|
index a5a99ca..e51ffd4 100644 |
|
--- a/xlators/features/quota/src/quota.h |
|
+++ b/xlators/features/quota/src/quota.h |
|
@@ -217,6 +217,9 @@ struct quota_priv { |
|
char *volume_uuid; |
|
uint64_t validation_count; |
|
int32_t quotad_conn_status; |
|
+ pthread_mutex_t conn_mutex; |
|
+ pthread_cond_t conn_cond; |
|
+ gf_boolean_t conn_status; |
|
}; |
|
typedef struct quota_priv quota_priv_t; |
|
|
|
diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c |
|
index a5f09fe..54d9c0f 100644 |
|
--- a/xlators/protocol/server/src/server.c |
|
+++ b/xlators/protocol/server/src/server.c |
|
@@ -409,7 +409,13 @@ server_call_xlator_mem_cleanup(xlator_t *this, char *victim_name) |
|
|
|
arg = calloc(1, sizeof(*arg)); |
|
arg->this = this; |
|
- arg->victim_name = gf_strdup(victim_name); |
|
+ arg->victim_name = strdup(victim_name); |
|
+ if (!arg->victim_name) { |
|
+ gf_smsg(this->name, GF_LOG_CRITICAL, ENOMEM, LG_MSG_NO_MEMORY, |
|
+ "Memory allocation is failed"); |
|
+ return; |
|
+ } |
|
+ |
|
th_ret = gf_thread_create_detached(&th_id, server_graph_janitor_threads, |
|
arg, "graphjanitor"); |
|
if (th_ret) { |
|
@@ -417,7 +423,7 @@ server_call_xlator_mem_cleanup(xlator_t *this, char *victim_name) |
|
"graph janitor Thread" |
|
" creation is failed for brick %s", |
|
victim_name); |
|
- GF_FREE(arg->victim_name); |
|
+ free(arg->victim_name); |
|
free(arg); |
|
} |
|
} |
|
@@ -628,7 +634,7 @@ server_graph_janitor_threads(void *data) |
|
} |
|
|
|
out: |
|
- GF_FREE(arg->victim_name); |
|
+ free(arg->victim_name); |
|
free(arg); |
|
return NULL; |
|
} |
|
-- |
|
1.8.3.1 |
|
|
|
|