Hello Alvaro,

If somebody specifies thousands of -f switches, they will waste a few
bytes with each, but I'm hardly concerned about a few dozen kilobytes
there ...

Ok, so you prefer a memory leak. I hate it on principle.

I don't "prefer" memory leaks -- I prefer interfaces that make sense.

C is not designed to return two things, and if it is what is needed it looks awkward whatever is done. The static variable trick is dirty, but it is the minimal fuss solution, IMO. So we are only trading awkward code against awkward code.

Speaking of which, I don't think the arrangement in your patch really
does.  I know I suggested it,

Yep:-)

but now that I look again, it turns out I chose badly and you implemented a bad idea, so can we go back and fix it, please?

Yep.

I have very little time available, so I'm trying to minimize the effort. I've tried "argue my point with committers", but it has proven very ineffective. I've switched to "do whatever is asked if it still works", but it is not very effective either.

What I now think should really happen is that the current sql_scripts
array, currently under an anonymous struct, should be a typedef, say
ParsedScript,

Why not.

and get a new member for the weight;

Hm... it already contains "weight".

process_file and process_builtin return a ParsedScript. The weight and Command ** should not be part of script_t at all.

Sure.

In fact, with ParsedScript I don't think we need to give a name to the anon struct used for builtin scripts.

It is useful that it has a name so that find_builtin can return it.

Rename the current sql_scripts.name to "desc", to mirror what
is actually put in there from the builtin array struct.  Make addScript
receive a ParsedScript and weight, fill in the weight into the struct,
and put it to the array after sanity-checking.  (I'm OK with keeping
"name" instead of renaming to "desc", if that change becomes too
invasive.)

See attached a v24 & v25.

The awkwardness in v24 is that functions allocate a struct which is freed afterwards, really just to return two data. Whether it is better or worst than a static is really a matter of taste.

Version v25 results a script which is then passed as an argument, so it avoid the dynamic allocation & later free. Maybe it is better. I had to cut short the error handling if a file does not exists, though, and it passes a struct by value.

Feel free to pick whichever you like most.

No need for N_BUILTIN; we can use lengthof(builtin_script) instead.

Indeed. "lengthof" does not seem to be standard... ok, it is a macro in some header file. I really wanted to avoid an ugly sizeof divide hack, but as it is hidden elsewhere this is fine.

--
Fabien.
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index cc80b3f..dd3fb1d 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</>.
         Unambiguous prefixes of builtin names are accepted.
@@ -322,12 +324,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>
@@ -687,9 +691,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>
@@ -1194,12 +1202,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: &lt;builtin: TPC-B (sort of)&gt;
- - 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 5a3c6cd..04deb2c 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>
@@ -180,6 +181,8 @@ char	   *login = NULL;
 char	   *dbName;
 const char *progname;
 
+#define WSEP '@'				/* weight separator */
+
 volatile bool timer_exceeded = false;	/* flag from signal handler */
 
 /* variable definitions */
@@ -298,26 +301,30 @@ typedef struct
 	SimpleStats stats;			/* time spent in this command */
 } Command;
 
-static struct
+typedef struct
 {
-	const char *name;
+	const char *desc;
+	int			weight;
 	Command   **commands;
-	StatsData stats;
-}	sql_script[MAX_SCRIPTS];	/* SQL script files */
+	StatsData	stats;
+} ParsedScript;
+
+static ParsedScript 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 */
-}
+	char	   *script;			/* actual pgbench script */
+} script_t;
 
