From 11dd368eae316c3cb9a1e82e7b7213d0ee4224c6 Mon Sep 17 00:00:00 2001
From: Venky Shankar <vshankar@redhat.com>
Date: Thu, 4 Jun 2015 08:50:48 +0530
Subject: [PATCH 159/190] features/bitrot: cleanup, v2
Backport of http://review.gluster.org/11148
This patch uses "cleanup, v1" infrastrcuture to cleanup scrubber
(data structures, threads, timers, etc..) on brick disconnection.
Signer is not cleaned up yet: probably would be done as part of
another patch.
Change-Id: I78a92b8a7f02b2f39078aa9a5a6b101fc499fd70
BUG: 1232309
Signed-off-by: Venky Shankar <vshankar@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/51744
Reviewed-by: Raghavendra Bhat <raghavendra@redhat.com>
Tested-by: Raghavendra Bhat <raghavendra@redhat.com>
---
contrib/timer-wheel/timer-wheel.c | 10 +-
contrib/timer-wheel/timer-wheel.h | 2 +-
libglusterfs/src/common-utils.c | 11 ++
libglusterfs/src/common-utils.h | 3 +
xlators/features/bit-rot/src/bitd/bit-rot-scrub.c | 27 ++-
xlators/features/bit-rot/src/bitd/bit-rot.c | 186 +++++++++++++++++---
xlators/features/bit-rot/src/bitd/bit-rot.h | 12 +-
xlators/features/changelog/src/changelog-helpers.c | 30 +---
8 files changed, 214 insertions(+), 67 deletions(-)
diff --git a/contrib/timer-wheel/timer-wheel.c b/contrib/timer-wheel/timer-wheel.c
index d9b2ccd..013c0f2 100644
--- a/contrib/timer-wheel/timer-wheel.c
+++ b/contrib/timer-wheel/timer-wheel.c
@@ -218,14 +218,20 @@ void gf_tw_add_timer (struct tvec_base *base, struct gf_tw_timer_list *timer)
/**
* Remove a timer from the timer wheel
*/
-void gf_tw_del_timer (struct tvec_base *base, struct gf_tw_timer_list *timer)
+int gf_tw_del_timer (struct tvec_base *base, struct gf_tw_timer_list *timer)
{
+ int ret = 0;
+
pthread_spin_lock (&base->lock);
{
- if (timer_pending (timer))
+ if (timer_pending (timer)) {
+ ret = 1;
__gf_tw_detach_timer (timer);
+ }
}
pthread_spin_unlock (&base->lock);
+
+ return ret;
}
int gf_tw_mod_timer_pending (struct tvec_base *base,
diff --git a/contrib/timer-wheel/timer-wheel.h b/contrib/timer-wheel/timer-wheel.h
index c52f2fc..faa941a 100644
--- a/contrib/timer-wheel/timer-wheel.h
+++ b/contrib/timer-wheel/timer-wheel.h
@@ -66,7 +66,7 @@ struct gf_tw_timer_list {
struct tvec_base *gf_tw_init_timers ();
int gf_tw_cleanup_timers (struct tvec_base *);
void gf_tw_add_timer (struct tvec_base *, struct gf_tw_timer_list *);
-void gf_tw_del_timer (struct tvec_base *, struct gf_tw_timer_list *);
+int gf_tw_del_timer (struct tvec_base *, struct gf_tw_timer_list *);
int gf_tw_mod_timer_pending (struct tvec_base *,
struct gf_tw_timer_list *, unsigned long);
diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c
index be5d897..cbc5230 100644
--- a/libglusterfs/src/common-utils.c
+++ b/libglusterfs/src/common-utils.c
@@ -4026,3 +4026,14 @@ out:
return ret;
}
+void
+_mask_cancellation (void)
+{
+ (void) pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL);
+}
+
+void
+_unmask_cancellation (void)
+{
+ (void) pthread_setcancelstate (PTHREAD_CANCEL_ENABLE, NULL);
+}
diff --git a/libglusterfs/src/common-utils.h b/libglusterfs/src/common-utils.h
index ad87559..a64917f 100644
--- a/libglusterfs/src/common-utils.h
+++ b/libglusterfs/src/common-utils.h
@@ -762,4 +762,7 @@ gf_nread (int fd, void *buf, size_t count);
ssize_t
gf_nwrite (int fd, const void *buf, size_t count);
+void _mask_cancellation (void);
+void _unmask_cancellation (void);
+
#endif /* _COMMON_UTILS_H */
diff --git a/xlators/features/bit-rot/src/bitd/bit-rot-scrub.c b/xlators/features/bit-rot/src/bitd/bit-rot-scrub.c
index 45499de..d6ee413 100644
--- a/xlators/features/bit-rot/src/bitd/bit-rot-scrub.c
+++ b/xlators/features/bit-rot/src/bitd/bit-rot-scrub.c
@@ -393,6 +393,14 @@ br_scrubber_scrub_begin (xlator_t *this, struct br_fsscan_entry *fsentry)
}
static void
+_br_lock_cleaner (void *arg)
+{
+ pthread_mutex_t *mutex = arg;
+
+ pthread_mutex_unlock (mutex);
+}
+
+static void
wait_for_scrubbing (xlator_t *this, struct br_scanfs *fsscan)
{
br_private_t *priv = NULL;
@@ -401,8 +409,10 @@ wait_for_scrubbing (xlator_t *this, struct br_scanfs *fsscan)
priv = this->private;
fsscrub = &priv->fsscrub;
+ pthread_cleanup_push (_br_lock_cleaner, &fsscan->waitlock);
pthread_mutex_lock (&fsscan->waitlock);
{
+ pthread_cleanup_push (_br_lock_cleaner, &fsscrub->mutex);
pthread_mutex_lock (&fsscrub->mutex);
{
list_replace_init (&fsscan->queued, &fsscan->ready);
@@ -411,12 +421,14 @@ wait_for_scrubbing (xlator_t *this, struct br_scanfs *fsscan)
pthread_cond_broadcast (&fsscrub->cond);
}
pthread_mutex_unlock (&fsscrub->mutex);
+ pthread_cleanup_pop (0);
while (fsscan->entries != 0)
pthread_cond_wait
(&fsscan->waitcond, &fsscan->waitlock);
}
pthread_mutex_unlock (&fsscan->waitlock);
+ pthread_cleanup_pop (0);
}
static inline void
@@ -465,6 +477,8 @@ br_fsscanner_handle_entry (xlator_t *subvol,
this = child->this;
fsscan = &child->fsscan;
+ _mask_cancellation ();
+
fsentry = GF_CALLOC (1, sizeof (*fsentry), gf_br_mt_br_fsscan_entry_t);
if (!fsentry)
goto error_return;
@@ -499,6 +513,8 @@ br_fsscanner_handle_entry (xlator_t *subvol,
}
UNLOCK (&fsscan->entrylock);
+ _unmask_cancellation ();
+
if (scrub)
wait_for_scrubbing (this, fsscan);
@@ -535,6 +551,7 @@ br_fsscanner_log_time (xlator_t *this, br_child_t *child, const char *sfx)
static void
br_fsscanner_wait_until_kicked (struct br_scanfs *fsscan)
{
+ pthread_cleanup_push (_br_lock_cleaner, &fsscan->wakelock);
pthread_mutex_lock (&fsscan->wakelock);
{
while (!fsscan->kick)
@@ -543,6 +560,7 @@ br_fsscanner_wait_until_kicked (struct br_scanfs *fsscan)
fsscan->kick = _gf_false;
}
pthread_mutex_unlock (&fsscan->wakelock);
+ pthread_cleanup_pop (0);
}
void *
@@ -778,13 +796,6 @@ br_scrubber_calc_scale (xlator_t *this,
}
-static void
-br_scrubber_cleanup_handler (void *arg)
-{
- struct br_scrubber *fsscrub = arg;
- pthread_mutex_unlock (&fsscrub->mutex);
-}
-
static inline br_child_t *
_br_scrubber_get_next_child (struct br_scrubber *fsscrub)
{
@@ -844,7 +855,7 @@ static void
br_scrubber_pick_entry (struct br_scrubber *fsscrub,
struct br_fsscan_entry **fsentry)
{
- pthread_cleanup_push (br_scrubber_cleanup_handler, fsscrub);
+ pthread_cleanup_push (_br_lock_cleaner, &fsscrub->mutex);
pthread_mutex_lock (&fsscrub->mutex);
{
diff --git a/xlators/features/bit-rot/src/bitd/bit-rot.c b/xlators/features/bit-rot/src/bitd/bit-rot.c
index b855cd7..f2cd1d8 100644
--- a/xlators/features/bit-rot/src/bitd/bit-rot.c
+++ b/xlators/features/bit-rot/src/bitd/bit-rot.c
@@ -1103,7 +1103,7 @@ br_set_child_state (br_child_t *child, br_child_state_t state)
* Also, we register to the changelog library to subscribe for event
* notifications.
*/
-static inline int32_t
+static int32_t
br_enact_signer (xlator_t *this, br_child_t *child, br_stub_init_t *stub)
{
int32_t ret = 0;
@@ -1145,32 +1145,16 @@ br_enact_signer (xlator_t *this, br_child_t *child, br_stub_init_t *stub)
return -1;
}
-static inline int32_t
-br_enact_scrubber (xlator_t *this, br_child_t *child)
+static int32_t
+br_launch_scrubber (xlator_t *this, br_child_t *child,
+ struct br_scanfs *fsscan, struct br_scrubber *fsscrub)
{
- int32_t ret = 0;
+ int32_t ret = -1;
br_private_t *priv = NULL;
- struct br_scanfs *fsscan = NULL;
- struct br_scrubber *fsscrub = NULL;
priv = this->private;
- fsscan = &child->fsscan;
- fsscrub = &priv->fsscrub;
-
- LOCK_INIT (&fsscan->entrylock);
- pthread_mutex_init (&fsscan->waitlock, NULL);
- pthread_cond_init (&fsscan->waitcond, NULL);
-
- fsscan->entries = 0;
- INIT_LIST_HEAD (&fsscan->queued);
- INIT_LIST_HEAD (&fsscan->ready);
-
- /* init scheduler related variables */
fsscan->kick = _gf_false;
- pthread_mutex_init (&fsscan->wakelock, NULL);
- pthread_cond_init (&fsscan->wakecond, NULL);
-
ret = gf_thread_create (&child->thread, NULL, br_fsscanner, child);
if (ret != 0) {
gf_msg (this->name, GF_LOG_ALERT, 0, BRB_MSG_SPAWN_FAILED,
@@ -1186,7 +1170,7 @@ br_enact_scrubber (xlator_t *this, br_child_t *child)
}
pthread_mutex_unlock (&priv->lock);
if (ret)
- goto error_return;
+ goto cleanup_thread;
/**
* Everything has been setup.. add this subvolume to scrubbers
@@ -1201,6 +1185,50 @@ br_enact_scrubber (xlator_t *this, br_child_t *child)
return 0;
+ cleanup_thread:
+ (void) gf_thread_cleanup_xint (child->thread);
+ error_return:
+ return -1;
+}
+
+static int32_t
+br_enact_scrubber (xlator_t *this, br_child_t *child)
+{
+ int32_t ret = 0;
+ br_private_t *priv = NULL;
+ struct br_scanfs *fsscan = NULL;
+ struct br_scrubber *fsscrub = NULL;
+
+ priv = this->private;
+
+ fsscan = &child->fsscan;
+ fsscrub = &priv->fsscrub;
+
+ /**
+ * if this child already witnesses a successfull connection earlier
+ * there's no need to initialize mutexes, condvars, etc..
+ */
+ if (_br_child_witnessed_connection (child))
+ return br_launch_scrubber (this, child, fsscan, fsscrub);
+
+ LOCK_INIT (&fsscan->entrylock);
+ pthread_mutex_init (&fsscan->waitlock, NULL);
+ pthread_cond_init (&fsscan->waitcond, NULL);
+
+ fsscan->entries = 0;
+ INIT_LIST_HEAD (&fsscan->queued);
+ INIT_LIST_HEAD (&fsscan->ready);
+
+ /* init scheduler related variables */
+ pthread_mutex_init (&fsscan->wakelock, NULL);
+ pthread_cond_init (&fsscan->wakecond, NULL);
+
+ ret = br_launch_scrubber (this, child, fsscan, fsscrub);
+ if (ret)
+ goto error_return;
+
+ return 0;
+
error_return:
LOCK_DESTROY (&fsscan->entrylock);
pthread_mutex_destroy (&fsscan->waitlock);
@@ -1223,6 +1251,7 @@ br_child_enaction (xlator_t *this, br_child_t *child, br_stub_init_t *stub)
ret = br_enact_signer (this, child, stub);
if (!ret) {
+ child->witnessed = 1;
_br_set_child_state (child, BR_CHILD_STATE_CONNECTED);
gf_log (this->name, GF_LOG_INFO,
"Connected to brick %s..", child->brick_path);
@@ -1304,6 +1333,100 @@ br_brick_connect (xlator_t *this, br_child_t *child)
return ret;
}
+/* TODO: cleanup signer */
+static int32_t
+br_cleanup_signer (xlator_t *this, br_child_t *child)
+{
+ return 0;
+}
+
+static int32_t
+br_cleanup_scrubber (xlator_t *this, br_child_t *child)
+{
+ int32_t ret = 0;
+ br_private_t *priv = NULL;
+ struct br_scanfs *fsscan = NULL;
+ struct br_scrubber *fsscrub = NULL;
+
+ priv = this->private;
+ fsscan = &child->fsscan;
+ fsscrub = &priv->fsscrub;
+
+ /**
+ * 0x0: child (brick) goes out of rotation
+ *
+ * This is fully safe w.r.t. entries for this child being actively
+ * scrubbed. Each of the scrubber thread(s) would finish scrubbing
+ * the entry (probably failing due to disconnection) and either
+ * putting the entry back into the queue or continuing further.
+ * Either way, pending entries for this child's queue need not be
+ * drained; entries just sit there in the queued/ready list to be
+ * consumed later upon re-connection.
+ */
+ pthread_mutex_lock (&fsscrub->mutex);
+ {
+ list_del_init (&child->list);
+ }
+ pthread_mutex_unlock (&fsscrub->mutex);
+
+ /**
+ * 0x1: cleanup scanner thread
+ *
+ * The pending timer needs to be removed _after_ cleaning up the
+ * filesystem scanner (scheduling the next scrub time is not a
+ * cancellation point).
+ */
+ ret = gf_thread_cleanup_xint (child->thread);
+ if (ret)
+ gf_log (this->name, GF_LOG_ERROR,
+ "Error cleaning up scanner thread");
+
+ /**
+ * 0x2: free()up resources
+ */
+ if (fsscan->timer) {
+ (void) gf_tw_del_timer (priv->timer_wheel, fsscan->timer);
+
+ GF_FREE (fsscan->timer);
+ fsscan->timer = NULL;
+ }
+
+ gf_log (this->name, GF_LOG_INFO,
+ "Cleaned up scrubber for brick [%s]", child->brick_path);
+
+ return 0;
+}
+
+/**
+ * OK.. this child has made it's mind to go down the drain. So,
+ * let's clean up what it touched. (NOTE: there's no need to clean
+ * the inode table, it's just reused taking care of stale inodes)
+ */
+int32_t
+br_brick_disconnect (xlator_t *this, br_child_t *child)
+{
+ int32_t ret = 0;
+ br_private_t *priv = this->private;
+
+ LOCK (&child->lock);
+ {
+ if (!_br_is_child_connected (child))
+ goto unblock;
+
+ /* child is on death row.. */
+ _br_set_child_state (child, BR_CHILD_STATE_DISCONNECTED);
+
+ if (priv->iamscrubber)
+ ret = br_cleanup_scrubber (this, child);
+ else
+ ret = br_cleanup_signer (this, child);
+ }
+ unblock:
+ UNLOCK (&child->lock);
+
+ return ret;
+}
+
/**
* This function is executed in a separate thread. The thread gets the
* brick from where CHILD_UP has received from the queue and gets the
@@ -1424,7 +1547,7 @@ notify (xlator_t *this, int32_t event, void *data, ...)
{
child = &priv->children[idx];
if (child->child_up == 1)
- goto unblock;
+ goto unblock_0;
priv->up_children++;
child->child_up = 1;
@@ -1435,7 +1558,7 @@ notify (xlator_t *this, int32_t event, void *data, ...)
_br_qchild_event (this, child, br_brick_connect);
pthread_cond_signal (&priv->cond);
}
- unblock:
+ unblock_0:
pthread_mutex_unlock (&priv->lock);
if (priv->up_children == priv->child_count)
@@ -1452,11 +1575,17 @@ notify (xlator_t *this, int32_t event, void *data, ...)
pthread_mutex_lock (&priv->lock);
{
- if (priv->children[idx].child_up == 1) {
- priv->children[idx].child_up = 0;
- priv->up_children--;
- }
+ child = &priv->children[idx];
+ if (child->child_up == 0)
+ goto unblock_1;
+
+ child->child_up = 0;
+ priv->up_children--;
+
+ _br_qchild_event (this, child, br_brick_disconnect);
+ pthread_cond_signal (&priv->cond);
}
+ unblock_1:
pthread_mutex_unlock (&priv->lock);
if (priv->up_children == 0)
@@ -1649,6 +1778,7 @@ br_init_children (xlator_t *this, br_private_t *priv)
child = &priv->children[i];
LOCK_INIT (&child->lock);
+ child->witnessed = 0;
br_set_child_state (child, BR_CHILD_STATE_DISCONNECTED);
child->this = this;
diff --git a/xlators/features/bit-rot/src/bitd/bit-rot.h b/xlators/features/bit-rot/src/bitd/bit-rot.h
index b8d9e3b..9a55773 100644
--- a/xlators/features/bit-rot/src/bitd/bit-rot.h
+++ b/xlators/features/bit-rot/src/bitd/bit-rot.h
@@ -85,8 +85,10 @@ typedef enum br_child_state {
} br_child_state_t;
struct br_child {
- gf_lock_t lock;
- br_child_state_t c_state;
+ gf_lock_t lock; /* protects child state */
+ char witnessed; /* witnessed at least one succesfull
+ connection */
+ br_child_state_t c_state; /* current state of this child */
char child_up; /* Indicates whether this child is
up or not */
@@ -236,4 +238,10 @@ _br_child_failed_conn (br_child_t *child)
return (child->c_state == BR_CHILD_STATE_CONNFAILED);
}
+static inline int
+_br_child_witnessed_connection (br_child_t *child)
+{
+ return (child->witnessed == 1);
+}
+
#endif /* __BIT_ROT_H__ */
diff --git a/xlators/features/changelog/src/changelog-helpers.c b/xlators/features/changelog/src/changelog-helpers.c
index 2c0a645..144bf54 100644
--- a/xlators/features/changelog/src/changelog-helpers.c
+++ b/xlators/features/changelog/src/changelog-helpers.c
@@ -27,28 +27,6 @@
#include "changelog-rpc-common.h"
#include <pthread.h>
-static inline void
-__mask_cancellation (xlator_t *this)
-{
- int ret = 0;
-
- ret = pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL);
- if (ret)
- gf_log (this->name, GF_LOG_WARNING,
- "failed to disable thread cancellation");
-}
-
-static inline void
-__unmask_cancellation (xlator_t *this)
-{
- int ret = 0;
-
- ret = pthread_setcancelstate (PTHREAD_CANCEL_ENABLE, NULL);
- if (ret)
- gf_log (this->name, GF_LOG_WARNING,
- "failed to enable thread cancellation");
-}
-
static void
changelog_cleanup_free_mutex (void *arg_mutex)
{
@@ -1338,7 +1316,7 @@ changelog_rollover (void *data)
continue;
}
- __mask_cancellation (this);
+ _mask_cancellation ();
LOCK (&priv->lock);
{
@@ -1348,7 +1326,7 @@ changelog_rollover (void *data)
}
UNLOCK (&priv->lock);
- __unmask_cancellation (this);
+ _unmask_cancellation ();
}
return NULL;
@@ -1376,14 +1354,14 @@ changelog_fsync_thread (void *data)
if (ret)
continue;
- __mask_cancellation (this);
+ _mask_cancellation ();
ret = changelog_inject_single_event (this, priv, &cld);
if (ret)
gf_log (this->name, GF_LOG_ERROR,
"failed to inject fsync event");
- __unmask_cancellation (this);
+ _unmask_cancellation ();
}
return NULL;
--
1.7.1