Hello Masao-san,
The benefit of controlling where begin/end actually occur is that it may
have an impact on performance, and it allows to check that.
I still fail to understand the benefit of addition of () settings.
Could you clarify what case () settings are useful for? You are
thinking to execute all initialization SQL statements within
single transaction, e.g., -I (dtgp), for some reasons?
Yep. Or anything else, including without (), to allow checking the
performance impact or non impact of transactions on the initialization
phase.
When using ( and ) with the -I, the documentation should indicate that double
quotes are required,
Or single quotes, or backslash, if launch from the command line. I added a
mention of escaping or protection in the doc in that case.
What about using, for example, b (BEGIN) and c (COMMIT) instead
to avoid such restriction?
It is indeed possible. Using a open/close symmetric character ( (), {},
[]) looks more pleasing and allows to see easily whether everything is
properly closed. I switched to {} which does not generate the same quoting
issue in shell.
I think that it's better to check whehter "v" is enclosed with () or not
at the beginning of pgbench, and report an error if it is.
Otherwise, if -I (dtgv) is specified, pgbench reports an error after
time-consuming data generation is performed, and of course that data
generation is rollbacked.
Patch v5 attached added a check for v inside (), although I'm not keen on
putting it there, and uses {} instead of ().
--
Fabien.
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index e3a0abb4c7..e150209b4e 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -167,7 +167,7 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
<replaceable>init_steps</replaceable> specifies the
initialization steps to be performed, using one character per step.
Each step is invoked in the specified order.
- The default is <literal>dtgvp</literal>.
+ The default is <literal>dt{g}vp</literal>.
The available steps are:
<variablelist>
@@ -193,12 +193,34 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
</listitem>
</varlistentry>
<varlistentry>
- <term><literal>g</literal> (Generate data)</term>
+ <term><literal>{</literal> (begin transaction)</term>
+ <listitem>
+ <para>
+ Emit a <command>BEGIN</command>.
+ </para>
+ <para>
+ Beware that some steps may not work when called within an explicit transaction.
+ </para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><literal>g</literal> or <literal>G</literal> (Generate data, client or server side)</term>
<listitem>
<para>
Generate data and load it into the standard tables,
replacing any data already present.
</para>
+ <para>
+ When data is generated server side, there is no progress output.
+ </para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><literal>}</literal> (commit transaction)</term>
+ <listitem>
+ <para>
+ Emit a <command>COMMIT</command>.
+ </para>
</listitem>
</varlistentry>
<varlistentry>
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index e72ad0036e..be10630926 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -130,7 +130,13 @@ static int pthread_join(pthread_t th, void **thread_return);
/********************************************************************
* some configurable parameters */
-#define DEFAULT_INIT_STEPS "dtgvp" /* default -I setting */
+/*
+ * we do all data generation in one transaction to enable the backend's
+ * data-loading optimizations
+ */
+#define DEFAULT_INIT_STEPS "dt{g}vp" /* default -I setting */
+
+#define ALL_INIT_STEPS "dtgGvpf{}" /* all possible steps */
#define LOG_STEP_SECONDS 5 /* seconds between log messages */
#define DEFAULT_NXACTS 10 /* default nxacts */
@@ -626,7 +632,7 @@ usage(void)
" %s [OPTION]... [DBNAME]\n"
"\nInitialization options:\n"
" -i, --initialize invokes initialization mode\n"
- " -I, --init-steps=[dtgvpf]+ (default \"dtgvp\")\n"
+ " -I, --init-steps=[" ALL_INIT_STEPS "]+ (default \"" DEFAULT_INIT_STEPS "\")\n"
" run selected initialization steps\n"
" -F, --fillfactor=NUM set fill factor\n"
" -n, --no-vacuum do not run VACUUM during initialization\n"
@@ -3802,10 +3808,23 @@ append_fillfactor(char *opts, int len)
}
/*
- * Fill the standard tables with some data
+ * truncate away any old data, in one command in case there are foreign keys
*/
static void
-initGenerateData(PGconn *con)
+initTruncateTables(PGconn *con)
+{
+ executeStatement(con, "truncate table "
+ "pgbench_accounts, "
+ "pgbench_branches, "
+ "pgbench_history, "
+ "pgbench_tellers");
+}
+
+/*
+ * Fill the standard tables with some data, from the client side
+ */
+static void
+initGenerateDataClientSide(PGconn *con)
{
char sql[256];
PGresult *res;
@@ -3819,23 +3838,9 @@ initGenerateData(PGconn *con)
remaining_sec;
int log_interval = 1;
- fprintf(stderr, "generating data...\n");
+ fprintf(stderr, "generating data client side...\n");
- /*
- * we do all of this in one transaction to enable the backend's
- * data-loading optimizations
- */
- executeStatement(con, "begin");
-
- /*
- * truncate away any old data, in one command in case there are foreign
- * keys
- */
- executeStatement(con, "truncate table "
- "pgbench_accounts, "
- "pgbench_branches, "
- "pgbench_history, "
- "pgbench_tellers");
+ initTruncateTables(con);
/*
* fill branches, tellers, accounts in that order in case foreign keys
@@ -3935,8 +3940,41 @@ initGenerateData(PGconn *con)
fprintf(stderr, "PQendcopy failed\n");
exit(1);
}
+}
- executeStatement(con, "commit");
+/*
+ * Fill the standard tables with some data, from the server side
+ *
+ * As already the case with the client-side filling, the filler
+ * column defaults to NULL in pgbench_branches and pgbench_tellers,
+ * and is a blank-padded string in pgbench_accounts.
+ */
+static void
+initGenerateDataServerSide(PGconn *con)
+{
+ char sql[256];
+
+ fprintf(stderr, "generating data server side...\n");
+
+ initTruncateTables(con);
+
+ snprintf(sql, sizeof(sql),
+ "insert into pgbench_branches(bid,bbalance) "
+ "select bid, 0 "
+ "from generate_series(1, %d) as bid", scale);
+ executeStatement(con, sql);
+
+ snprintf(sql, sizeof(sql),
+ "insert into pgbench_tellers(tid,bid,tbalance) "
+ "select tid, (tid - 1) / %d + 1, 0 "
+ "from generate_series(1, %d) as tid", ntellers, scale * ntellers);
+ executeStatement(con, sql);
+
+ snprintf(sql, sizeof(sql),
+ "insert into pgbench_accounts(aid,bid,abalance,filler) "
+ "select aid, (aid - 1) / %d + 1, 0, '' "
+ "from generate_series(1, %d) as aid", naccounts, scale * naccounts);
+ executeStatement(con, sql);
}
/*
@@ -4020,6 +4058,7 @@ static void
checkInitSteps(const char *initialize_steps)
{
const char *step;
+ int begins = 0;
if (initialize_steps[0] == '\0')
{
@@ -4029,13 +4068,46 @@ checkInitSteps(const char *initialize_steps)
for (step = initialize_steps; *step != '\0'; step++)
{
- if (strchr("dtgvpf ", *step) == NULL)
+ if (strchr(ALL_INIT_STEPS " ", *step) == NULL)
{
- fprintf(stderr, "unrecognized initialization step \"%c\"\n",
+ fprintf(stderr,
+ "unrecognized initialization step \"%c\"\n"
+ "Allowed step characters are: \"" ALL_INIT_STEPS "\".\n",
*step);
- fprintf(stderr, "allowed steps are: \"d\", \"t\", \"g\", \"v\", \"p\", \"f\"\n");
exit(1);
}
+
+ if (*step == 'v' && begins > 0)
+ {
+ fprintf(stderr, "VACUUM cannot run inside a transaction block\n");
+ exit(1);
+ }
+
+ /* count BEGIN/COMMIT for matching */
+ if (*step == '{')
+ begins++;
+ else if (*step == '}')
+ begins--;
+
+ if (begins < 0)
+ {
+ fprintf(stderr, "COMMIT \"}\" at char %ld of \"%s\" does not have a matching BEGIN \"{\"\n",
+ step - initialize_steps, initialize_steps);
+ exit(1);
+ }
+ else if (begins >= 2)
+ {
+ fprintf(stderr, "BEGIN \"{\" within a BEGIN at char %ld of \"%s\", nested transactions are not supported\n",
+ step - initialize_steps, initialize_steps);
+ exit(1);
+ }
+ }
+
+ if (begins > 0)
+ {
+ fprintf(stderr, "a BEGIN \"{\" in \"%s\" does not have a matching COMMIT \"}\"\n",
+ initialize_steps);
+ exit(1);
}
}
@@ -4073,9 +4145,20 @@ runInitSteps(const char *initialize_steps)
op = "create tables";
initCreateTables(con);
break;
+ case '{':
+ executeStatement(con, "begin");
+ break;
case 'g':
- op = "generate";
- initGenerateData(con);
+ op = "client generate";
+ initGenerateDataClientSide(con);
+ break;
+ case 'G':
+ op = "server generate";
+ initGenerateDataServerSide(con);
+ break;
+ case '}':
+ op = "commit";
+ executeStatement(con, "commit");
break;
case 'v':
op = "vacuum";
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index c441626d7c..256b377e09 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -113,7 +113,7 @@ pgbench(
# Again, with all possible options
pgbench(
- '--initialize --init-steps=dtpvg --scale=1 --unlogged-tables --fillfactor=98 --foreign-keys --quiet --tablespace=regress_pgbench_tap_1_ts --index-tablespace=regress_pgbench_tap_1_ts --partitions=2 --partition-method=hash',
+ '--initialize --init-steps=dtpv{g} --scale=1 --unlogged-tables --fillfactor=98 --foreign-keys --quiet --tablespace=regress_pgbench_tap_1_ts --index-tablespace=regress_pgbench_tap_1_ts --partitions=2 --partition-method=hash',
0,
[qr{^$}i],
[
@@ -130,7 +130,7 @@ pgbench(
# Test interaction of --init-steps with legacy step-selection options
pgbench(
- '--initialize --init-steps=dtpvgvv --no-vacuum --foreign-keys --unlogged-tables --partitions=3',
+ '--initialize --init-steps=dtpv{G}vv --no-vacuum --foreign-keys --unlogged-tables --partitions=3',
0,
[qr{^$}],
[
@@ -138,7 +138,7 @@ pgbench(
qr{creating tables},
qr{creating 3 partitions},
qr{creating primary keys},
- qr{.* of .* tuples \(.*\) done},
+ qr{generating data server side},
qr{creating foreign keys},
qr{(?!vacuuming)}, # no vacuum
qr{done in \d+\.\d\d s }
diff --git a/src/bin/pgbench/t/002_pgbench_no_server.pl b/src/bin/pgbench/t/002_pgbench_no_server.pl
index 1e9542af3f..43b20ebcde 100644
--- a/src/bin/pgbench/t/002_pgbench_no_server.pl
+++ b/src/bin/pgbench/t/002_pgbench_no_server.pl
@@ -147,7 +147,27 @@ my @options = (
[
'invalid init step',
'-i -I dta',
- [ qr{unrecognized initialization step}, qr{allowed steps are} ]
+ [ qr{unrecognized initialization step}, qr{Allowed step characters are} ]
+ ],
+ [
+ 'invalid init step begin/begin',
+ '-i -I {{',
+ [ qr{nested transactions are not supported} ]
+ ],
+ [
+ 'invalid init step end',
+ '-i -I }',
+ [ qr{does not have a matching BEGIN} ]
+ ],
+ [
+ 'invalid init step begin',
+ '-i -I {dt',
+ [ qr{does not have a matching COMMIT} ]
+ ],
+ [
+ 'invalid vacuum inside transaction block',
+ '-i -I {v}',
+ [ qr{VACUUM cannot run inside a transaction block} ]
],
[
'bad random seed',