-			builtin_script[] =
+static script_t builtin_script[] =
 {
 	{
 		"tpcb-like",
@@ -393,9 +400,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"
@@ -1313,13 +1320,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 */
@@ -1493,7 +1510,7 @@ top:
 			commands = sql_script[st->use_file].commands;
 			if (debug)
 				fprintf(stderr, "client %d executing script \"%s\"\n", st->id,
-						sql_script[st->use_file].name);
+						sql_script[st->use_file].desc);
 			st->is_throttled = false;
 
 			/*
@@ -2625,34 +2642,36 @@ read_line_from_file(FILE *fd)
 }
 
 /*
- * Given a file name, read it and return the array of Commands contained
- * therein.  "-" means to read stdin.
+ * Given a file name, read it and return a script_t with a description &
+ * an array of commands.  "-" means to read stdin.
  */
-static Command **
+static ParsedScript *
 process_file(char *filename)
 {
 #define COMMANDS_ALLOC_NUM 128
 
-	Command   **my_commands;
+	ParsedScript * ps;
+
 	FILE	   *fd;
 	int			lineno,
 				index;
 	char	   *buf;
 	int			alloc_num;
 
-	alloc_num = COMMANDS_ALLOC_NUM;
-	my_commands = (Command **) pg_malloc(sizeof(Command *) * alloc_num);
-
 	if (strcmp(filename, "-") == 0)
 		fd = stdin;
 	else if ((fd = fopen(filename, "r")) == NULL)
 	{
 		fprintf(stderr, "could not open file \"%s\": %s\n",
 				filename, strerror(errno));
-		pg_free(my_commands);
 		return NULL;
 	}
 
+	alloc_num = COMMANDS_ALLOC_NUM;
+	ps = pg_malloc(sizeof(ParsedScript));
+	ps->commands = (Command **) pg_malloc(sizeof(Command *) * alloc_num);
+	ps->desc = filename;
+
 	lineno = 0;
 	index = 0;
 
@@ -2669,35 +2688,40 @@ process_file(char *filename)
 		if (command == NULL)
 			continue;
 
-		my_commands[index] = command;
+		ps->commands[index] = command;
 		index++;
 
 		if (index >= alloc_num)
 		{
 			alloc_num += COMMANDS_ALLOC_NUM;
-			my_commands = pg_realloc(my_commands, sizeof(Command *) * alloc_num);
+			ps->commands = pg_realloc(ps->commands,
+									  sizeof(Command *) * alloc_num);
 		}
 	}
 	fclose(fd);
 
-	my_commands[index] = NULL;
+	ps->commands[index] = NULL;
 
-	return my_commands;
+	return ps;
 }
 
-static Command **
-process_builtin(const char *tb, const char *source)
+/* process builtin script bi by adding the array of commands and returning it */
+static ParsedScript *
+process_builtin(script_t * bi)
 {
 #define COMMANDS_ALLOC_NUM 128
 
-	Command   **my_commands;
 	int			lineno,
 				index;
 	char		buf[BUFSIZ];
 	int			alloc_num;
+	char	   *tb = bi->script;
+	ParsedScript *ps;
 
 	alloc_num = COMMANDS_ALLOC_NUM;
-	my_commands = (Command **) pg_malloc(sizeof(Command *) * alloc_num);
+	ps = (ParsedScript *) pg_malloc(sizeof(ParsedScript));
+	ps->desc = bi->desc;
+	ps->commands = (Command **) pg_malloc(sizeof(Command *) * alloc_num);
 
 	lineno = 0;
 	index = 0;
@@ -2707,6 +2731,7 @@ process_builtin(const char *tb, const char *source)
 		char	   *p;
 		Command    *command;
 
+		/* buffer overflow check? */
 		p = buf;
 		while (*tb && *tb != '\n')
 			*p++ = *tb++;
@@ -2721,58 +2746,59 @@ process_builtin(const char *tb, const char *source)
 
 		lineno += 1;
 
-		command = process_commands(buf, source, lineno);
+		command = process_commands(buf, bi->desc, lineno);
 		if (command == NULL)
 			continue;
 
-		my_commands[index] = command;
+		ps->commands[index] = command;
 		index++;
 
 		if (index >= alloc_num)
 		{
 			alloc_num += COMMANDS_ALLOC_NUM;
-			my_commands = pg_realloc(my_commands, sizeof(Command *) * alloc_num);
+			ps->commands =
+				pg_realloc(ps->commands, sizeof(Command *) * alloc_num);
 		}
 	}
 
-	my_commands[index] = NULL;
+	ps->commands[index] = NULL;
 
-	return my_commands;
+	return ps;
 }
 
