summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorShyamsundarR <srangana@redhat.com>2018-07-05 14:44:04 -0400
committerShyamsundar Ranganathan <srangana@redhat.com>2018-07-09 15:07:56 +0000
commitc30283fd2786dc24b6adf5aff8d39e24730b1a11 (patch)
treec7f0761fe58c252092baea2509a934c1f6dde700
parent16f02433df9e37e28783309060f469d2e50024ee (diff)
io-stats: Terminate dump thread when dump interval is set to 0
_ios_dump_thread is not terminated by the function _ios_destroy_dump_thread when the diagnostic interval is set to 0 (which means disable auto dumping). During reconfigure, if the value changes from 0 to another then the thread is started, but on reconfiguring this to 0 the thread is not being terminated. Further, if the value is changed from 0 to X to 0 to Y, where X and Y are 2 arbitrary duration numbers, the reconfigure code ends up starting one more thread (for each change from 0 to a valid interval). This patch fixes the same by terminating the thread when the value changes from non-zero to 0. NOTE: It would seem nicer to use conf->dump_thread and check its value for thread presence etc. but there is no documented invalid value for the same, and hence an invalid check is not feasible, thus introducing a new running bool to determine the same. Fixes: bz#1598548 Change-Id: I3e7d2ce8f033879542932ac730d57bfcaf14af73 Signed-off-by: ShyamsundarR <srangana@redhat.com>
-rwxr-xr-xtests/bugs/io-stats/bug-1598548.t41
-rw-r--r--xlators/debug/io-stats/src/io-stats.c12
2 files changed, 52 insertions, 1 deletions
diff --git a/tests/bugs/io-stats/bug-1598548.t b/tests/bugs/io-stats/bug-1598548.t
new file mode 100755
index 00000000000..19b0c053d08
--- /dev/null
+++ b/tests/bugs/io-stats/bug-1598548.t
@@ -0,0 +1,41 @@
+#!/bin/bash
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+. $(dirname $0)/../../nfs.rc
+
+checkdumpthread () {
+ local brick_pid=$(get_brick_pid $1 $2 $3)
+ local thread_count=$(gstack $brick_pid | grep -c _ios_dump_thread)
+ echo $thread_count
+}
+
+cleanup;
+
+TEST glusterd
+TEST pidof glusterd
+TEST $CLI volume create $V0 $H0:$B0/${V0}0
+TEST $CLI volume start $V0
+
+TEST $CLI volume profile $V0 start
+EXPECT 0 checkdumpthread $V0 $H0 $B0/${V0}0
+
+TEST $CLI volume set $V0 diagnostics.stats-dump-interval 3
+EXPECT 1 checkdumpthread $V0 $H0 $B0/${V0}0
+
+TEST $CLI volume set $V0 diagnostics.stats-dump-interval 10
+EXPECT 1 checkdumpthread $V0 $H0 $B0/${V0}0
+
+TEST $CLI volume set $V0 diagnostics.stats-dump-interval 0
+EXPECT 0 checkdumpthread $V0 $H0 $B0/${V0}0
+
+TEST $CLI volume set $V0 diagnostics.stats-dump-interval 7
+EXPECT 1 checkdumpthread $V0 $H0 $B0/${V0}0
+
+TEST $CLI volume set $V0 diagnostics.stats-dump-interval 0
+EXPECT 0 checkdumpthread $V0 $H0 $B0/${V0}0
+
+TEST $CLI volume set $V0 diagnostics.stats-dump-interval 11
+EXPECT 1 checkdumpthread $V0 $H0 $B0/${V0}0
+
+cleanup;
diff --git a/xlators/debug/io-stats/src/io-stats.c b/xlators/debug/io-stats/src/io-stats.c
index f5d86e1bc6a..28e6e362b6b 100644
--- a/xlators/debug/io-stats/src/io-stats.c
+++ b/xlators/debug/io-stats/src/io-stats.c
@@ -153,6 +153,7 @@ struct ios_conf {
int32_t ios_dump_interval;
pthread_t dump_thread;
gf_boolean_t dump_thread_should_die;
+ gf_boolean_t dump_thread_running;
gf_lock_t ios_sampling_lock;
int32_t ios_sample_interval;
int32_t ios_sample_buf_size;
@@ -3129,7 +3130,7 @@ conditional_dump (dict_t *dict, char *key, data_t *value, void *data)
int
_ios_destroy_dump_thread (struct ios_conf *conf) {
conf->dump_thread_should_die = _gf_true;
- if (conf->ios_dump_interval > 0) {
+ if (conf->dump_thread_running) {
(void) pthread_cancel (conf->dump_thread);
(void) pthread_join (conf->dump_thread, NULL);
}
@@ -3248,6 +3249,7 @@ _ios_dump_thread (xlator_t *this) {
}
}
out:
+ conf->dump_thread_running = _gf_false;
gf_log (this->name, GF_LOG_INFO, "IO stats dump thread terminated");
return NULL;
}
@@ -3883,14 +3885,19 @@ reconfigure (xlator_t *this, dict_t *options)
GF_OPTION_RECONF ("ios-dump-interval", conf->ios_dump_interval, options,
int32, out);
if ((old_dump_interval <= 0) && (conf->ios_dump_interval > 0)) {
+ conf->dump_thread_running = _gf_true;
+ conf->dump_thread_should_die = _gf_false;
ret = gf_thread_create (&conf->dump_thread, NULL,
(void *) &_ios_dump_thread, this, "iosdump");
if (ret) {
+ conf->dump_thread_running = _gf_false;
gf_log (this ? this->name : "io-stats",
GF_LOG_ERROR, "Failed to start thread"
"while reconfigure. Returning %d", ret);
goto out;
}
+ } else if ((old_dump_interval > 0) && (conf->ios_dump_interval == 0)) {
+ _ios_destroy_dump_thread (conf);
}
GF_OPTION_RECONF ("ios-sample-interval", conf->ios_sample_interval,
@@ -4114,10 +4121,13 @@ init (xlator_t *this)
this->private = conf;
if (conf->ios_dump_interval > 0) {
+ conf->dump_thread_running = _gf_true;
+ conf->dump_thread_should_die = _gf_false;
ret = gf_thread_create (&conf->dump_thread, NULL,
(void *) &_ios_dump_thread,
this, "iosdump");
if (ret) {
+ conf->dump_thread_running = _gf_false;
gf_log (this ? this->name : "io-stats",
GF_LOG_ERROR, "Failed to start thread"
"in init. Returning %d", ret);