Hello Alvaro,
Something is wrong with patch d. I noticed two things,
1. the total_weight stuff can overflow,
It can generate an error on overflow by checking the total_weight while
it is being computed. I've switched total_weight to int64 so it is now
really impossible to overflow with the 32 bit int_max limit on weight.
2. the chooseScript stuff is broken, or something.
Sorry, probably a <=/< error. I think I've fixed it and I've simplified
the code a little bit.
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.
For me the current output is clear for the reader, which does not
mean that it cannot be improve, but how is rather a matter of taste.
I've tried to improve it further, see attached patch.
If you want something else, it would help to provide a sample of what you
expect.
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.
Ok, it can return a pointer to the builtin script.
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.
Why not.
Find attached a 18-d which addresses these concerns, and a actualized 18-e
for the prefix.
--
Fabien.
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index ade1b53..780a520 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -262,11 +262,13 @@ pgbench <optional> <replaceable>options</> </optional> <replaceable>dbname</>
<variablelist>
<varlistentry>
- <term><option>-b</> <replaceable>scriptname</></term>
- <term><option>--builtin</> <replaceable>scriptname</></term>
+ <term><option>-b</> <replaceable>scriptname[@weight]</></term>
+ <term><option>--builtin</>=<replaceable>scriptname[@weight]</></term>
<listitem>
<para>
Add the specified builtin script to the list of executed scripts.
+ An optional integer weight after <literal>@</> allows to adjust the
+ probability of drawing the script. If not specified, it is set to 1.
Available builtin scripts are: <literal>tpcb-like</>,
<literal>simple-update</> and <literal>select-only</>.
With special name <literal>list</>, show the list of builtin scripts
@@ -321,12 +323,14 @@ pgbench <optional> <replaceable>options</> </optional> <replaceable>dbname</>
</varlistentry>
<varlistentry>
- <term><option>-f</> <replaceable>filename</></term>
- <term><option>--file=</><replaceable>filename</></term>
+ <term><option>-f</> <replaceable>filename[@weight]</></term>
+ <term><option>--file=</><replaceable>filename[@weight]</></term>
<listitem>
<para>
Add a transaction script read from <replaceable>filename</> to
the list of executed scripts.
+ An optional integer weight after <literal>@</> allows to adjust the
+ probability of drawing the test.
See below for details.
</para>
</listitem>
@@ -686,9 +690,13 @@ pgbench <optional> <replaceable>options</> </optional> <replaceable>dbname</>
<title>What is the <quote>Transaction</> Actually Performed in <application>pgbench</application>?</title>
<para>
- Pgbench executes test scripts chosen randomly from a specified list.
+ <application>pgbench</> executes test scripts chosen randomly
+ from a specified list.
They include built-in scripts with <option>-b</> and
user-provided custom scripts with <option>-f</>.
+ Each script may be given a relative weight specified after a
+ <literal>@</> so as to change its drawing probability.
+ The default weight is <literal>1</>.
</para>
<para>
@@ -1135,12 +1143,11 @@ number of clients: 10
number of threads: 1
number of transactions per client: 1000
number of transactions actually processed: 10000/10000
+latency average = 15.844 ms
+latency stddev = 2.715 ms
tps = 618.764555 (including connections establishing)
tps = 622.977698 (excluding connections establishing)
-SQL script 1: <builtin: TPC-B (sort of)>
- - 10000 transactions (100.0% of total, tps = 618.764555)
- - latency average = 15.844 ms
- - latency stddev = 2.715 ms
+script statistics:
- statement latencies in milliseconds:
0.004386 \set nbranches 1 * :scale
0.001343 \set ntellers 10 * :scale
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 7eb6a2d..da4f05c 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -38,6 +38,7 @@
#include "portability/instr_time.h"
#include <ctype.h>
+#include <limits.h>
#include <math.h>
#include <signal.h>
#include <sys/time.h>
@@ -179,6 +180,8 @@ char *login = NULL;
char *dbName;
const char *progname;
+#define WSEP '@' /* weight separator */
+
volatile bool timer_exceeded = false; /* flag from signal handler */
/* variable definitions */
@@ -299,23 +302,26 @@ typedef struct
static struct
{
const char *name;
+ int weight;
Command **commands;
- StatsData stats;
+ StatsData stats;
} sql_script[MAX_SCRIPTS]; /* SQL script files */
static int num_scripts; /* number of scripts in sql_script[] */
static int num_commands = 0; /* total number of Command structs */
+static int64 total_weight = 0;
+
static int debug = 0; /* debug flag */
/* Define builtin test scripts */
-#define N_BUILTIN 3
-static struct
+typedef struct script_t
{
char *name; /* very short name for -b ... */
char *desc; /* short description */
char *commands; /* actual pgbench script */
-}
+} script_t;
- builtin_script[] =
+#define N_BUILTIN 3
+static script_t builtin_script[] =
{
{
"tpcb-like",
@@ -389,9 +395,9 @@ usage(void)
" --tablespace=TABLESPACE create tables in the specified tablespace\n"
" --unlogged-tables create tables as unlogged tables\n"
"\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"
+ " -b, --builtin=NAME[@W] add builtin script NAME weighted at W (default: 1)\n"
+ " (use \"-b list\" to list available scripts)\n"
+ " -f, --file=FILENAME[@W] add script FILENAME weighted at W (default: 1)\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"
@@ -1235,13 +1241,23 @@ clientDone(CState *st, bool ok)
return false; /* always false */
}
+/* return a script number with a weighted choice. */
static int
chooseScript(TState *thread)
{
+ int i = 0;
+ int64 w;
+
if (num_scripts == 1)
return 0;
- return getrand(thread, 0, num_scripts - 1);
+ w = getrand(thread, 0, total_weight - 1);
+ do
+ {
+ w -= sql_script[i++].weight;
+ } while (w >= 0);
+
+ return i - 1;
}
/* return false iff client should be disconnected */
@@ -2597,6 +2613,7 @@ process_file(char *filename)
return my_commands;
}
+/* process builtin script tb and return an array of its commands */
static Command **
process_builtin(const char *tb, const char *source)
{
@@ -2652,6 +2669,7 @@ process_builtin(const char *tb, const char *source)
return my_commands;
}
+/* show available builtin scripts */
static void
listAvailableScripts(void)
{
@@ -2663,8 +2681,9 @@ listAvailableScripts(void)
fprintf(stderr, "\n");
}
-static char *
-findBuiltin(const char *name, char **desc)
+/* return builtin script "name", or fail if not found */
+static script_t *
+findBuiltin(const char *name)
{
int i;
@@ -2672,10 +2691,7 @@ findBuiltin(const char *name, char **desc)
{
if (strncmp(builtin_script[i].name, name,
strlen(builtin_script[i].name)) == 0)
- {
- *desc = builtin_script[i].desc;
- return builtin_script[i].commands;
- }
+ return & builtin_script[i];
}
fprintf(stderr, "no builtin script found for name \"%s\"\n", name);
@@ -2683,8 +2699,57 @@ findBuiltin(const char *name, char **desc)
exit(1);
}
+/*
+ * Determine the weight specification from a script option (-b, -f), if any,
+ * and return it as an integer (1 is returned if there's no weight). The
+ * script name is returned in *script as a malloc'd string.
+ */
+static int
+parseScriptWeight(const char *option, char **script)
+{
+ char *sep;
+ int weight;
+
+ if ((sep = strrchr(option, WSEP)))
+ {
+ int namelen = sep - option;
+ long wtmp;
+ char *badp;
+
+ /* generate the script name */
+ *script = pg_malloc(namelen + 1);
+ strncpy(*script, option, namelen);
+ (*script)[namelen] = '\0';
+
+ /* process digits of the weight spec */
+ errno = 0;
+ wtmp = strtol(sep + 1, &badp, 10);
+ if (errno != 0 || badp == sep + 1 || *badp != '\0')
+ {
+ fprintf(stderr, "invalid weight specification: %s\n", sep);
+ exit(1);
+ }
+ if (wtmp > INT_MAX || wtmp <= 0)
+ {
+ fprintf(stderr,
+ "weight specification out of range (1 .. %u): " INT64_FORMAT "\n",
+ INT_MAX, (int64) wtmp);
+ exit(1);
+ }
+ weight = wtmp;
+ }
+ else
+ {
+ *script = pg_strdup(option);
+ weight = 1;
+ }
+
+ return weight;
+}
+
+/* append a script to the list of script to process */
static void
-addScript(const char *name, Command **commands)
+addScript(const char *name, Command **commands, int weight)
{
if (commands == NULL)
{
@@ -2699,6 +2764,7 @@ addScript(const char *name, Command **commands)
}
sql_script[num_scripts].name = name;
+ sql_script[num_scripts].weight = weight;
sql_script[num_scripts].commands = commands;
initStats(&sql_script[num_scripts].stats, 0.0);
num_scripts++;
@@ -2784,19 +2850,24 @@ printResults(TState *threads, StatsData *total, instr_time total_time,
printf("tps = %f (including connections establishing)\n", tps_include);
printf("tps = %f (excluding connections establishing)\n", tps_exclude);
- /* Report per-command statistics */
- if (per_script_stats)
+ /* Report per-script/command statistics */
+ if (per_script_stats || latency_limit || is_latencies)
{
int i;
for (i = 0; i < num_scripts; i++)
{
- printf("SQL script %d: %s\n"
- " - " INT64_FORMAT " transactions (%.1f%% of total, tps = %f)\n",
- i + 1, sql_script[i].name,
- sql_script[i].stats.cnt,
- 100.0 * sql_script[i].stats.cnt / total->cnt,
- sql_script[i].stats.cnt / time_include);
+ if (num_scripts > 1)
+ printf("SQL script %d: %s\n"
+ " - weight = %d\n"
+ " - " INT64_FORMAT " transactions (%.1f%% of total, tps = %f)\n",
+ i + 1, sql_script[i].name,
+ sql_script[i].weight,
+ sql_script[i].stats.cnt,
+ 100.0 * sql_script[i].stats.cnt / total->cnt,
+ sql_script[i].stats.cnt / time_include);
+ else
+ printf("script statistics:\n");
if (latency_limit)
printf(" - number of transactions skipped: " INT64_FORMAT " (%.3f%%)\n",
@@ -2804,7 +2875,8 @@ printResults(TState *threads, StatsData *total, instr_time total_time,
100.0 * sql_script[i].stats.skipped /
(sql_script[i].stats.skipped + sql_script[i].stats.cnt));
- printSimpleStats(" - latency", &sql_script[i].stats.latency);
+ if (num_scripts > 1)
+ printSimpleStats(" - latency", &sql_script[i].stats.latency);
/* Report per-command latencies */
if (is_latencies)
@@ -2887,7 +2959,7 @@ main(int argc, char **argv)
instr_time conn_total_time;
int64 latency_late = 0;
StatsData stats;
- char *desc;
+ int weight;
int i;
int nclients_dealt;
@@ -2935,6 +3007,9 @@ main(int argc, char **argv)
while ((c = getopt_long(argc, argv, "ih:nvp:dqb:SNc:j:Crs:t:T:U:lf:D:F:M:P:R:L:", long_options, &optindex)) != -1)
{
+ char *script;
+ script_t *bi;
+
switch (c)
{
case 'i':
@@ -3066,27 +3141,29 @@ main(int argc, char **argv)
exit(0);
}
- addScript(desc,
- process_builtin(findBuiltin(optarg, &desc), desc));
+ weight = parseScriptWeight(optarg, &script);
+ bi = findBuiltin(script);
+ addScript(bi->desc, process_builtin(bi->commands, bi->desc),
+ weight);
benchmarking_option_set = true;
internal_script_used = true;
break;
+
case 'S':
- addScript(desc,
- process_builtin(findBuiltin("select-only", &desc),
- desc));
+ bi = findBuiltin("select-only");
+ addScript(bi->desc, process_builtin(bi->commands, bi->desc), 1);
benchmarking_option_set = true;
internal_script_used = true;
break;
case 'N':
- addScript(desc,
- process_builtin(findBuiltin("simple-update", &desc),
- desc));
+ bi = findBuiltin("simple-update");
+ addScript(bi->desc, process_builtin(bi->commands, bi->desc), 1);
benchmarking_option_set = true;
internal_script_used = true;
break;
case 'f':
- addScript(optarg, process_file(optarg));
+ weight = parseScriptWeight(optarg, &script);
+ addScript(script, process_file(script), weight);
benchmarking_option_set = true;
break;
case 'D':
@@ -3224,12 +3301,25 @@ main(int argc, char **argv)
/* set default script if none */
if (num_scripts == 0 && !is_init_mode)
{
- addScript(desc,
- process_builtin(findBuiltin("tpcb-like", &desc), desc));
+ script_t *bi = findBuiltin("tpcb-like");
+ addScript(bi->desc, process_builtin(bi->commands, bi->desc), 1);
benchmarking_option_set = true;
internal_script_used = true;
}
+ /* compute total_weight */
+ for (i = 0; i < num_scripts; i++)
+ {
+ total_weight += sql_script[i].weight;
+
+ /* detect overflow... */
+ if (total_weight < 0)
+ {
+ fprintf(stderr, "script weight overflow at script %d\n", i+1);
+ exit(1);
+ }
+ }
+
/* show per script stats if several scripts are used */
if (num_scripts > 1)
per_script_stats = true;
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 780a520..82e217a 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -271,6 +271,9 @@ pgbench <optional> <replaceable>options</> </optional> <replaceable>dbname</>
probability of drawing the script. If not specified, it is set to 1.
Available builtin scripts are: <literal>tpcb-like</>,
<literal>simple-update</> and <literal>select-only</>.
+ The provided <replaceable>scriptname</> needs only be an unambiguous
+ prefix of the builtin name, hence <literal>si</> would be enough to
+ select <literal>simple-update</>.
With special name <literal>list</>, show the list of builtin scripts
and exit immediately.
</para>
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index da4f05c..e394c1d 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -2681,20 +2681,33 @@ listAvailableScripts(void)
fprintf(stderr, "\n");
}
-/* return builtin script "name", or fail if not found */
+/* return commands for selected builtin script, if unambiguous */
static script_t *
findBuiltin(const char *name)
{
- int i;
+ int i, found = 0, len = strlen(name);
+ script_t *script = NULL;
for (i = 0; i < N_BUILTIN; i++)
{
- if (strncmp(builtin_script[i].name, name,
- strlen(builtin_script[i].name)) == 0)
- return & builtin_script[i];
+ if (strncmp(builtin_script[i].name, name, len) == 0)
+ {
+ script = & builtin_script[i];
+ found++;
+ }
}
- fprintf(stderr, "no builtin script found for name \"%s\"\n", name);
+ /* ok, unambiguous result */
+ if (found == 1)
+ return script;
+
+ /* error cases */
+ if (found == 0)
+ fprintf(stderr, "no builtin script found for name \"%s\"\n", name);
+ else /* found > 1 */
+ fprintf(stderr,
+ "%d builtin scripts found for prefix \"%s\"\n", found, name);
+
listAvailableScripts();
exit(1);
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
