summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMohit Agrawal <moagrawa@redhat.com>2016-06-13 12:41:15 +0530
committerRaghavendra G <rgowdapp@redhat.com>2016-07-26 04:42:18 -0700
commita0f66d107a29e2db5bdd96dec4bc112330b5f5aa (patch)
tree2eb967d946171214bffca92eba29cf1a002ff92d
parent2b5ae915d6dcd6593b5b7554b4b5c38b3bfa441c (diff)
rpc/socket.c: Modify approach to cleanup threads of socket_poller in socket_spawn.
Problem: Current approach to cleanup threads of socket_poller is not appropriate. Solution: Enable detach flag at the time of thread creation in socket_spawn. Fix: Write a new wrapper(gf_create_detach_thread) to create detachable thread instead of store thread ids in a queue. Test: Fix is verfied on gluster process, To test the patch followed below procedure Enable the client.ssl and server.ssl option on the volume Start the volume and count anon segment in pmap output for glusterd process pmap -x <glusterd-pid> | grep "\[ anon \]" | wc -l Stop the volume and check again count of anon segment it should not increase. Backport of commit 2ee48474be32f6ead2f3834677fee89d88348382 > Signed-off-by: Mohit Agrawal <moagrawa@redhat.com> > Change-Id: Ib8f7ec7504ec8f6f74b45ce6719b6fb47f9fdc37 > BUG: 1336508 > Reviewed-on: http://review.gluster.org/14694 > Smoke: Gluster Build System <jenkins@build.gluster.org> > Reviewed-by: Atin Mukherjee <amukherj@redhat.com> > CentOS-regression: Gluster Build System <jenkins@build.gluster.org> > NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org> > Reviewed-by: Jeff Darcy <jdarcy@redhat.com> BUG: 1354394 Change-Id: I271e83e7a210ecd27a7471c53147ceb837a33cad Signed-off-by: Mohit Agrawal <moagrawa@redhat.com> Reviewed-on: http://review.gluster.org/14886 CentOS-regression: Gluster Build System <jenkins@build.gluster.org> Smoke: Gluster Build System <jenkins@build.gluster.org> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org> Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
-rw-r--r--libglusterfs/src/common-utils.c30
-rw-r--r--libglusterfs/src/common-utils.h3
-rw-r--r--libglusterfs/src/libglusterfs-messages.h18
-rw-r--r--rpc/rpc-transport/socket/src/socket.c188
4 files changed, 96 insertions, 143 deletions
diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c
index 337d5ed00fc..7669b6b4ca3 100644
--- a/libglusterfs/src/common-utils.c
+++ b/libglusterfs/src/common-utils.c
@@ -3357,6 +3357,36 @@ gf_thread_create (pthread_t *thread, const pthread_attr_t *attr,
}
int
+gf_thread_create_detached (pthread_t *thread,
+ void *(*start_routine)(void *), void *arg)
+{
+ pthread_attr_t attr;
+ int ret = -1;
+
+ ret = pthread_attr_init (&attr);
+ if (ret) {
+ gf_msg (THIS->name, GF_LOG_ERROR, ret,
+ LG_MSG_PTHREAD_ATTR_INIT_FAILED,
+ "Thread attribute initialization failed");
+ return -1;
+ }
+
+ pthread_attr_setdetachstate (&attr, PTHREAD_CREATE_DETACHED);
+
+ ret = gf_thread_create (thread, &attr, start_routine, arg);
+ if (ret) {
+ gf_msg (THIS->name, GF_LOG_ERROR, ret,
+ LG_MSG_PTHREAD_FAILED,
+ "Thread creation failed");
+ ret = -1;
+ }
+
+ pthread_attr_destroy (&attr);
+
+ return ret;
+}
+
+int
gf_skip_header_section (int fd, int header_len)
{
int ret = -1;
diff --git a/libglusterfs/src/common-utils.h b/libglusterfs/src/common-utils.h
index bfcc480293a..5b330053208 100644
--- a/libglusterfs/src/common-utils.h
+++ b/libglusterfs/src/common-utils.h
@@ -736,6 +736,9 @@ int gf_set_timestamp (const char *src, const char* dest);
int gf_thread_create (pthread_t *thread, const pthread_attr_t *attr,
void *(*start_routine)(void *), void *arg);
+int gf_thread_create_detached (pthread_t *thread,
+ void *(*start_routine)(void *), void *arg);
+
gf_boolean_t
gf_is_service_running (char *pidfile, int *pid);
int
diff --git a/libglusterfs/src/libglusterfs-messages.h b/libglusterfs/src/libglusterfs-messages.h
index 68754e6b869..d18f4cb3112 100644
--- a/libglusterfs/src/libglusterfs-messages.h
+++ b/libglusterfs/src/libglusterfs-messages.h
@@ -36,7 +36,7 @@
*/
#define GLFS_LG_BASE GLFS_MSGID_COMP_LIBGLUSTERFS
-#define GLFS_LG_NUM_MESSAGES 204
+#define GLFS_LG_NUM_MESSAGES 206
#define GLFS_LG_MSGID_END (GLFS_LG_BASE + GLFS_LG_NUM_MESSAGES + 1)
/* Messaged with message IDs */
#define glfs_msg_start_lg GLFS_LG_BASE, "Invalid: Start of messages"
@@ -1746,6 +1746,22 @@
* @recommendedaction
*
*/
+#define LG_MSG_LEASEID_BUF_INIT_FAILED (GLFS_LG_BASE + 205)
+
+/*!
+ * @messageid
+ * @diagnosis
+ * @recommendedaction
+ *
+ */
+#define LG_MSG_PTHREAD_ATTR_INIT_FAILED (GLFS_LG_BASE + 206)
+
+/*!
+ * @messageid
+ * @diagnosis
+ * @recommendedaction
+ *
+ */
/*------------*/
#define glfs_msg_end_lg GLFS_LG_MSGID_END, "Invalid: End of messages"
diff --git a/rpc/rpc-transport/socket/src/socket.c b/rpc/rpc-transport/socket/src/socket.c
index 176dca6340c..298e5348098 100644
--- a/rpc/rpc-transport/socket/src/socket.c
+++ b/rpc/rpc-transport/socket/src/socket.c
@@ -25,6 +25,7 @@
#include "compat-errno.h"
#include "socket-mem-types.h"
#include "timer.h"
+#include "syscall.h"
/* ugly #includes below */
#include "protocol-common.h"
@@ -192,117 +193,6 @@ struct socket_connect_error_state_ {
};
typedef struct socket_connect_error_state_ socket_connect_error_state_t;
-
-/* This timer and queue are used to reap dead threads. The timer triggers every
- * minute and pthread_joins any threads that added themselves to the reap queue
- *
- * TODO: Make the timer configurable? (Not sure if required)
- */
-static gf_timer_t *reap_timer;
-static struct list_head reap_queue;
-static pthread_mutex_t reap_lock = PTHREAD_MUTEX_INITIALIZER;
-const struct timespec reap_ts = {60, 0};
-
-struct tid_wrap {
- struct list_head list;
- pthread_t tid;
-};
-
-/* _socket_reap_own_threads iterated over the queue of tid's and pthread_joins
- * them. If a thread join fails, it logs the failure and continues
- */
-static void
-_socket_reap_own_threads() {
- struct tid_wrap *node = NULL;
- struct tid_wrap *tmp = NULL;
- pthread_t tid = 0;
- int i = 0;
-
- list_for_each_entry_safe (node, tmp, &reap_queue, list) {
- list_del_init (&node->list);
- if (pthread_join (node->tid, NULL)) {
- gf_log (THIS->name, GF_LOG_ERROR,
- "own-thread: failed to join thread (tid: %zu)",
- tid);
- }
- node->tid = 0;
- GF_FREE (node);
- node = NULL;
- i++;
- }
-
- if (i) {
- gf_log (THIS->name, GF_LOG_TRACE, "reaped %d own-threads", i);
- }
-
- return;
-}
-
-/* socket_thread_reaper reaps threads and restarts the reap_timer
- */
-static void
-socket_thread_reaper () {
-
- pthread_mutex_lock (&reap_lock);
-
- gf_timer_call_cancel (THIS->ctx, reap_timer);
- reap_timer = 0;
-
- _socket_reap_own_threads();
-
- reap_timer = gf_timer_call_after (THIS->ctx, reap_ts,
- socket_thread_reaper, NULL);
- if (!reap_timer)
- gf_log (THIS->name, GF_LOG_ERROR,
- "failed to restart socket own-thread reap timer");
-
- pthread_mutex_unlock (&reap_lock);
-
- return;
-}
-
-/* socket_thread_reaper_init initializes reap_timer and reap_queue.
- * Initializations are done only the first time this is called.
- *
- * To make sure that the reap_timer is always run, reaper_init it is better to
- * call this whenever an own-thread is launched
- */
-static void
-socket_thread_reaper_init () {
- pthread_mutex_lock (&reap_lock);
-
- if (reap_timer == NULL) {
- reap_timer = gf_timer_call_after (THIS->ctx, reap_ts,
- socket_thread_reaper, NULL);
- INIT_LIST_HEAD (&reap_queue);
- }
-
- pthread_mutex_unlock (&reap_lock);
-
- return;
-}
-
-/* socket_thread_reaper_add adds the given thread id to the queue of threads
- * that will be reaped by socket_thread_reaper
- * own-threads need to call this with their thread-ids before dying
- */
-static int
-socket_thread_reaper_add (pthread_t tid) {
- struct tid_wrap *node = NULL;
-
- pthread_mutex_lock (&reap_lock);
-
- node = GF_CALLOC (1, sizeof (*node), gf_sock_mt_tid_wrap);
- node->tid = tid;
- INIT_LIST_HEAD (&node->list);
- list_add_tail (&node->list, &reap_queue);
-
- pthread_mutex_unlock (&reap_lock);
-
- return 0;
-}
-
-
static int socket_init (rpc_transport_t *this);
static void
@@ -2646,10 +2536,6 @@ err:
pthread_mutex_unlock(&priv->lock);
rpc_transport_notify (this, RPC_TRANSPORT_DISCONNECT, this);
- /* Add the thread to the reap_queue before freeing up the transport and
- * dying
- */
- socket_thread_reaper_add (priv->thread);
rpc_transport_unref (this);
@@ -2657,11 +2543,11 @@ err:
}
-static void
+static int
socket_spawn (rpc_transport_t *this)
{
socket_private_t *priv = this->private;
-
+ int ret = -1;
switch (priv->ot_state) {
case OT_IDLE:
case OT_PLEASE_DIE:
@@ -2669,7 +2555,7 @@ socket_spawn (rpc_transport_t *this)
default:
gf_log (this->name, GF_LOG_WARNING,
"refusing to start redundant poller");
- return;
+ return ret;
}
priv->ot_gen += 7;
@@ -2677,12 +2563,16 @@ socket_spawn (rpc_transport_t *this)
gf_log (this->name, GF_LOG_TRACE,
"spawning %p with gen %u", this, priv->ot_gen);
- if (gf_thread_create(&priv->thread,NULL,socket_poller,this) != 0) {
+ /* Create thread after enable detach flag */
+
+ ret = gf_thread_create_detached (&priv->thread, socket_poller, this);
+ if (ret) {
gf_log (this->name, GF_LOG_ERROR,
"could not create poll thread");
+ ret = -1;
}
- /* start the reaper thread */
- socket_thread_reaper_init();
+
+ return ret;
}
static int
@@ -2866,30 +2756,36 @@ socket_server_event_handler (int fd, int idx, void *data,
new_priv->is_server = _gf_true;
rpc_transport_ref (new_trans);
- if (new_priv->own_thread) {
- if (pipe(new_priv->pipe) < 0) {
- gf_log(this->name,GF_LOG_ERROR,
- "could not create pipe");
- }
- socket_spawn(new_trans);
- }
- else {
- new_priv->idx =
- event_register (ctx->event_pool,
- new_sock,
- socket_event_handler,
- new_trans,
- 1, 0);
- if (new_priv->idx == -1)
- ret = -1;
- }
+ if (new_priv->own_thread) {
+ if (pipe(new_priv->pipe) < 0) {
+ gf_log(this->name, GF_LOG_ERROR,
+ "could not create pipe");
+ }
+ ret = socket_spawn(new_trans);
+ if (ret) {
+ gf_log(this->name, GF_LOG_ERROR,
+ "could not spawn thread");
+ sys_close (new_priv->pipe[0]);
+ sys_close (new_priv->pipe[1]);
+ }
+ } else {
+ new_priv->idx =
+ event_register (ctx->event_pool,
+ new_sock,
+ socket_event_handler,
+ new_trans,
+ 1, 0);
+ if (new_priv->idx == -1) {
+ ret = -1;
+ gf_log(this->name, GF_LOG_ERROR,
+ "failed to register the socket with event");
+ }
+ }
}
pthread_mutex_unlock (&new_priv->lock);
if (ret == -1) {
- gf_log (this->name, GF_LOG_WARNING,
- "failed to register the socket with event");
- close (new_sock);
+ sys_close (new_sock);
rpc_transport_unref (new_trans);
goto unlock;
}
@@ -3207,7 +3103,15 @@ handler:
}
this->listener = this;
- socket_spawn(this);
+ ret = socket_spawn(this);
+ if (ret) {
+ gf_log(this->name, GF_LOG_ERROR,
+ "could not spawn thread");
+ sys_close (priv->pipe[0]);
+ sys_close (priv->pipe[1]);
+ sys_close (priv->sock);
+ priv->sock = -1;
+ }
}
else {
priv->idx = event_register (ctx->event_pool, priv->sock,