* [PATCH 0/2] Add fio performance tracking to fstests
@ 2017-10-10 17:21 Josef Bacik
2017-10-10 17:21 ` [PATCH 1/2] fstests: add fio perf results support Josef Bacik
2017-10-10 17:21 ` [PATCH 2/2] perf/001: a random write buffered fio perf test Josef Bacik
0 siblings, 2 replies; 6+ messages in thread
From: Josef Bacik @ 2017-10-10 17:21 UTC (permalink / raw)
To: kernel-team, fstests, david, tytso
Everybody agrees that a new project for performance testing is not super useful,
and that it would be better to integrate it into something we all already run.
These two patches take what was fsperf and puts it into fstests proper. This
provides better flexibility for test environments, and at least gets us started
towards adding more of these sort of tests into our day to day activities.
The only thing extra to run these tests is the PERF_CONFIGNAME option that must
be specified in order for the perf tests to be run. We need this (or something
like it) to make sure that we compare runs that have similar setups so that we
don't end up failing because we did one run with NVME but the next run with
SATA.
I've only added one of the fio tests from fsperf because I want to make sure
we're all happy with the generic support that I've added. Once these are merged
I'll send the next tests along and rework how we do comparisons so we're
comparing averages of previous runs instead of just whatever the last run was.
Thanks,
Josef
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] fstests: add fio perf results support
2017-10-10 17:21 [PATCH 0/2] Add fio performance tracking to fstests Josef Bacik
@ 2017-10-10 17:21 ` Josef Bacik
2017-10-10 17:53 ` Darrick J. Wong
2017-10-10 17:21 ` [PATCH 2/2] perf/001: a random write buffered fio perf test Josef Bacik
1 sibling, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2017-10-10 17:21 UTC (permalink / raw)
To: kernel-team, fstests, david, tytso; +Cc: Josef Bacik
From: Josef Bacik <jbacik@fb.com>
This patch does the nuts and bolts of grabbing fio results and storing
them in a database in order to check against for future runs. This
works by storing the results in resuts/fio-results.db as a sqlite
database. The src/perf directory has all the supporting python code for
parsing the fio json results, storing it in the database, and loading
previous results from the database to compare with the current results.
This also adds a PERF_CONFIGNAME option that must be set for this to
work. Since we all have various ways we run fstests it doesn't make
sense to compare different configurations with each other (unless
specifically desired). The PERF_CONFIGNAME will allow us to separate
out results for different test run configurations to make sure we're
comparing results correctly.
Currently we only check against the last perf result. In the future I
will flesh this out to compare against the average of N number of runs
to be a little more complete, and hopefully that will allow us to also
watch latencies as well.
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
.gitignore | 1 +
common/config | 2 +
common/rc | 32 +++++++++++
src/perf/FioCompare.py | 106 +++++++++++++++++++++++++++++++++++++
src/perf/FioResultDecoder.py | 58 ++++++++++++++++++++
src/perf/ResultData.py | 43 +++++++++++++++
src/perf/fio-insert-and-compare.py | 32 +++++++++++
src/perf/fio-results.sql | 93 ++++++++++++++++++++++++++++++++
src/perf/generate-schema.py | 49 +++++++++++++++++
9 files changed, 416 insertions(+)
create mode 100644 src/perf/FioCompare.py
create mode 100644 src/perf/FioResultDecoder.py
create mode 100644 src/perf/ResultData.py
create mode 100644 src/perf/fio-insert-and-compare.py
create mode 100644 src/perf/fio-results.sql
create mode 100644 src/perf/generate-schema.py
diff --git a/.gitignore b/.gitignore
index ae7ef87ab384..986a6f7ff0ad 100644
--- a/.gitignore
+++ b/.gitignore
@@ -156,6 +156,7 @@
/src/aio-dio-regress/aiocp
/src/aio-dio-regress/aiodio_sparse2
/src/log-writes/replay-log
+/src/perf/*.pyc
# dmapi/ binaries
/dmapi/src/common/cmd/read_invis
diff --git a/common/config b/common/config
index 71798f0adb1e..d2b2e2cda688 100644
--- a/common/config
+++ b/common/config
@@ -195,6 +195,8 @@ export MAN_PROG="`set_prog_path man`"
export NFS4_SETFACL_PROG="`set_prog_path nfs4_setfacl`"
export NFS4_GETFACL_PROG="`set_prog_path nfs4_getfacl`"
export UBIUPDATEVOL_PROG="`set_prog_path ubiupdatevol`"
+export PYTHON_PROG="`set_prog_path python`"
+export SQLITE3_PROG="`set_prog_path sqlite3`"
# use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
# newer systems have udevadm command but older systems like RHEL5 don't.
diff --git a/common/rc b/common/rc
index 53bbb1187f81..2660ad51ed26 100644
--- a/common/rc
+++ b/common/rc
@@ -2997,6 +2997,38 @@ _require_fio()
[ $? -eq 0 ] || _notrun "$FIO_PROG too old, see $seqres.full"
}
+_fio_results_init()
+{
+ if [ -z "$PERF_CONFIGNAME" ]
+ then
+ _notrun "this test requires \$PERF_CONFIGNAME to be set"
+ fi
+ _require_command $PYTHON_PROG python
+
+ $PYTHON_PROG -c "import sqlite3" >/dev/null 2>&1
+ [ $? -ne 0 ] && _notrun "this test requires python sqlite support"
+
+ $PYTHON_PROG -c "import json" >/dev/null 2>&1
+ [ $? -ne 0 ] && _notrun "this test requires python json support"
+
+ _require_command $SQLITE3_PROG sqlite3
+ cat $here/src/perf/fio-results.sql | \
+ $SQLITE3_PROG $RESULT_BASE/fio-results.db
+ [ $? -ne 0 ] && _notrun "failed to create results database"
+ [ ! -e $RESULT_BASE/fio-results.db ] && \
+ _notrun "failed to create results database"
+}
+
+_fio_results_compare()
+{
+ _testname=$1
+ _resultfile=$2
+
+ run_check $PYTHON_PROG $here/src/perf/fio-insert-and-compare.py \
+ -c $PERF_CONFIGNAME -d $RESULT_BASE/fio-results.db \
+ -n $_testname $_resultfile
+}
+
# Does freeze work on this fs?
_require_freeze()
{
diff --git a/src/perf/FioCompare.py b/src/perf/FioCompare.py
new file mode 100644
index 000000000000..55d13699c34c
--- /dev/null
+++ b/src/perf/FioCompare.py
@@ -0,0 +1,106 @@
+default_keys = [ 'iops', 'io_kbytes', 'bw' ]
+latency_keys = [ 'lat_ns_min', 'lat_ns_max' ]
+main_job_keys = [ 'sys_cpu', 'elapsed' ]
+io_ops = ['read', 'write', 'trim' ]
+
+def _fuzzy_compare(a, b, fuzzy):
+ if a == b:
+ return 0
+ if a == 0:
+ return 100
+ a = float(a)
+ b = float(b)
+ fuzzy = float(fuzzy)
+ val = ((b - a) / a) * 100
+ if val > fuzzy or val < -fuzzy:
+ return val;
+ return 0
+
+def _compare_jobs(ijob, njob, latency, fuzz):
+ failed = 0
+ for k in default_keys:
+ for io in io_ops:
+ key = "{}_{}".format(io, k)
+ comp = _fuzzy_compare(ijob[key], njob[key], fuzz)
+ if comp < 0:
+ print(" {} regressed: old {} new {} {}%".format(key,
+ ijob[key], njob[key], comp))
+ failed += 1
+ elif comp > 0:
+ print(" {} improved: old {} new {} {}%".format(key,
+ ijob[key], njob[key], comp))
+ for k in latency_keys:
+ if not latency:
+ break
+ for io in io_ops:
+ key = "{}_{}".format(io, k)
+ comp = _fuzzy_compare(ijob[key], njob[key], fuzz)
+ if comp > 0:
+ print(" {} regressed: old {} new {} {}%".format(key,
+ ijob[key], njob[key], comp))
+ failed += 1
+ elif comp < 0:
+ print(" {} improved: old {} new {} {}%".format(key,
+ ijob[key], njob[key], comp))
+ for k in main_job_keys:
+ comp = _fuzzy_compare(ijob[k], njob[k], fuzz)
+ if comp > 0:
+ print(" {} regressed: old {} new {} {}%".format(k, ijob[k],
+ njob[k], comp))
+ failed += 1
+ elif comp < 0:
+ print(" {} improved: old {} new {} {}%".format(k, ijob[k],
+ njob[k], comp))
+ return failed
+
+def compare_individual_jobs(initial, data, fuzz):
+ failed = 0;
+ initial_jobs = initial['jobs'][:]
+ for njob in data['jobs']:
+ for ijob in initial_jobs:
+ if njob['jobname'] == ijob['jobname']:
+ print(" Checking results for {}".format(njob['jobname']))
+ failed += _compare_jobs(ijob, njob, fuzz)
+ initial_jobs.remove(ijob)
+ break
+ return failed
+
+def default_merge(data):
+ '''Default merge function for multiple jobs in one run
+
+ For runs that include multiple threads we will have a lot of variation
+ between the different threads, which makes comparing them to eachother
+ across multiple runs less that useful. Instead merge the jobs into a single
+ job. This function does that by adding up 'iops', 'io_kbytes', and 'bw' for
+ read/write/trim in the merged job, and then taking the maximal values of the
+ latency numbers.
+ '''
+ merge_job = {}
+ for job in data['jobs']:
+ for k in main_job_keys:
+ if k not in merge_job:
+ merge_job[k] = job[k]
+ else:
+ merge_job[k] += job[k]
+ for io in io_ops:
+ for k in default_keys:
+ key = "{}_{}".format(io, k)
+ if key not in merge_job:
+ merge_job[key] = job[key]
+ else:
+ merge_job[key] += job[key]
+ for k in latency_keys:
+ key = "{}_{}".format(io, k)
+ if key not in merge_job:
+ merge_job[key] = job[key]
+ elif merge_job[key] < job[key]:
+ merge_job[key] = job[key]
+ return merge_job
+
+def compare_fiodata(initial, data, latency, merge_func=default_merge, fuzz=5):
+ failed = 0
+ if merge_func is None:
+ return compare_individual_jobs(initial, data, fuzz)
+ ijob = merge_func(initial)
+ njob = merge_func(data)
+ return _compare_jobs(ijob, njob, latency, fuzz)
diff --git a/src/perf/FioResultDecoder.py b/src/perf/FioResultDecoder.py
new file mode 100644
index 000000000000..51efae308add
--- /dev/null
+++ b/src/perf/FioResultDecoder.py
@@ -0,0 +1,58 @@
+import json
+
+class FioResultDecoder(json.JSONDecoder):
+ """Decoder for decoding fio result json to an object for our database
+
+ This decodes the json output from fio into an object that can be directly
+ inserted into our database. This just strips out the fields we don't care
+ about and collapses the read/write/trim classes into a flat value structure
+ inside of the jobs object.
+
+ For example
+ "write" : {
+ "io_bytes" : 313360384,
+ "bw" : 1016,
+ }
+
+ Get's collapsed to
+
+ "write_io_bytes" : 313360384,
+ "write_bw": 1016,
+
+ Currently any dict under 'jobs' get's dropped, with the exception of 'read',
+ 'write', and 'trim'. For those sub sections we drop any dict's under those.
+
+ Attempt to keep this as generic as possible, we don't want to break every
+ time fio changes it's json output format.
+ """
+ _ignore_types = ['dict', 'list']
+ _override_keys = ['lat_ns']
+ _io_ops = ['read', 'write', 'trim']
+
+ def decode(self, json_string):
+ """This does the dirty work of converting everything"""
+ default_obj = super(FioResultDecoder, self).decode(json_string)
+ obj = {}
+ obj['global'] = {}
+ obj['global']['time'] = default_obj['time']
+ obj['jobs'] = []
+ for job in default_obj['jobs']:
+ new_job = {}
+ for key,value in job.iteritems():
+ if key not in self._io_ops:
+ if value.__class__.__name__ in self._ignore_types:
+ continue
+ new_job[key] = value
+ continue
+ for k,v in value.iteritems():
+ if k in self._override_keys:
+ for subk,subv in v.iteritems():
+ collapsed_key = "{}_{}_{}".format(key, k, subk)
+ new_job[collapsed_key] = subv
+ continue
+ if v.__class__.__name__ in self._ignore_types:
+ continue
+ collapsed_key = "{}_{}".format(key, k)
+ new_job[collapsed_key] = v
+ obj['jobs'].append(new_job)
+ return obj
diff --git a/src/perf/ResultData.py b/src/perf/ResultData.py
new file mode 100644
index 000000000000..f0c7eace6dad
--- /dev/null
+++ b/src/perf/ResultData.py
@@ -0,0 +1,43 @@
+import sqlite3
+
+def _dict_factory(cursor, row):
+ d = {}
+ for idx,col in enumerate(cursor.description):
+ d[col[0]] = row[idx]
+ return d
+
+class ResultData:
+ def __init__(self, filename):
+ self.db = sqlite3.connect(filename)
+ self.db.row_factory = _dict_factory
+
+ def load_last(self, testname, config):
+ d = {}
+ cur = self.db.cursor()
+ cur.execute("SELECT * FROM fio_runs WHERE config = ? AND name = ?ORDER BY time DESC LIMIT 1",
+ (config,testname))
+ d['global'] = cur.fetchone()
+ if d['global'] is None:
+ return None
+ cur.execute("SELECT * FROM fio_jobs WHERE run_id = ?",
+ (d['global']['id'],))
+ d['jobs'] = cur.fetchall()
+ return d
+
+ def _insert_obj(self, tablename, obj):
+ keys = obj.keys()
+ values = obj.values()
+ cur = self.db.cursor()
+ cmd = "INSERT INTO {} ({}) VALUES ({}".format(tablename,
+ ",".join(keys),
+ '?,' * len(values))
+ cmd = cmd[:-1] + ')'
+ cur.execute(cmd, tuple(values))
+ self.db.commit()
+ return cur.lastrowid
+
+ def insert_result(self, result):
+ row_id = self._insert_obj('fio_runs', result['global'])
+ for job in result['jobs']:
+ job['run_id'] = row_id
+ self._insert_obj('fio_jobs', job)
diff --git a/src/perf/fio-insert-and-compare.py b/src/perf/fio-insert-and-compare.py
new file mode 100644
index 000000000000..0a7460fcbab7
--- /dev/null
+++ b/src/perf/fio-insert-and-compare.py
@@ -0,0 +1,32 @@
+import FioResultDecoder
+import ResultData
+import FioCompare
+import json
+import argparse
+import sys
+import platform
+
+parser = argparse.ArgumentParser()
+parser.add_argument('-c', '--configname', type=str,
+ help="The config name to save the results under.",
+ required=True)
+parser.add_argument('-d', '--db', type=str,
+ help="The db that is being used", required=True)
+parser.add_argument('-n', '--testname', type=str,
+ help="The testname for the result", required=True)
+parser.add_argument('result', type=str,
+ help="The result file to compare and insert")
+args = parser.parse_args()
+
+result_data = ResultData.ResultData(args.db)
+
+json_data = open(args.result)
+data = json.load(json_data, cls=FioResultDecoder.FioResultDecoder)
+data['global']['name'] = args.testname
+data['global']['config'] = args.configname
+data['global']['kernel'] = platform.release()
+result_data.insert_result(data)
+
+compare = result_data.load_last(args.testname, args.configname)
+if FioCompare.compare_fiodata(compare, data, False):
+ sys.exit(1)
diff --git a/src/perf/fio-results.sql b/src/perf/fio-results.sql
new file mode 100644
index 000000000000..b7f6708e1265
--- /dev/null
+++ b/src/perf/fio-results.sql
@@ -0,0 +1,93 @@
+CREATE TABLE IF NOT EXISTS `fio_runs` (
+ `id` INTEGER PRIMARY KEY AUTOINCREMENT,
+ `kernel` datetime NOT NULL,
+ `config` varchar(256) NOT NULL,
+ `name` varchar(256) NOT NULL,
+ `time` datetime NOT NULL
+);
+CREATE TABLE IF NOT EXISTS `fio_jobs` (
+ `run_id` int NOT NULL,
+ `latency_window` int NOT NULL,
+ `trim_lat_ns_mean` float NOT NULL,
+ `read_iops_min` int NOT NULL,
+ `read_bw_dev` float NOT NULL,
+ `trim_runtime` int NOT NULL,
+ `read_io_bytes` int NOT NULL,
+ `read_short_ios` int NOT NULL,
+ `read_iops_samples` int NOT NULL,
+ `minf` int NOT NULL,
+ `read_drop_ios` int NOT NULL,
+ `trim_iops_samples` int NOT NULL,
+ `trim_iops_max` int NOT NULL,
+ `trim_bw_agg` float NOT NULL,
+ `write_bw_min` int NOT NULL,
+ `write_iops_mean` float NOT NULL,
+ `read_bw_max` int NOT NULL,
+ `read_bw_min` int NOT NULL,
+ `trim_bw_dev` float NOT NULL,
+ `read_iops_max` int NOT NULL,
+ `read_total_ios` int NOT NULL,
+ `read_lat_ns_mean` float NOT NULL,
+ `write_iops` float NOT NULL,
+ `latency_target` int NOT NULL,
+ `trim_bw` int NOT NULL,
+ `eta` int NOT NULL,
+ `read_bw_samples` int NOT NULL,
+ `trim_io_kbytes` int NOT NULL,
+ `write_iops_max` int NOT NULL,
+ `write_drop_ios` int NOT NULL,
+ `trim_iops_min` int NOT NULL,
+ `write_bw_samples` int NOT NULL,
+ `read_iops_stddev` float NOT NULL,
+ `write_io_kbytes` int NOT NULL,
+ `trim_bw_mean` float NOT NULL,
+ `write_bw_agg` float NOT NULL,
+ `write_bw_dev` float NOT NULL,
+ `write_lat_ns_stddev` float NOT NULL,
+ `trim_lat_ns_stddev` float NOT NULL,
+ `groupid` int NOT NULL,
+ `latency_depth` int NOT NULL,
+ `trim_short_ios` int NOT NULL,
+ `read_lat_ns_stddev` float NOT NULL,
+ `write_iops_min` int NOT NULL,
+ `write_iops_stddev` float NOT NULL,
+ `read_io_kbytes` int NOT NULL,
+ `trim_bw_samples` int NOT NULL,
+ `trim_lat_ns_min` int NOT NULL,
+ `error` int NOT NULL,
+ `read_bw_mean` float NOT NULL,
+ `trim_iops_mean` float NOT NULL,
+ `elapsed` int NOT NULL,
+ `write_bw_mean` float NOT NULL,
+ `write_short_ios` int NOT NULL,
+ `ctx` int NOT NULL,
+ `write_io_bytes` int NOT NULL,
+ `usr_cpu` float NOT NULL,
+ `trim_drop_ios` int NOT NULL,
+ `write_bw` int NOT NULL,
+ `jobname` varchar(256) NOT NULL,
+ `trim_bw_min` int NOT NULL,
+ `read_runtime` int NOT NULL,
+ `sys_cpu` float NOT NULL,
+ `trim_lat_ns_max` int NOT NULL,
+ `read_iops_mean` float NOT NULL,
+ `write_lat_ns_min` int NOT NULL,
+ `trim_iops_stddev` float NOT NULL,
+ `write_lat_ns_max` int NOT NULL,
+ `majf` int NOT NULL,
+ `write_total_ios` int NOT NULL,
+ `read_bw` int NOT NULL,
+ `read_lat_ns_min` int NOT NULL,
+ `trim_bw_max` int NOT NULL,
+ `write_iops_samples` int NOT NULL,
+ `write_runtime` int NOT NULL,
+ `trim_io_bytes` int NOT NULL,
+ `latency_percentile` float NOT NULL,
+ `read_iops` float NOT NULL,
+ `trim_total_ios` int NOT NULL,
+ `write_lat_ns_mean` float NOT NULL,
+ `write_bw_max` int NOT NULL,
+ `read_bw_agg` float NOT NULL,
+ `read_lat_ns_max` int NOT NULL,
+ `trim_iops` float NOT NULL
+);
diff --git a/src/perf/generate-schema.py b/src/perf/generate-schema.py
new file mode 100644
index 000000000000..91dbdbd41b97
--- /dev/null
+++ b/src/perf/generate-schema.py
@@ -0,0 +1,49 @@
+import json
+import argparse
+import FioResultDecoder
+from dateutil.parser import parse
+
+def is_date(string):
+ try:
+ parse(string)
+ return True
+ except ValueError:
+ return False
+
+def print_schema_def(key, value):
+ typestr = value.__class__.__name__
+ if typestr == 'str' or typestr == 'unicode':
+ if (is_date(value)):
+ typestr = "datetime"
+ else:
+ typestr = "varchar(256)"
+ return ",\n `{}` {} NOT NULL".format(key, typestr)
+
+parser = argparse.ArgumentParser()
+parser.add_argument('infile', help="The json file to strip")
+args = parser.parse_args()
+
+json_data = open(args.infile)
+data = json.load(json_data, cls=FioResultDecoder.FioResultDecoder)
+
+# These get populated by the test runner, not fio, so add them so their
+# definitions get populated in the schema properly
+data['global']['config'] = 'default'
+data['global']['kernel'] = '4.14'
+
+print("CREATE TABLE `fio_runs` (")
+outstr = " `id` int(11) PRIMARY KEY"
+for key,value in data['global'].iteritems():
+ outstr += print_schema_def(key, value)
+print(outstr)
+print(");")
+
+job = data['jobs'][0]
+job['run_id'] = 0
+
+print("CREATE TABLE `fio_jobs` (")
+outstr = " `id` int PRIMARY KEY"
+for key,value in job.iteritems():
+ outstr += print_schema_def(key, value)
+print(outstr)
+print(");")
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] perf/001: a random write buffered fio perf test
2017-10-10 17:21 [PATCH 0/2] Add fio performance tracking to fstests Josef Bacik
2017-10-10 17:21 ` [PATCH 1/2] fstests: add fio perf results support Josef Bacik
@ 2017-10-10 17:21 ` Josef Bacik
1 sibling, 0 replies; 6+ messages in thread
From: Josef Bacik @ 2017-10-10 17:21 UTC (permalink / raw)
To: kernel-team, fstests, david, tytso; +Cc: Josef Bacik
From: Josef Bacik <jbacik@fb.com>
This uses the new fio results perf helpers to run a rand write buffered
workload on the scratch device.
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
tests/perf/001 | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
tests/perf/001.out | 2 ++
2 files changed, 75 insertions(+)
create mode 100644 tests/perf/001
create mode 100644 tests/perf/001.out
diff --git a/tests/perf/001 b/tests/perf/001
new file mode 100644
index 000000000000..d2188ebaaf31
--- /dev/null
+++ b/tests/perf/001
@@ -0,0 +1,73 @@
+#! /bin/bash
+# perf/001 Test
+#
+# Buffered random write performance test.
+#
+#-----------------------------------------------------------------------
+# (c) 2017 Josef Bacik
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+#
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+fio_config=$tmp.fio
+fio_results=$tmp.json
+status=1 # failure is the default!
+trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+_require_block_device $SCRATCH_DEV
+_fio_results_init
+
+rm -f $seqres.full
+
+_size=(16 * $LOAD_FACTOR)
+
+cat >$fio_config <<EOF
+[t1]
+directory=${SCRATCH_MNT}
+allrandrepeat=1
+readwrite=randwrite
+size=${_size}G
+ioengine=psync
+end_fsync=1
+fallocate=none
+EOF
+
+_require_fio $fio_config
+
+_scratch_mkfs >> $seqres.full 2>&1
+_scratch_mount
+
+cat $fio_config >> $seqres.full
+run_check $FIO_PROG --output-format=json --output=$fio_results $fio_config
+
+_scratch_unmount
+_fio_results_compare $seq $fio_results
+echo "Silence is golden"
+status=0; exit
diff --git a/tests/perf/001.out b/tests/perf/001.out
new file mode 100644
index 000000000000..88678b8ed5ad
--- /dev/null
+++ b/tests/perf/001.out
@@ -0,0 +1,2 @@
+QA output created by 001
+Silence is golden
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] fstests: add fio perf results support
2017-10-10 17:21 ` [PATCH 1/2] fstests: add fio perf results support Josef Bacik
@ 2017-10-10 17:53 ` Darrick J. Wong
2017-10-10 18:14 ` Josef Bacik
0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2017-10-10 17:53 UTC (permalink / raw)
To: Josef Bacik; +Cc: kernel-team, fstests, david, tytso, Josef Bacik
On Tue, Oct 10, 2017 at 01:21:24PM -0400, Josef Bacik wrote:
> From: Josef Bacik <jbacik@fb.com>
>
> This patch does the nuts and bolts of grabbing fio results and storing
> them in a database in order to check against for future runs. This
> works by storing the results in resuts/fio-results.db as a sqlite
> database. The src/perf directory has all the supporting python code for
> parsing the fio json results, storing it in the database, and loading
> previous results from the database to compare with the current results.
>
> This also adds a PERF_CONFIGNAME option that must be set for this to
> work. Since we all have various ways we run fstests it doesn't make
> sense to compare different configurations with each other (unless
> specifically desired). The PERF_CONFIGNAME will allow us to separate
> out results for different test run configurations to make sure we're
> comparing results correctly.
>
> Currently we only check against the last perf result. In the future I
> will flesh this out to compare against the average of N number of runs
> to be a little more complete, and hopefully that will allow us to also
> watch latencies as well.
>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
> ---
> .gitignore | 1 +
> common/config | 2 +
> common/rc | 32 +++++++++++
> src/perf/FioCompare.py | 106 +++++++++++++++++++++++++++++++++++++
> src/perf/FioResultDecoder.py | 58 ++++++++++++++++++++
> src/perf/ResultData.py | 43 +++++++++++++++
> src/perf/fio-insert-and-compare.py | 32 +++++++++++
> src/perf/fio-results.sql | 93 ++++++++++++++++++++++++++++++++
> src/perf/generate-schema.py | 49 +++++++++++++++++
> 9 files changed, 416 insertions(+)
> create mode 100644 src/perf/FioCompare.py
> create mode 100644 src/perf/FioResultDecoder.py
> create mode 100644 src/perf/ResultData.py
> create mode 100644 src/perf/fio-insert-and-compare.py
> create mode 100644 src/perf/fio-results.sql
> create mode 100644 src/perf/generate-schema.py
>
> diff --git a/.gitignore b/.gitignore
> index ae7ef87ab384..986a6f7ff0ad 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -156,6 +156,7 @@
> /src/aio-dio-regress/aiocp
> /src/aio-dio-regress/aiodio_sparse2
> /src/log-writes/replay-log
> +/src/perf/*.pyc
>
> # dmapi/ binaries
> /dmapi/src/common/cmd/read_invis
> diff --git a/common/config b/common/config
> index 71798f0adb1e..d2b2e2cda688 100644
> --- a/common/config
> +++ b/common/config
> @@ -195,6 +195,8 @@ export MAN_PROG="`set_prog_path man`"
> export NFS4_SETFACL_PROG="`set_prog_path nfs4_setfacl`"
> export NFS4_GETFACL_PROG="`set_prog_path nfs4_getfacl`"
> export UBIUPDATEVOL_PROG="`set_prog_path ubiupdatevol`"
> +export PYTHON_PROG="`set_prog_path python`"
Given that /usr/bin/python will some day point to python 3 instead of
python 2 (and all of the grief that will ensue when someone has to run
fstests on, say, 2019 and 2021-era Linux distros at the same time when
they finally start pointing /usr/bin/python to 3.x), I think we should
avoid relying on /usr/bin/python to run our fstests scripts.
ISTR in the previous thread that python 2.7 was a requirement for the
scripts below, so find the python2 binary instead. It might even be a
good idea to have a _require_python_version helper to check that the
interpreter exists and that it's of a specific vintage.
> +export SQLITE3_PROG="`set_prog_path sqlite3`"
>
> # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
> # newer systems have udevadm command but older systems like RHEL5 don't.
> diff --git a/common/rc b/common/rc
> index 53bbb1187f81..2660ad51ed26 100644
> --- a/common/rc
> +++ b/common/rc
Should we put this in common/perf in case we start adding more
performance testing helper functions? common/rc is already a mess.
> @@ -2997,6 +2997,38 @@ _require_fio()
> [ $? -eq 0 ] || _notrun "$FIO_PROG too old, see $seqres.full"
> }
>
> +_fio_results_init()
I'd put the requirements checking into a separate
_require_fio_results_dependencies() helper because any funcion call
starting with '_require' signals (to me anyway) that we're checking for
required software and will _notrun if we don't find it.
Test initialization functions, by contrast, only get run once the
_require* calls are finished and we therefore know that we have all the
external pieces we need to run the test. If init doesn't succeed, the
test can _fail.
--D
> +{
> + if [ -z "$PERF_CONFIGNAME" ]
> + then
> + _notrun "this test requires \$PERF_CONFIGNAME to be set"
> + fi
> + _require_command $PYTHON_PROG python
> +
> + $PYTHON_PROG -c "import sqlite3" >/dev/null 2>&1
> + [ $? -ne 0 ] && _notrun "this test requires python sqlite support"
> +
> + $PYTHON_PROG -c "import json" >/dev/null 2>&1
> + [ $? -ne 0 ] && _notrun "this test requires python json support"
> +
> + _require_command $SQLITE3_PROG sqlite3
> + cat $here/src/perf/fio-results.sql | \
> + $SQLITE3_PROG $RESULT_BASE/fio-results.db
> + [ $? -ne 0 ] && _notrun "failed to create results database"
> + [ ! -e $RESULT_BASE/fio-results.db ] && \
> + _notrun "failed to create results database"
> +}
> +
> +_fio_results_compare()
> +{
> + _testname=$1
> + _resultfile=$2
> +
> + run_check $PYTHON_PROG $here/src/perf/fio-insert-and-compare.py \
> + -c $PERF_CONFIGNAME -d $RESULT_BASE/fio-results.db \
> + -n $_testname $_resultfile
> +}
> +
> # Does freeze work on this fs?
> _require_freeze()
> {
> diff --git a/src/perf/FioCompare.py b/src/perf/FioCompare.py
> new file mode 100644
> index 000000000000..55d13699c34c
> --- /dev/null
> +++ b/src/perf/FioCompare.py
> @@ -0,0 +1,106 @@
> +default_keys = [ 'iops', 'io_kbytes', 'bw' ]
> +latency_keys = [ 'lat_ns_min', 'lat_ns_max' ]
> +main_job_keys = [ 'sys_cpu', 'elapsed' ]
> +io_ops = ['read', 'write', 'trim' ]
> +
> +def _fuzzy_compare(a, b, fuzzy):
> + if a == b:
> + return 0
> + if a == 0:
> + return 100
> + a = float(a)
> + b = float(b)
> + fuzzy = float(fuzzy)
> + val = ((b - a) / a) * 100
> + if val > fuzzy or val < -fuzzy:
> + return val;
> + return 0
> +
> +def _compare_jobs(ijob, njob, latency, fuzz):
> + failed = 0
> + for k in default_keys:
> + for io in io_ops:
> + key = "{}_{}".format(io, k)
> + comp = _fuzzy_compare(ijob[key], njob[key], fuzz)
> + if comp < 0:
> + print(" {} regressed: old {} new {} {}%".format(key,
> + ijob[key], njob[key], comp))
> + failed += 1
> + elif comp > 0:
> + print(" {} improved: old {} new {} {}%".format(key,
> + ijob[key], njob[key], comp))
> + for k in latency_keys:
> + if not latency:
> + break
> + for io in io_ops:
> + key = "{}_{}".format(io, k)
> + comp = _fuzzy_compare(ijob[key], njob[key], fuzz)
> + if comp > 0:
> + print(" {} regressed: old {} new {} {}%".format(key,
> + ijob[key], njob[key], comp))
> + failed += 1
> + elif comp < 0:
> + print(" {} improved: old {} new {} {}%".format(key,
> + ijob[key], njob[key], comp))
> + for k in main_job_keys:
> + comp = _fuzzy_compare(ijob[k], njob[k], fuzz)
> + if comp > 0:
> + print(" {} regressed: old {} new {} {}%".format(k, ijob[k],
> + njob[k], comp))
> + failed += 1
> + elif comp < 0:
> + print(" {} improved: old {} new {} {}%".format(k, ijob[k],
> + njob[k], comp))
> + return failed
> +
> +def compare_individual_jobs(initial, data, fuzz):
> + failed = 0;
> + initial_jobs = initial['jobs'][:]
> + for njob in data['jobs']:
> + for ijob in initial_jobs:
> + if njob['jobname'] == ijob['jobname']:
> + print(" Checking results for {}".format(njob['jobname']))
> + failed += _compare_jobs(ijob, njob, fuzz)
> + initial_jobs.remove(ijob)
> + break
> + return failed
> +
> +def default_merge(data):
> + '''Default merge function for multiple jobs in one run
> +
> + For runs that include multiple threads we will have a lot of variation
> + between the different threads, which makes comparing them to eachother
> + across multiple runs less that useful. Instead merge the jobs into a single
> + job. This function does that by adding up 'iops', 'io_kbytes', and 'bw' for
> + read/write/trim in the merged job, and then taking the maximal values of the
> + latency numbers.
> + '''
> + merge_job = {}
> + for job in data['jobs']:
> + for k in main_job_keys:
> + if k not in merge_job:
> + merge_job[k] = job[k]
> + else:
> + merge_job[k] += job[k]
> + for io in io_ops:
> + for k in default_keys:
> + key = "{}_{}".format(io, k)
> + if key not in merge_job:
> + merge_job[key] = job[key]
> + else:
> + merge_job[key] += job[key]
> + for k in latency_keys:
> + key = "{}_{}".format(io, k)
> + if key not in merge_job:
> + merge_job[key] = job[key]
> + elif merge_job[key] < job[key]:
> + merge_job[key] = job[key]
> + return merge_job
> +
> +def compare_fiodata(initial, data, latency, merge_func=default_merge, fuzz=5):
> + failed = 0
> + if merge_func is None:
> + return compare_individual_jobs(initial, data, fuzz)
> + ijob = merge_func(initial)
> + njob = merge_func(data)
> + return _compare_jobs(ijob, njob, latency, fuzz)
> diff --git a/src/perf/FioResultDecoder.py b/src/perf/FioResultDecoder.py
> new file mode 100644
> index 000000000000..51efae308add
> --- /dev/null
> +++ b/src/perf/FioResultDecoder.py
> @@ -0,0 +1,58 @@
> +import json
> +
> +class FioResultDecoder(json.JSONDecoder):
> + """Decoder for decoding fio result json to an object for our database
> +
> + This decodes the json output from fio into an object that can be directly
> + inserted into our database. This just strips out the fields we don't care
> + about and collapses the read/write/trim classes into a flat value structure
> + inside of the jobs object.
> +
> + For example
> + "write" : {
> + "io_bytes" : 313360384,
> + "bw" : 1016,
> + }
> +
> + Get's collapsed to
> +
> + "write_io_bytes" : 313360384,
> + "write_bw": 1016,
> +
> + Currently any dict under 'jobs' get's dropped, with the exception of 'read',
> + 'write', and 'trim'. For those sub sections we drop any dict's under those.
> +
> + Attempt to keep this as generic as possible, we don't want to break every
> + time fio changes it's json output format.
> + """
> + _ignore_types = ['dict', 'list']
> + _override_keys = ['lat_ns']
> + _io_ops = ['read', 'write', 'trim']
> +
> + def decode(self, json_string):
> + """This does the dirty work of converting everything"""
> + default_obj = super(FioResultDecoder, self).decode(json_string)
> + obj = {}
> + obj['global'] = {}
> + obj['global']['time'] = default_obj['time']
> + obj['jobs'] = []
> + for job in default_obj['jobs']:
> + new_job = {}
> + for key,value in job.iteritems():
> + if key not in self._io_ops:
> + if value.__class__.__name__ in self._ignore_types:
> + continue
> + new_job[key] = value
> + continue
> + for k,v in value.iteritems():
> + if k in self._override_keys:
> + for subk,subv in v.iteritems():
> + collapsed_key = "{}_{}_{}".format(key, k, subk)
> + new_job[collapsed_key] = subv
> + continue
> + if v.__class__.__name__ in self._ignore_types:
> + continue
> + collapsed_key = "{}_{}".format(key, k)
> + new_job[collapsed_key] = v
> + obj['jobs'].append(new_job)
> + return obj
> diff --git a/src/perf/ResultData.py b/src/perf/ResultData.py
> new file mode 100644
> index 000000000000..f0c7eace6dad
> --- /dev/null
> +++ b/src/perf/ResultData.py
> @@ -0,0 +1,43 @@
> +import sqlite3
> +
> +def _dict_factory(cursor, row):
> + d = {}
> + for idx,col in enumerate(cursor.description):
> + d[col[0]] = row[idx]
> + return d
> +
> +class ResultData:
> + def __init__(self, filename):
> + self.db = sqlite3.connect(filename)
> + self.db.row_factory = _dict_factory
> +
> + def load_last(self, testname, config):
> + d = {}
> + cur = self.db.cursor()
> + cur.execute("SELECT * FROM fio_runs WHERE config = ? AND name = ?ORDER BY time DESC LIMIT 1",
> + (config,testname))
> + d['global'] = cur.fetchone()
> + if d['global'] is None:
> + return None
> + cur.execute("SELECT * FROM fio_jobs WHERE run_id = ?",
> + (d['global']['id'],))
> + d['jobs'] = cur.fetchall()
> + return d
> +
> + def _insert_obj(self, tablename, obj):
> + keys = obj.keys()
> + values = obj.values()
> + cur = self.db.cursor()
> + cmd = "INSERT INTO {} ({}) VALUES ({}".format(tablename,
> + ",".join(keys),
> + '?,' * len(values))
> + cmd = cmd[:-1] + ')'
> + cur.execute(cmd, tuple(values))
> + self.db.commit()
> + return cur.lastrowid
> +
> + def insert_result(self, result):
> + row_id = self._insert_obj('fio_runs', result['global'])
> + for job in result['jobs']:
> + job['run_id'] = row_id
> + self._insert_obj('fio_jobs', job)
> diff --git a/src/perf/fio-insert-and-compare.py b/src/perf/fio-insert-and-compare.py
> new file mode 100644
> index 000000000000..0a7460fcbab7
> --- /dev/null
> +++ b/src/perf/fio-insert-and-compare.py
> @@ -0,0 +1,32 @@
> +import FioResultDecoder
> +import ResultData
> +import FioCompare
> +import json
> +import argparse
> +import sys
> +import platform
> +
> +parser = argparse.ArgumentParser()
> +parser.add_argument('-c', '--configname', type=str,
> + help="The config name to save the results under.",
> + required=True)
> +parser.add_argument('-d', '--db', type=str,
> + help="The db that is being used", required=True)
> +parser.add_argument('-n', '--testname', type=str,
> + help="The testname for the result", required=True)
> +parser.add_argument('result', type=str,
> + help="The result file to compare and insert")
> +args = parser.parse_args()
> +
> +result_data = ResultData.ResultData(args.db)
> +
> +json_data = open(args.result)
> +data = json.load(json_data, cls=FioResultDecoder.FioResultDecoder)
> +data['global']['name'] = args.testname
> +data['global']['config'] = args.configname
> +data['global']['kernel'] = platform.release()
> +result_data.insert_result(data)
> +
> +compare = result_data.load_last(args.testname, args.configname)
> +if FioCompare.compare_fiodata(compare, data, False):
> + sys.exit(1)
> diff --git a/src/perf/fio-results.sql b/src/perf/fio-results.sql
> new file mode 100644
> index 000000000000..b7f6708e1265
> --- /dev/null
> +++ b/src/perf/fio-results.sql
> @@ -0,0 +1,93 @@
> +CREATE TABLE IF NOT EXISTS `fio_runs` (
> + `id` INTEGER PRIMARY KEY AUTOINCREMENT,
> + `kernel` datetime NOT NULL,
> + `config` varchar(256) NOT NULL,
> + `name` varchar(256) NOT NULL,
> + `time` datetime NOT NULL
> +);
> +CREATE TABLE IF NOT EXISTS `fio_jobs` (
> + `run_id` int NOT NULL,
> + `latency_window` int NOT NULL,
> + `trim_lat_ns_mean` float NOT NULL,
> + `read_iops_min` int NOT NULL,
> + `read_bw_dev` float NOT NULL,
> + `trim_runtime` int NOT NULL,
> + `read_io_bytes` int NOT NULL,
> + `read_short_ios` int NOT NULL,
> + `read_iops_samples` int NOT NULL,
> + `minf` int NOT NULL,
> + `read_drop_ios` int NOT NULL,
> + `trim_iops_samples` int NOT NULL,
> + `trim_iops_max` int NOT NULL,
> + `trim_bw_agg` float NOT NULL,
> + `write_bw_min` int NOT NULL,
> + `write_iops_mean` float NOT NULL,
> + `read_bw_max` int NOT NULL,
> + `read_bw_min` int NOT NULL,
> + `trim_bw_dev` float NOT NULL,
> + `read_iops_max` int NOT NULL,
> + `read_total_ios` int NOT NULL,
> + `read_lat_ns_mean` float NOT NULL,
> + `write_iops` float NOT NULL,
> + `latency_target` int NOT NULL,
> + `trim_bw` int NOT NULL,
> + `eta` int NOT NULL,
> + `read_bw_samples` int NOT NULL,
> + `trim_io_kbytes` int NOT NULL,
> + `write_iops_max` int NOT NULL,
> + `write_drop_ios` int NOT NULL,
> + `trim_iops_min` int NOT NULL,
> + `write_bw_samples` int NOT NULL,
> + `read_iops_stddev` float NOT NULL,
> + `write_io_kbytes` int NOT NULL,
> + `trim_bw_mean` float NOT NULL,
> + `write_bw_agg` float NOT NULL,
> + `write_bw_dev` float NOT NULL,
> + `write_lat_ns_stddev` float NOT NULL,
> + `trim_lat_ns_stddev` float NOT NULL,
> + `groupid` int NOT NULL,
> + `latency_depth` int NOT NULL,
> + `trim_short_ios` int NOT NULL,
> + `read_lat_ns_stddev` float NOT NULL,
> + `write_iops_min` int NOT NULL,
> + `write_iops_stddev` float NOT NULL,
> + `read_io_kbytes` int NOT NULL,
> + `trim_bw_samples` int NOT NULL,
> + `trim_lat_ns_min` int NOT NULL,
> + `error` int NOT NULL,
> + `read_bw_mean` float NOT NULL,
> + `trim_iops_mean` float NOT NULL,
> + `elapsed` int NOT NULL,
> + `write_bw_mean` float NOT NULL,
> + `write_short_ios` int NOT NULL,
> + `ctx` int NOT NULL,
> + `write_io_bytes` int NOT NULL,
> + `usr_cpu` float NOT NULL,
> + `trim_drop_ios` int NOT NULL,
> + `write_bw` int NOT NULL,
> + `jobname` varchar(256) NOT NULL,
> + `trim_bw_min` int NOT NULL,
> + `read_runtime` int NOT NULL,
> + `sys_cpu` float NOT NULL,
> + `trim_lat_ns_max` int NOT NULL,
> + `read_iops_mean` float NOT NULL,
> + `write_lat_ns_min` int NOT NULL,
> + `trim_iops_stddev` float NOT NULL,
> + `write_lat_ns_max` int NOT NULL,
> + `majf` int NOT NULL,
> + `write_total_ios` int NOT NULL,
> + `read_bw` int NOT NULL,
> + `read_lat_ns_min` int NOT NULL,
> + `trim_bw_max` int NOT NULL,
> + `write_iops_samples` int NOT NULL,
> + `write_runtime` int NOT NULL,
> + `trim_io_bytes` int NOT NULL,
> + `latency_percentile` float NOT NULL,
> + `read_iops` float NOT NULL,
> + `trim_total_ios` int NOT NULL,
> + `write_lat_ns_mean` float NOT NULL,
> + `write_bw_max` int NOT NULL,
> + `read_bw_agg` float NOT NULL,
> + `read_lat_ns_max` int NOT NULL,
> + `trim_iops` float NOT NULL
> +);
> diff --git a/src/perf/generate-schema.py b/src/perf/generate-schema.py
> new file mode 100644
> index 000000000000..91dbdbd41b97
> --- /dev/null
> +++ b/src/perf/generate-schema.py
> @@ -0,0 +1,49 @@
> +import json
> +import argparse
> +import FioResultDecoder
> +from dateutil.parser import parse
> +
> +def is_date(string):
> + try:
> + parse(string)
> + return True
> + except ValueError:
> + return False
> +
> +def print_schema_def(key, value):
> + typestr = value.__class__.__name__
> + if typestr == 'str' or typestr == 'unicode':
> + if (is_date(value)):
> + typestr = "datetime"
> + else:
> + typestr = "varchar(256)"
> + return ",\n `{}` {} NOT NULL".format(key, typestr)
> +
> +parser = argparse.ArgumentParser()
> +parser.add_argument('infile', help="The json file to strip")
> +args = parser.parse_args()
> +
> +json_data = open(args.infile)
> +data = json.load(json_data, cls=FioResultDecoder.FioResultDecoder)
> +
> +# These get populated by the test runner, not fio, so add them so their
> +# definitions get populated in the schema properly
> +data['global']['config'] = 'default'
> +data['global']['kernel'] = '4.14'
> +
> +print("CREATE TABLE `fio_runs` (")
> +outstr = " `id` int(11) PRIMARY KEY"
> +for key,value in data['global'].iteritems():
> + outstr += print_schema_def(key, value)
> +print(outstr)
> +print(");")
> +
> +job = data['jobs'][0]
> +job['run_id'] = 0
> +
> +print("CREATE TABLE `fio_jobs` (")
> +outstr = " `id` int PRIMARY KEY"
> +for key,value in job.iteritems():
> + outstr += print_schema_def(key, value)
> +print(outstr)
> +print(");")
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] fstests: add fio perf results support
2017-10-10 17:53 ` Darrick J. Wong
@ 2017-10-10 18:14 ` Josef Bacik
2017-10-10 18:27 ` Darrick J. Wong
0 siblings, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2017-10-10 18:14 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Josef Bacik, kernel-team, fstests, david, tytso, Josef Bacik
On Tue, Oct 10, 2017 at 10:53:12AM -0700, Darrick J. Wong wrote:
> On Tue, Oct 10, 2017 at 01:21:24PM -0400, Josef Bacik wrote:
> > From: Josef Bacik <jbacik@fb.com>
> >
> > This patch does the nuts and bolts of grabbing fio results and storing
> > them in a database in order to check against for future runs. This
> > works by storing the results in resuts/fio-results.db as a sqlite
> > database. The src/perf directory has all the supporting python code for
> > parsing the fio json results, storing it in the database, and loading
> > previous results from the database to compare with the current results.
> >
> > This also adds a PERF_CONFIGNAME option that must be set for this to
> > work. Since we all have various ways we run fstests it doesn't make
> > sense to compare different configurations with each other (unless
> > specifically desired). The PERF_CONFIGNAME will allow us to separate
> > out results for different test run configurations to make sure we're
> > comparing results correctly.
> >
> > Currently we only check against the last perf result. In the future I
> > will flesh this out to compare against the average of N number of runs
> > to be a little more complete, and hopefully that will allow us to also
> > watch latencies as well.
> >
> > Signed-off-by: Josef Bacik <jbacik@fb.com>
> > ---
> > .gitignore | 1 +
> > common/config | 2 +
> > common/rc | 32 +++++++++++
> > src/perf/FioCompare.py | 106 +++++++++++++++++++++++++++++++++++++
> > src/perf/FioResultDecoder.py | 58 ++++++++++++++++++++
> > src/perf/ResultData.py | 43 +++++++++++++++
> > src/perf/fio-insert-and-compare.py | 32 +++++++++++
> > src/perf/fio-results.sql | 93 ++++++++++++++++++++++++++++++++
> > src/perf/generate-schema.py | 49 +++++++++++++++++
> > 9 files changed, 416 insertions(+)
> > create mode 100644 src/perf/FioCompare.py
> > create mode 100644 src/perf/FioResultDecoder.py
> > create mode 100644 src/perf/ResultData.py
> > create mode 100644 src/perf/fio-insert-and-compare.py
> > create mode 100644 src/perf/fio-results.sql
> > create mode 100644 src/perf/generate-schema.py
> >
> > diff --git a/.gitignore b/.gitignore
> > index ae7ef87ab384..986a6f7ff0ad 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -156,6 +156,7 @@
> > /src/aio-dio-regress/aiocp
> > /src/aio-dio-regress/aiodio_sparse2
> > /src/log-writes/replay-log
> > +/src/perf/*.pyc
> >
> > # dmapi/ binaries
> > /dmapi/src/common/cmd/read_invis
> > diff --git a/common/config b/common/config
> > index 71798f0adb1e..d2b2e2cda688 100644
> > --- a/common/config
> > +++ b/common/config
> > @@ -195,6 +195,8 @@ export MAN_PROG="`set_prog_path man`"
> > export NFS4_SETFACL_PROG="`set_prog_path nfs4_setfacl`"
> > export NFS4_GETFACL_PROG="`set_prog_path nfs4_getfacl`"
> > export UBIUPDATEVOL_PROG="`set_prog_path ubiupdatevol`"
> > +export PYTHON_PROG="`set_prog_path python`"
>
> Given that /usr/bin/python will some day point to python 3 instead of
> python 2 (and all of the grief that will ensue when someone has to run
> fstests on, say, 2019 and 2021-era Linux distros at the same time when
> they finally start pointing /usr/bin/python to 3.x), I think we should
> avoid relying on /usr/bin/python to run our fstests scripts.
>
> ISTR in the previous thread that python 2.7 was a requirement for the
> scripts below, so find the python2 binary instead. It might even be a
> good idea to have a _require_python_version helper to check that the
> interpreter exists and that it's of a specific vintage.
>
My scripts run fine on both 2 and 3. I'm worried about other distro's, does
everybody do the python2/python3 thing? If so then I'll just default to
python2, I just have no idea if that's a thing beyond Fedora/RHEL.
> > +export SQLITE3_PROG="`set_prog_path sqlite3`"
> >
> > # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
> > # newer systems have udevadm command but older systems like RHEL5 don't.
> > diff --git a/common/rc b/common/rc
> > index 53bbb1187f81..2660ad51ed26 100644
> > --- a/common/rc
> > +++ b/common/rc
>
> Should we put this in common/perf in case we start adding more
> performance testing helper functions? common/rc is already a mess.
>
Yeah I can do that.
> > @@ -2997,6 +2997,38 @@ _require_fio()
> > [ $? -eq 0 ] || _notrun "$FIO_PROG too old, see $seqres.full"
> > }
> >
> > +_fio_results_init()
>
> I'd put the requirements checking into a separate
> _require_fio_results_dependencies() helper because any funcion call
> starting with '_require' signals (to me anyway) that we're checking for
> required software and will _notrun if we don't find it.
>
> Test initialization functions, by contrast, only get run once the
> _require* calls are finished and we therefore know that we have all the
> external pieces we need to run the test. If init doesn't succeed, the
> test can _fail.
Agreed, I'll fix it. Thanks,
Josef
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] fstests: add fio perf results support
2017-10-10 18:14 ` Josef Bacik
@ 2017-10-10 18:27 ` Darrick J. Wong
0 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2017-10-10 18:27 UTC (permalink / raw)
To: Josef Bacik; +Cc: kernel-team, fstests, david, tytso, Josef Bacik
On Tue, Oct 10, 2017 at 02:14:40PM -0400, Josef Bacik wrote:
> On Tue, Oct 10, 2017 at 10:53:12AM -0700, Darrick J. Wong wrote:
> > On Tue, Oct 10, 2017 at 01:21:24PM -0400, Josef Bacik wrote:
> > > From: Josef Bacik <jbacik@fb.com>
> > >
> > > This patch does the nuts and bolts of grabbing fio results and storing
> > > them in a database in order to check against for future runs. This
> > > works by storing the results in resuts/fio-results.db as a sqlite
> > > database. The src/perf directory has all the supporting python code for
> > > parsing the fio json results, storing it in the database, and loading
> > > previous results from the database to compare with the current results.
> > >
> > > This also adds a PERF_CONFIGNAME option that must be set for this to
> > > work. Since we all have various ways we run fstests it doesn't make
> > > sense to compare different configurations with each other (unless
> > > specifically desired). The PERF_CONFIGNAME will allow us to separate
> > > out results for different test run configurations to make sure we're
> > > comparing results correctly.
> > >
> > > Currently we only check against the last perf result. In the future I
> > > will flesh this out to compare against the average of N number of runs
> > > to be a little more complete, and hopefully that will allow us to also
> > > watch latencies as well.
> > >
> > > Signed-off-by: Josef Bacik <jbacik@fb.com>
> > > ---
> > > .gitignore | 1 +
> > > common/config | 2 +
> > > common/rc | 32 +++++++++++
> > > src/perf/FioCompare.py | 106 +++++++++++++++++++++++++++++++++++++
> > > src/perf/FioResultDecoder.py | 58 ++++++++++++++++++++
> > > src/perf/ResultData.py | 43 +++++++++++++++
> > > src/perf/fio-insert-and-compare.py | 32 +++++++++++
> > > src/perf/fio-results.sql | 93 ++++++++++++++++++++++++++++++++
> > > src/perf/generate-schema.py | 49 +++++++++++++++++
> > > 9 files changed, 416 insertions(+)
> > > create mode 100644 src/perf/FioCompare.py
> > > create mode 100644 src/perf/FioResultDecoder.py
> > > create mode 100644 src/perf/ResultData.py
> > > create mode 100644 src/perf/fio-insert-and-compare.py
> > > create mode 100644 src/perf/fio-results.sql
> > > create mode 100644 src/perf/generate-schema.py
> > >
> > > diff --git a/.gitignore b/.gitignore
> > > index ae7ef87ab384..986a6f7ff0ad 100644
> > > --- a/.gitignore
> > > +++ b/.gitignore
> > > @@ -156,6 +156,7 @@
> > > /src/aio-dio-regress/aiocp
> > > /src/aio-dio-regress/aiodio_sparse2
> > > /src/log-writes/replay-log
> > > +/src/perf/*.pyc
> > >
> > > # dmapi/ binaries
> > > /dmapi/src/common/cmd/read_invis
> > > diff --git a/common/config b/common/config
> > > index 71798f0adb1e..d2b2e2cda688 100644
> > > --- a/common/config
> > > +++ b/common/config
> > > @@ -195,6 +195,8 @@ export MAN_PROG="`set_prog_path man`"
> > > export NFS4_SETFACL_PROG="`set_prog_path nfs4_setfacl`"
> > > export NFS4_GETFACL_PROG="`set_prog_path nfs4_getfacl`"
> > > export UBIUPDATEVOL_PROG="`set_prog_path ubiupdatevol`"
> > > +export PYTHON_PROG="`set_prog_path python`"
> >
> > Given that /usr/bin/python will some day point to python 3 instead of
> > python 2 (and all of the grief that will ensue when someone has to run
> > fstests on, say, 2019 and 2021-era Linux distros at the same time when
> > they finally start pointing /usr/bin/python to 3.x), I think we should
> > avoid relying on /usr/bin/python to run our fstests scripts.
> >
> > ISTR in the previous thread that python 2.7 was a requirement for the
> > scripts below, so find the python2 binary instead. It might even be a
> > good idea to have a _require_python_version helper to check that the
> > interpreter exists and that it's of a specific vintage.
> >
>
> My scripts run fine on both 2 and 3. I'm worried about other distro's, does
> everybody do the python2/python3 thing? If so then I'll just default to
> python2, I just have no idea if that's a thing beyond Fedora/RHEL.
Debian & Ubuntu both have python2/python3 binaries in their oldest
stable releases.
ISTR /usr/bin/python -> python2.7 and /usr/bin/python2 -> python2.7
Good to hear that the scripts work on both, though.
Though that adds the extra twist of what about scripts that have been
carefully written to work on either...
...I guess a solution to that could be a _require_python that sets
PYTHON_PROG. If you "_require_python $ver" then you'll get PYTHON_PROG
set to an interpreter that's compatible with $ver, or PYTHON_PROG is set
to /usr/bin/python if you don't provide a version number at all.
(Yuck yuck yuck...)
> > > +export SQLITE3_PROG="`set_prog_path sqlite3`"
> > >
> > > # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
> > > # newer systems have udevadm command but older systems like RHEL5 don't.
> > > diff --git a/common/rc b/common/rc
> > > index 53bbb1187f81..2660ad51ed26 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> >
> > Should we put this in common/perf in case we start adding more
> > performance testing helper functions? common/rc is already a mess.
> >
>
> Yeah I can do that.
>
> > > @@ -2997,6 +2997,38 @@ _require_fio()
> > > [ $? -eq 0 ] || _notrun "$FIO_PROG too old, see $seqres.full"
> > > }
> > >
> > > +_fio_results_init()
> >
> > I'd put the requirements checking into a separate
> > _require_fio_results_dependencies() helper because any funcion call
> > starting with '_require' signals (to me anyway) that we're checking for
> > required software and will _notrun if we don't find it.
> >
> > Test initialization functions, by contrast, only get run once the
> > _require* calls are finished and we therefore know that we have all the
> > external pieces we need to run the test. If init doesn't succeed, the
> > test can _fail.
>
> Agreed, I'll fix it. Thanks,
<nod>
--D
>
> Josef
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-10 18:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-10 17:21 [PATCH 0/2] Add fio performance tracking to fstests Josef Bacik
2017-10-10 17:21 ` [PATCH 1/2] fstests: add fio perf results support Josef Bacik
2017-10-10 17:53 ` Darrick J. Wong
2017-10-10 18:14 ` Josef Bacik
2017-10-10 18:27 ` Darrick J. Wong
2017-10-10 17:21 ` [PATCH 2/2] perf/001: a random write buffered fio perf test Josef Bacik
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.