+/* show available builtin scripts */
 static void
 listAvailableScripts(void)
 {
 	int			i;
 
 	fprintf(stderr, "Available builtin scripts:\n");
-	for (i = 0; i < N_BUILTIN; i++)
+	for (i = 0; i < lengthof(builtin_script); i++)
 		fprintf(stderr, "\t%s\n", builtin_script[i].name);
 	fprintf(stderr, "\n");
 }
 
-/* return builtin script "name" if unambiguous */
-static char *
-findBuiltin(const char *name, char **desc)
+/* return builtin script "name" if unambiguous, of fails if not found */
+static script_t *
+findBuiltin(const char *name)
 {
 	int			i,
 				found = 0,
 				len = strlen(name);
-	char	   *commands = NULL;
+	script_t   *result = NULL;
 
-	for (i = 0; i < N_BUILTIN; i++)
+	for (i = 0; i < lengthof(builtin_script); i++)
 	{
 		if (strncmp(builtin_script[i].name, name, len) == 0)
 		{
-			*desc = builtin_script[i].desc;
-			commands = builtin_script[i].commands;
+			result = & builtin_script[i];
 			found++;
 		}
 	}
 
 	/* ok, unambiguous result */
 	if (found == 1)
-		return commands;
+		return result;
 
 	/* error cases */
 	if (found == 0)
@@ -2785,13 +2811,61 @@ 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 scripts to process */
 static void
-addScript(const char *name, Command **commands)
+addScript(ParsedScript *script, int weight)
 {
-	if (commands == NULL ||
-		commands[0] == NULL)
+	if (script->commands == NULL || script->commands[0] == NULL)
 	{
-		fprintf(stderr, "empty command list for script \"%s\"\n", name);
+		fprintf(stderr, "empty command list for script \"%s\"\n", script->desc);
 		exit(1);
 	}
 
@@ -2801,10 +2875,13 @@ addScript(const char *name, Command **commands)
 		exit(1);
 	}
 
-	sql_script[num_scripts].name = name;
-	sql_script[num_scripts].commands = commands;
+	sql_script[num_scripts] = *script;
+	sql_script[num_scripts].weight = weight;
 	initStats(&sql_script[num_scripts].stats, 0.0);
 	num_scripts++;
+
+	/* was allocated in process_builtin or process_file */
+	free(script);
 }
 
 static void
@@ -2833,7 +2910,7 @@ printResults(TState *threads, StatsData *total, instr_time total_time,
 						(INSTR_TIME_GET_DOUBLE(conn_total_time) / nclients));
 
 	printf("transaction type: %s\n",
-		   num_scripts == 1 ? sql_script[0].name : "multiple scripts");
+		   num_scripts == 1 ? sql_script[0].desc : "multiple scripts");
 	printf("scaling factor: %d\n", scale);
 	printf("query mode: %s\n", QUERYMODE[querymode]);
 	printf("number of clients: %d\n", nclients);
@@ -2887,19 +2964,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].desc,
+					   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",
@@ -2907,7 +2989,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)
@@ -2990,7 +3073,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;
@@ -3038,6 +3121,8 @@ 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;
+
 		switch (c)
 		{
 			case 'i':
@@ -3169,27 +3254,25 @@ main(int argc, char **argv)
 					exit(0);
 				}
 
-				addScript(desc,
-						  process_builtin(findBuiltin(optarg, &desc), desc));
+				weight = parseScriptWeight(optarg, &script);
+				addScript(process_builtin(findBuiltin(script)), weight);
 				benchmarking_option_set = true;
 				internal_script_used = true;
 				break;
+
 			case 'S':
-				addScript(desc,
-						  process_builtin(findBuiltin("select-only", &desc),
-										  desc));
+				addScript(process_builtin(findBuiltin("select-only")), 1);
 				benchmarking_option_set = true;
 				internal_script_used = true;
 				break;
 			case 'N':
-				addScript(desc,
-						  process_builtin(findBuiltin("simple-update", &desc),
-										  desc));
+				addScript(process_builtin(findBuiltin("simple-update")), 1);
 				benchmarking_option_set = true;
 				internal_script_used = true;
 				break;
 			case 'f':
