summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorShyam <srangana@redhat.com>2016-05-27 14:00:40 -0400
committerJeff Darcy <jdarcy@redhat.com>2016-06-15 10:38:00 -0700
commitc04df79dc453ef5cb7b3a0ca8ba14598da6189ac (patch)
treede0bd18610edade1302b64f73e694a92d4035b0f
parent3b69e25c0f3cf0b958fbd2b8459712ad21239caa (diff)
io-stats: Fix io-stat dump to dump at all levels
Previous commit to fix the bug, where io-stat-dump was overwriting the dump file when the client and a brick was on the same host, failed to consider the existing behaviour where io-stats can help generate closely correlated set of stats across clients and bricks, by triggering the dump using the same command. This was introduced in commit: 0facb11220aea20a6573b656785922219c9650cf Further, by limiting the first io-stat to unwind the dump request, there is no way to trigger other io-stat xlators in the stack to dump their stat information. This bug hence is being fixed by this commit keeping the following in mind, - We need to trigger io-stat-dump for all instances in the graph when this attr is set - We need to write the output to different files, so that they do not overwrite each others data - We need to prevent this xattr from being set on the path that is used to trigger the io-stat-dump information Change-Id: I31ec380f0d85e10313a9d7b977da0e1ec74638a6 BUG: 1322825 Signed-off-by: Shyam <srangana@redhat.com> Reviewed-on: http://review.gluster.org/14552 Smoke: Gluster Build System <jenkins@build.gluster.org> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org> Reviewed-by: Ravishankar N <ravishankar@redhat.com> CentOS-regression: Gluster Build System <jenkins@build.gluster.org> Reviewed-by: Jeff Darcy <jdarcy@redhat.com>
-rw-r--r--libglusterfs/src/glusterfs.h1
-rwxr-xr-xtests/bugs/core/io-stats-1322825.t61
-rw-r--r--xlators/debug/io-stats/src/io-stats.c35
-rw-r--r--xlators/storage/posix/src/posix.c2
4 files changed, 81 insertions, 18 deletions
diff --git a/libglusterfs/src/glusterfs.h b/libglusterfs/src/glusterfs.h
index 128dcd32e5f..1accb62d9e9 100644
--- a/libglusterfs/src/glusterfs.h
+++ b/libglusterfs/src/glusterfs.h
@@ -90,6 +90,7 @@
#define GF_XATTR_GET_REAL_FILENAME_KEY "glusterfs.get_real_filename:"
#define GF_XATTR_USER_PATHINFO_KEY "glusterfs.pathinfo"
#define GF_INTERNAL_IGNORE_DEEM_STATFS "ignore-deem-statfs"
+#define GF_XATTR_IOSTATS_DUMP_KEY "trusted.io-stats-dump"
#define GF_READDIR_SKIP_DIRS "readdir-filter-directories"
#define GF_MDC_LOADED_KEY_NAMES "glusterfs.mdc.loaded.key.names"
diff --git a/tests/bugs/core/io-stats-1322825.t b/tests/bugs/core/io-stats-1322825.t
index 924251a432c..d232ecb2420 100755
--- a/tests/bugs/core/io-stats-1322825.t
+++ b/tests/bugs/core/io-stats-1322825.t
@@ -1,16 +1,67 @@
#!/bin/bash
+# Test details:
+# This is to test that the io-stat-dump xattr is not set on the brick,
+# against the path that is used to trigger the stats dump.
+# Additionally it also tests if as many io-stat dumps are generated as there
+# are io-stat xlators in the graphs, which is 2 by default
+
. $(dirname $0)/../../include.rc
cleanup;
TEST glusterd
TEST pidof glusterd
-TEST $CLI volume create $V0 $H0:$B0/${V0} $H0:$B1/${V0}
+
+# Covering replication and distribution in the test
+TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}{1..4}
TEST $CLI volume start $V0
-TEST glusterfs --volfile-id=/$V0 --volfile-server=$H0 $M0 --attribute-timeout=0 --entry-timeout=0
-TEST "echo hello > $M0/file1"
-TEST setfattr -n trusted.io-stats-dump -v /tmp/io-stats.log $M0
-TEST ! getfattr -n trusted.io-stats-dump $B0/${V0}
+TEST $GFS -s $H0 --volfile-id $V0 $M0
+
+# Generate some activity for the stats to produce something useful
+TEST $CLI volume profile $V0 start
+TEST mkdir $M0/dir1
+
+# Generate the stat dump across the io-stat instances
+TEST setfattr -n trusted.io-stats-dump -v /tmp/io-stats-1322825 $M0
+
+# Check if $M0 is clean w.r.t xattr information
+# TODO: if there are better ways to check we really get no attr error, please
+# correct the following.
+getfattr -n trusted.io-stats-dump $B0/${V0}1 2>&1 | grep -qi "no such attribute"
+ret=$(echo $?)
+EXPECT 0 echo $ret
+getfattr -n trusted.io-stats-dump $B0/${V0}2 2>&1 | grep -qi "no such attribute"
+ret=$(echo $?)
+EXPECT 0 echo $ret
+getfattr -n trusted.io-stats-dump $B0/${V0}3 2>&1 | grep -qi "no such attribute"
+ret=$(echo $?)
+EXPECT 0 echo $ret
+getfattr -n trusted.io-stats-dump $B0/${V0}4 2>&1 | grep -qi "no such attribute"
+ret=$(echo $?)
+EXPECT 0 echo $ret
+
+# Check if we have 5 io-stat files in /tmp
+EXPECT 5 ls -1 /tmp/io-stats-1322825*
+# Cleanup the 5 generated files
+rm -f /tmp/io-stats-1322825*
+
+# Rinse and repeat above for a directory
+TEST setfattr -n trusted.io-stats-dump -v /tmp/io-stats-1322825 $M0/dir1
+getfattr -n trusted.io-stats-dump $B0/${V0}1/dir1 2>&1 | grep -qi "no such attribute"
+ret=$(echo $?)
+EXPECT 0 echo $ret
+getfattr -n trusted.io-stats-dump $B0/${V0}2/dir1 2>&1 | grep -qi "no such attribute"
+ret=$(echo $?)
+EXPECT 0 echo $ret
+getfattr -n trusted.io-stats-dump $B0/${V0}3/dir1 2>&1 | grep -qi "no such attribute"
+ret=$(echo $?)
+EXPECT 0 echo $ret
+getfattr -n trusted.io-stats-dump $B0/${V0}4/dir1 2>&1 | grep -qi "no such attribute"
+ret=$(echo $?)
+EXPECT 0 echo $ret
+
+EXPECT 5 ls -1 /tmp/io-stats-1322825*
+rm -f /tmp/io-stats-1322825*
cleanup;
diff --git a/xlators/debug/io-stats/src/io-stats.c b/xlators/debug/io-stats/src/io-stats.c
index 62e85bbb99b..5e82fb4c029 100644
--- a/xlators/debug/io-stats/src/io-stats.c
+++ b/xlators/debug/io-stats/src/io-stats.c
@@ -21,7 +21,8 @@
* d) counts of write IO block size - since process start, last interval and per fd
* e) counts of all FOP types passing through it
*
- * Usage: setfattr -n io-stats-dump /tmp/filename /mnt/gluster
+ * Usage: setfattr -n trusted.io-stats-dump /tmp/filename /mnt/gluster
+ * output is written to /tmp/filename.<iostats xlator instance name>
*
*/
@@ -2827,15 +2828,32 @@ conditional_dump (dict_t *dict, char *key, data_t *value, void *data)
char *filename = NULL;
FILE *logfp = NULL;
struct ios_dump_args args = {0};
- int pid;
+ int pid, namelen;
char dump_key[100];
+ char *slash_ptr = NULL;
stub = data;
this = stub->this;
- filename = alloca (value->len + 1);
- memset (filename, 0, value->len + 1);
+ /* Create a file name that is appended with the io-stats instance
+ name as well. This helps when there is more than a single io-stats
+ instance in the graph, or the client and server processes are running
+ on the same node */
+ /* hmmm... no check for this */
+ /* name format: <passed in path/filename>.<xlator name slashes to -> */
+ namelen = value->len + strlen (this->name) + 2; /* '.' and '\0' */
+ filename = alloca (namelen);
+ memset (filename, 0, namelen);
memcpy (filename, data_to_str (value), value->len);
+ memcpy (filename + value->len, ".", 1);
+ memcpy (filename + value->len + 1, this->name, strlen(this->name));
+
+ /* convert any slashes to '-' so that fopen works correctly */
+ slash_ptr = strchr (filename + value->len + 1, '/');
+ while (slash_ptr) {
+ *slash_ptr = '-';
+ slash_ptr = strchr (slash_ptr, '/');
+ }
pid = getpid ();
@@ -3018,11 +3036,6 @@ io_stats_setxattr (call_frame_t *frame, xlator_t *this,
ret = dict_foreach_match (dict, match_special_xattr, NULL,
conditional_dump, &stub);
- if (ret > 0) {
- /* Setxattr was on key 'io-stat-dump', hence dump and unwind
- * from here */
- goto out;
- }
START_FOP_LATENCY (frame);
@@ -3031,10 +3044,6 @@ io_stats_setxattr (call_frame_t *frame, xlator_t *this,
FIRST_CHILD(this)->fops->setxattr,
loc, dict, flags, xdata);
return 0;
-
-out:
- STACK_UNWIND_STRICT (setxattr, frame, 0, 0, NULL);
- return 0;
}
diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c
index 2320bf13449..4294fa4fe36 100644
--- a/xlators/storage/posix/src/posix.c
+++ b/xlators/storage/posix/src/posix.c
@@ -3746,6 +3746,8 @@ posix_setxattr (call_frame_t *frame, xlator_t *this,
dict_del (dict, GFID_XATTR_KEY);
dict_del (dict, GF_XATTR_VOL_ID_KEY);
+ /* the io-stats-dump key should not reach disk */
+ dict_del (dict, GF_XATTR_IOSTATS_DUMP_KEY);
filler.real_path = real_path;
filler.this = this;