Re: pgbench stats per script & other stuff
Re: pgbench stats per script & other stuff
От:
Alvaro Herrera <alvherre@2ndquadrant.com>
Дата:
Fabien COELHO wrote: > a) add -b option for cumulating builtins and rework internal script > management so that builtin and external scripts are managed the > same way. I tweaked this a bit. I found a bug in threadRun: it was reading the commands first, and setting st->use_file later. This led to the wrong commands being read. Some other less interesting changes: * made chooseScript have the logic to react to single existing script; no need to inject ternary operators in each caller to check for that condition. * Added a debug line every time a script is chosen, + if (debug) + fprintf(stderr, "client %d executing script \"%s\"\n", st->id, + sql_script[st->use_file].name); (I'd have liked to have chooseScript itself do it, but it doesn't have the script name handy. Maybe this indicates that the data structures are slightly wrong.) * Added a separate routine to list available scripts; originally that was duplicated in "-b list" and when -b got an invalid script name. * In usage(), I split out the options to select a script instead of mixing them within "Benchmarking options"; also changed wording of parenthical comment, no longer carrying the full list of scripts (a choice which also omitted "-b list" itself): + "\nOptions to select what to run:\n" + " -b, --builtin=NAME add buitin script (use \"-b list\" to display\n" + " available scripts)\n" + " -f, --file=FILENAME add transaction script from FILENAME\n" + " -N, --skip-some-updates skip updates of pgbench_tellers and pgbench_branches\n" + " (same as \"-b simple-update\")\n" + " -S, --select-only perform SELECT-only transactions\n" + " (same as \"-b select-only\")\n" I couldn't find a better heading to use there, so that'll have to do unless someone has a better idea. Some other trivial changes. Patch attached. I plan to push this as soon as I'm able. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: pgbench stats per script & other stuff
От:
Alvaro Herrera <alvherre@2ndquadrant.com>
Дата:
Alvaro Herrera wrote: > I'm uncomfortable with the prefix-matching aspect of -b. It makes > "-b s" ambiguous -- whether it stands for select-only or simple-update > is merely a matter of what comes earlier in the table, which doesn't > seem reasonable to me. [...] > I'm going to change this to use strlen(builtin_script[i].name) instead > of "len" here. I pushed like that, but of course that means you can use "-b simple-update-foo" and it works. I could have used just strcmp(). (Part e is pushed too along with an initial pgindent). Here's part b rebased, pgindented and with some minor additional tweaks (mostly function commands and the function renames I mentioned). Still concerned about the unlocked stat accums. I haven't tried to rebase the other ones yet, they need manual conflict fixes. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: pgbench stats per script & other stuff
От:
Alvaro Herrera <alvherre@2ndquadrant.com>
Дата:
Something is wrong with patch d. I noticed two things,
1. the total_weight stuff can overflow,
2. the chooseScript stuff is broken, or something.
See the output below and notice how the percentages don't add up to 100%
(this exact case is an absurd one, of course, but I noticed totals of
99.5% and others while playing with reasonable numbers, so this is
something that really needs to be fixed.)
Another thing is that the "transaction type" output really deserves some
more work. I think "multiple scripts" really doesn't cut it; we should
have some YAML-like as in the latency reports, which lists all scripts
in use and their weights.
Also, while I have your attention regarding accumulated "technical
debt", please have a look at the "desc" argument used in addScript etc.
It's pretty ridiculous currently. Maybe findBuiltin / process_builtin /
process_file should return a struct containing Command ** and the
"desc" string, rather than passing desc as a separate argument.
I changed the getWeight stuff completely (and renamed it, and added
comments); I wasn't comfortable with the idea of messing with the
optarg, and neither with the idea of continuing to use it without
copying after processing the arguments. I think some platforms don't
like any of those things.
Attached is my version of the patch. While you're messing with it, it'd
be nice if you added comments on top of your recently added functions
such as findBuiltin, process_builtin, chooseScript.
$ ./pgbench -r -j4 -c4 -t1000 -b tpcb-like@10 -f uno.sql@214748364
starting vacuum...end.
transaction type: multiple scripts
scaling factor: 1
query mode: simple
number of clients: 4
number of threads: 4
number of transactions per client: 1000
number of transactions actually processed: 4000/4000
latency average: 0.000 ms
tps = 23422.357812 (including connections establishing)
tps = 24172.981858 (excluding connections establishing)
SQL script 1, weight 10:
- 0 transactions (0.0% of total, tps = 0.000000)
- latency average = -nan ms
- latency stddev = -nan ms
- statement latencies in milliseconds:
-nan \set nbranches 1 * :scale
-nan \set ntellers 10 * :scale
-nan \set naccounts 100000 * :scale
-nan \setrandom aid 1 :naccounts
-nan \setrandom bid 1 :nbranches
-nan \setrandom tid 1 :ntellers
-nan \setrandom delta -5000 5000
-nan BEGIN;
-nan UPDATE pgbench_accounts SET abalance = abalance + :delta WHERE aid = :aid;
-nan SELECT abalance FROM pgbench_accounts WHERE aid = :aid;
-nan UPDATE pgbench_tellers SET tbalance = tbalance + :delta WHERE tid = :tid;
-nan UPDATE pgbench_branches SET bbalance = bbalance + :delta WHERE bid = :bid;
-nan INSERT INTO pgbench_history (tid, bid, aid, delta, mtime) VALUES (:tid, :bid, :aid, :delta, CURRENT_TIMESTAMP);
-nan END;
SQL script 2, weight 214748364: uno.sql
- 2649 transactions (66.2% of total, tps = 15511.456461)
- latency average = 0.163 ms
- latency stddev = 0.337 ms
- statement latencies in milliseconds:
0.158 select 1;
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: pgbench stats per script & other stuff
От:
Alvaro Herrera <alvherre@2ndquadrant.com>
Дата:
Jeff Janes wrote: > On Sat, Mar 19, 2016 at 8:41 AM, Alvaro Herrera > wrote: > > I pushed your 25, with some additional minor tweaks. I hope I didn't > > break anything; please test. > > I'm now getting compiler warnings: > > gcc version 4.4.7 20120313 (Red Hat 4.4.7-16) (GCC) > > > pgbench.c: In function 'process_builtin': > pgbench.c:2765: warning: 'ps.stats.lag.sum2' is used uninitialized in > this function Fair complaints. I suppose the following should fix them? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: pgbench stats per script & other stuff
От:
Alvaro Herrera <alvherre@2ndquadrant.com>
Дата:
Alvaro Herrera wrote: > In doing this, I noticed that the latency output is wrong if you use -T > instead of -t; it always says the latency is zero because "duration" is > zero. I suppose it should be like in the attached instead. At the same > time, it says "latency average: XYZ" instead of "latency average = XYZ" > as in printSimpleStats, which doesn't look terribly important. But the > line appears in the SGML docs. Patch actually attached here. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: pgbench stats per script & other stuff
От:
Jeff Janes <jeff.janes@gmail.com>
Дата:
On Fri, Jul 17, 2015 at 6:50 AM, Fabien wrote: > > This patch adds per-script statistics & other improvements to pgbench > > Rationale: Josh asked for the per-script stats:-) > > Some restructuring is done so that all stats (-l --aggregate-interval > --progress --per-script-stats, latency & lag...) share the same structures > and functions to accumulate data. This limits a lot the growth of pgbench > from this patch (+17 lines). > > In passing, remove the distinction between internal and external scripts. > Pgbench just execute scripts, some of them may be internal... > > As a side effect, all scripts can be accumulated "pgbench -B -N -S -f ..." > would execute 4 scripts, 3 of which internal (tpc-b, simple-update, > select-only and another externally supplied one). > > Also add a weight option to change the probability of choosing some scripts > when several are available. I was eager to use this to do some performance testing on a series of workloads gradually transitioning from write-heavy to read-only. So I wanted to do something like: for f in `seq 0 5 100`; do pgbench -T 180 -c8 -j8 -b tpcb-like@$f -b select-only@100 done; But, I'm not allowed to specify a weight of zero. That means I have to special-case the first iteration of the "for" loop where $f is zero. I think it would be more convenient if I was allowed to specify a zero weight, and the script would just ignore that script. All I had to do to make this work is remove the check that prevents from setting the weight to zero. But then I would need to add in a check that the sum of all weights is not zero, which I have done here. We could get more complex by not adding a zero-weight script into the array of scripts at all, rather than adding it in a way where it can never be selected. But then that would complicate the parsing of the per-script stats report, when one of the scripts was no longer reported. I like this way better. Would this be a welcome change? Cheers, Jeff