-				addScript(optarg, process_file(optarg));
+				weight = parseScriptWeight(optarg, &script);
+				addScript(process_file(script), weight);
 				benchmarking_option_set = true;
 				break;
 			case 'D':
@@ -3327,12 +3410,16 @@ 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));
+		addScript(process_builtin(findBuiltin("tpcb-like")), 1);
 		benchmarking_option_set = true;
 		internal_script_used = true;
 	}
 
+	/* compute total_weight */
+	for (i = 0; i < num_scripts; i++)
+		/* cannot overflow: weight is 32b, total_weight 64b */
+		total_weight += sql_script[i].weight;
+
 	/* show per script stats if several scripts are used */
 	if (num_scripts > 1)
 		per_script_stats = true;
@@ -3738,7 +3825,7 @@ threadRun(void *arg)
 		commands = sql_script[st->use_file].commands;
 		if (debug)
 			fprintf(stderr, "client %d executing script \"%s\"\n", st->id,
-					sql_script[st->use_file].name);
+					sql_script[st->use_file].desc);
 		if (!doCustom(thread, st, &aggs))
 			remains--;			/* I've aborted */
 
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index cc80b3f..dd3fb1d 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</>.
         Unambiguous prefixes of builtin names are accepted.
@@ -322,12 +324,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>
@@ -687,9 +691,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>
@@ -1194,12 +1202,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: &lt;builtin: TPC-B (sort of)&gt;
- - 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 5a3c6cd..a32db2b 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>
@@ -180,6 +181,8 @@ char	   *login = NULL;
 char	   *dbName;
 const char *progname;
 
+#define WSEP '@'				/* weight separator */
+
 volatile bool timer_exceeded = false;	/* flag from signal handler */
 
 /* variable definitions */
@@ -298,26 +301,30 @@ typedef struct
 	SimpleStats stats;			/* time spent in this command */
 } Command;
 
-static struct
+typedef struct
 {
-	const char *name;
+	const char *desc;
+	int			weight;
 	Command   **commands;
-	StatsData stats;
-}	sql_script[MAX_SCRIPTS];	/* SQL script files */
+	StatsData	stats;
+} ParsedScript;
+
+static ParsedScript 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 */
-}
+	char	   *script;			/* actual pgbench script */
+} script_t;
 
-			builtin_script[] =
+static script_t builtin_script[] =
 {
 	{
 		"tpcb-like",
@@ -393,9 +400,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"
@@ -1313,13 +1320,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 */
@@ -1493,7 +1510,7 @@ top:
 			commands = sql_script[st->use_file].commands;
 			if (debug)
 				fprintf(stderr, "client %d executing script \"%s\"\n", st->id,
-						sql_script[st->use_file].name);
+						sql_script[st->use_file].desc);
 			st->is_throttled = false;
 
 			/*
@@ -2625,34 +2642,35 @@ read_line_from_file(FILE *fd)
 }
 
 /*
- * Given a file name, read it and return the array of Commands contained
- * therein.  "-" means to read stdin.
+ * Given a file name, read it and return a script_t with a description &
+ * an array of commands.  "-" means to read stdin.
  */
-static Command **
+static ParsedScript
 process_file(char *filename)
 {
 #define COMMANDS_ALLOC_NUM 128
 
-	Command   **my_commands;
+	ParsedScript ps;
+
 	FILE	   *fd;
 	int			lineno,
 				index;
 	char	   *buf;
 	int			alloc_num;
 
-	alloc_num = COMMANDS_ALLOC_NUM;
-	my_commands = (Command **) pg_malloc(sizeof(Command *) * alloc_num);
-
 	if (strcmp(filename, "-") == 0)
 		fd = stdin;
 	else if ((fd = fopen(filename, "r")) == NULL)
 	{
 		fprintf(stderr, "could not open file \"%s\": %s\n",
 				filename, strerror(errno));
-		pg_free(my_commands);
-		return NULL;
+		exit(1);
 	}
 
+	alloc_num = COMMANDS_ALLOC_NUM;
+	ps.commands = (Command **) pg_malloc(sizeof(Command *) * alloc_num);
+	ps.desc = filename;
+
 	lineno = 0;
 	index = 0;
 
@@ -2669,35 +2687,38 @@ process_file(char *filename)
 		if (command == NULL)
 			continue;
 
-		my_commands[index] = command;
+		ps.commands[index] = command;
 		index++;
 
 		if (index >= alloc_num)
 		{
 			alloc_num += COMMANDS_ALLOC_NUM;
-			my_commands = pg_realloc(my_commands, sizeof(Command *) * alloc_num);
+			ps.commands = pg_realloc(ps.commands, sizeof(Command *) * alloc_num);
 		}
 	}
 	fclose(fd);
 
-	my_commands[index] = NULL;
+	ps.commands[index] = NULL;
 
-	return my_commands;
+	return ps;
 }
 
