- Do not update <structname>pgbench_tellers</> and - <structname>pgbench_branches</>. - This will avoid update contention on these tables, but - it makes the test case even less like TPC-B. + Shorthand for <option>-b simple-update@1</>.
I don't think it is a good idea to remove entirely the description of what the default scenarios can do. The description would be better at the bottom in some <para> with a list of each default test and what to expect from them.
I'm trying to avoid to have the same explanation twice, otherwise someone is bound to complain.
+/* data structure to hold various statistics. + * it is used for interval statistics as well as file statistics. */ Nitpick: this is not a comment formatted the Postgres-way.
Indeed.
This is surprisingly broken: $ pgbench -i some of the specified options cannot be used in initialization (-i) mode
Hmmm.
Any file name or path including "@" will fail strangely: $ pgbench -f "t...@1.sql" could not open file "test": No such file or directory empty commands for test Perhaps instead of failing we should warn the user and enforce the weight to be set at 1?
Yep, I can have a look at that.
$ pgbench -b foo no builtin found for "foo" This is not really helpful for the user, I think that the list of potential options should be listed as an error hint.
Yep.
- " -S, --select-only perform SELECT-only transactions\n" + " -S, --select-only same as \"-b select-only@1\"\n" It is good to mention that there is an equivalent, but I think that the description should be kept.
The reason replace it is to keep the help message short column-wise.
+ /* although a mutex would make sense, the likelyhood of an issue + * is small and these are only stats which may be slightly false + */ + doSimpleStats(& commands[st->state]->stats, + INSTR_TIME_GET_DOUBLE(now) -
Why would the likelyhood of an issue be small here?
The time to update one stat (<< 100 cycles ?) to the time to do a transaction with the database (typically Y ms), so the likelyhood of two thread to update the very same stat at the same time is probably under 1/10,000,000. Even if it occurs, then one stat is slightly false, no big deal. So I think the potential slowdown induced by a mutex is not worth it, so I a comment instead.
+ /* print NaN if no transactions where executed */ + double latency = ss->sum / ss->count; This does not look like a good idea, ss->count can be 0.
"sum" is a double so count is converted to 0.0, 0.0/0.0 == NaN, hence the comment.
It seems also that it would be a good idea to split the patch into two parts: 1) Refactor the code so as the existing test scripts are put under the same umbrella with addScript, adding at the same time the new option -b. 2) Add the weight facility and its related statistics.
Sigh. The patch & documentation are probably not independent, so that would make two dependent patches, probably.
-- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