-static Command **
-process_builtin(const char *tb, const char *source)
+/* process builtin script bi by adding the array of commands and returning it */
+static ParsedScript
+process_builtin(script_t * bi)
 {
 #define COMMANDS_ALLOC_NUM 128
 
-	Command   **my_commands;
 	int			lineno,
 				index;
 	char		buf[BUFSIZ];
 	int			alloc_num;
+	char	   *tb = bi->script;
+	ParsedScript ps;
 
 	alloc_num = COMMANDS_ALLOC_NUM;
-	my_commands = (Command **) pg_malloc(sizeof(Command *) * alloc_num);
+	ps.desc = bi->desc;
+	ps.commands = (Command **) pg_malloc(sizeof(Command *) * alloc_num);
 
 	lineno = 0;
 	index = 0;
@@ -2707,6 +2728,7 @@ process_builtin(const char *tb, const char *source)
 		char	   *p;
 		Command    *command;
 
+		/* buffer overflow check? */
 		p = buf;
 		while (*tb && *tb != '\n')
 			*p++ = *tb++;
@@ -2721,58 +2743,58 @@ process_builtin(const char *tb, const char *source)
 
 		lineno += 1;
 
-		command = process_commands(buf, source, lineno);
+		command = process_commands(buf, bi->desc, lineno);
 		if (command == NULL)
 			continue;
 
-		my_commands[index] = command;
+		ps.commands[index] = command;
 		index++;
 
 		if (index >= alloc_num)
 		{
 			alloc_num += COMMANDS_ALLOC_NUM;
-			my_commands = pg_realloc(my_commands, sizeof(Command *) * alloc_num);
+			ps.commands = pg_realloc(ps.commands, sizeof(Command *) * alloc_num);
 		}
 	}
 
-	my_commands[index] = NULL;
+	ps.commands[index] = NULL;
 
-	return my_commands;
+	return ps;
 }
 
+/* show available builtin scripts */
 static void
 listAvailableScripts(void)
 {
 	int			i;
 
 	fprintf(stderr, "Available builtin scripts:\n");
-	for (i = 0; i < N_BUILTIN; i++)
+	for (i = 0; i < lengthof(builtin_script); i++)
 		fprintf(stderr, "\t%s\n", builtin_script[i].name);
 	fprintf(stderr, "\n");
 }
 
-/* return builtin script "name" if unambiguous */
-static char *
-findBuiltin(const char *name, char **desc)
+/* return builtin script "name" if unambiguous, of fails if not found */
+static script_t *
+findBuiltin(const char *name)
 {
 	int			i,
 				found = 0,
 				len = strlen(name);
-	char	   *commands = NULL;
+	script_t   *result = NULL;
 
-	for (i = 0; i < N_BUILTIN; i++)
+	for (i = 0; i < lengthof(builtin_script); i++)
 	{
 		if (strncmp(builtin_script[i].name, name, len) == 0)
 		{
-			*desc = builtin_script[i].desc;
-			commands = builtin_script[i].commands;
+			result = & builtin_script[i];
 			found++;
 		}
 	}
 
 	/* ok, unambiguous result */
 	if (found == 1)
-		return commands;
+		return result;
 
 	/* error cases */
 	if (found == 0)
@@ -2785,13 +2807,61 @@ 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 scripts to process */
 static void
-addScript(const char *name, Command **commands)
+addScript(ParsedScript script, int weight)
 {
-	if (commands == NULL ||
-		commands[0] == NULL)
+	if (script.commands == NULL || script.commands[0] == NULL)
 	{
-		fprintf(stderr, "empty command list for script \"%s\"\n", name);
+		fprintf(stderr, "empty command list for script \"%s\"\n", script.desc);
 		exit(1);
 	}
 
@@ -2801,8 +2871,8 @@ addScript(const char *name, Command **commands)
 		exit(1);
 	}
 
-	sql_script[num_scripts].name = name;
-	sql_script[num_scripts].commands = commands;
+	sql_script[num_scripts] = script;
+	sql_script[num_scripts].weight = weight;
 	initStats(&sql_script[num_scripts].stats, 0.0);
 	num_scripts++;
 }
@@ -2833,7 +2903,7 @@ printResults(TState *threads, StatsData *total, instr_time total_time,
 						(INSTR_TIME_GET_DOUBLE(conn_total_time) / nclients));
 
 	printf("transaction type: %s\n",
-		   num_scripts == 1 ? sql_script[0].name : "multiple scripts");
+		   num_scripts == 1 ? sql_script[0].desc : "multiple scripts");
 	printf("scaling factor: %d\n", scale);
 	printf("query mode: %s\n", QUERYMODE[querymode]);
 	printf("number of clients: %d\n", nclients);
@@ -2887,19 +2957,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].desc,
+					   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",
@@ -2907,7 +2982,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)
@@ -2990,7 +3066,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;
@@ -3038,6 +3114,8 @@ 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;
+
 		switch (c)
 		{
 			case 'i':
@@ -3169,27 +3247,25 @@ main(int argc, char **argv)
 					exit(0);
 				}
 
-				addScript(desc,
-						  process_builtin(findBuiltin(optarg, &desc), desc));
+				weight = parseScriptWeight(optarg, &script);
+				addScript(process_builtin(findBuiltin(script)), weight);
 				benchmarking_option_set = true;
 				internal_script_used = true;
 				break;
+
 			case 'S':
-				addScript(desc,
-						  process_builtin(findBuiltin("select-only", &desc),
-										  desc));
+				addScript(process_builtin(findBuiltin("select-only")), 1);
 				benchmarking_option_set = true;
 				internal_script_used = true;
 				break;
 			case 'N':
-				addScript(desc,
-						  process_builtin(findBuiltin("simple-update", &desc),
-										  desc));
+				addScript(process_builtin(findBuiltin("simple-update")), 1);
 				benchmarking_option_set = true;
 				internal_script_used = true;
 				break;
 			case 'f':
-				addScript(optarg, process_file(optarg));
+				weight = parseScriptWeight(optarg, &script);
+				addScript(process_file(script), weight);
 				benchmarking_option_set = true;
 				break;
 			case 'D':
@@ -3327,12 +3403,16 @@ 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));
+		addScript(process_builtin(findBuiltin("tpcb-like")), 1);
 		benchmarking_option_set = true;
 		internal_script_used = true;
 	}
 
+	/* compute total_weight */
+	for (i = 0; i < num_scripts; i++)
+		/* cannot overflow: weight is 32b, total_weight 64b */
+		total_weight += sql_script[i].weight;
+
 	/* show per script stats if several scripts are used */
 	if (num_scripts > 1)
 		per_script_stats = true;
@@ -3738,7 +3818,7 @@ threadRun(void *arg)
 		commands = sql_script[st->use_file].commands;
 		if (debug)
 			fprintf(stderr, "client %d executing script \"%s\"\n", st->id,
-					sql_script[st->use_file].name);
+					sql_script[st->use_file].desc);
 		if (!doCustom(thread, st, &aggs))
 			remains--;			/* I've aborted */
 
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to