Hi, On 2/1/19 4:58 AM, Alvaro Herrera wrote:
On 2019-Feb-01, Jamison, Kirk wrote:I'm not sure if misunderstood the purpose of $VACUUMDB_OPTS. I thought what the other developers suggested is to utilize it so that --jobs for vacuumdb would be optional and just based from user-specified option $VACUUMDB_OPTS. By which it would not utilize the amount of jobs used for pg_upgrade. To address your need of needing a parallel, the script would just then suggest if the user wants a --jobs option. The if/else for number of jobs is not needed in that case, maybe only for ifndef WIN32 else case.No Kirk, you got it right -- (some of) those ifdefs are not needed, as adding the --jobs in the command line is not needed. I do think some extra whitespace in the format string is needed.
I'm confused by this.The point of the patch is to highlight to the user that even though he/she does "pg_upgrade -j 8" the "-j 8" option isn't passed down to vacuumdb.
Default value is 1, so in that case the echo command doesn't highlight that fact, otherwise it is. The user can then cancel the script and use the suggested command line.
The script then executes the vaccumdb command with the $VACUUMDB_OPTS argument as noted in the documentation.
Sample script attached as well. I added extra space in the --analyze-in-stages part. Kirk, can you provide a delta patch to match what you are thinking ? Best regards, Jesper
>From 4b5856725af5d971a26a2ba150c1067ee21bb242 Mon Sep 17 00:00:00 2001
From: jesperpedersen <jesper.peder...@redhat.com>
Date: Fri, 21 Dec 2018 05:05:31 -0500
Subject: [PATCH] Highlight that the --jobs option isn't passed down to
vacuumdb by default.
Author: Jesper Pedersen <jesper.peder...@redhat.com>
---
doc/src/sgml/ref/pgupgrade.sgml | 6 +++++-
src/bin/pg_upgrade/check.c | 28 ++++++++++++++++++++++------
2 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml
index 7e1afaf0a5..84a7ccb6f1 100644
--- a/doc/src/sgml/ref/pgupgrade.sgml
+++ b/doc/src/sgml/ref/pgupgrade.sgml
@@ -395,7 +395,11 @@ NET STOP postgresql-&majorversion;
in parallel; a good place to start is the maximum of the number of
CPU cores and tablespaces. This option can dramatically reduce the
time to upgrade a multi-database server running on a multiprocessor
- machine.
+ machine. Note that this option isn't passed to the
+ <application>vacuumdb</application> application by default.
+ The system environment variable <literal>VACUUMDB_OPTS</literal> can be
+ used to pass extra options to the <application>vacuumdb</application>
+ application during the script execution.
</para>
<para>
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index fc5aa7010f..570b7c9ae7 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -495,15 +495,31 @@ create_script_for_cluster_analyze(char **analyze_script_file_name)
ECHO_QUOTE, ECHO_QUOTE);
fprintf(script, "echo %sthis script and run:%s\n",
ECHO_QUOTE, ECHO_QUOTE);
- fprintf(script, "echo %s \"%s/vacuumdb\" %s--all %s%s\n", ECHO_QUOTE,
- new_cluster.bindir, user_specification.data,
- /* Did we copy the free space files? */
- (GET_MAJOR_VERSION(old_cluster.major_version) >= 804) ?
- "--analyze-only" : "--analyze", ECHO_QUOTE);
+ if (user_opts.jobs <= 1)
+ {
+ fprintf(script, "echo %s \"%s/vacuumdb\" %s--all %s%s\n", ECHO_QUOTE,
+ new_cluster.bindir, user_specification.data,
+ /* Did we copy the free space files? */
+ (GET_MAJOR_VERSION(old_cluster.major_version) >= 804) ?
+ "--analyze-only" : "--analyze", ECHO_QUOTE);
+ }
+ else
+ {
+ fprintf(script, "echo %s \"%s/vacuumdb\" %s--jobs %d --all %s%s\n", ECHO_QUOTE,
+ new_cluster.bindir, user_specification.data, user_opts.jobs,
+ /* Did we copy the free space files? */
+ (GET_MAJOR_VERSION(old_cluster.major_version) >= 804) ?
+ "--analyze-only" : "--analyze", ECHO_QUOTE);
+ }
fprintf(script, "echo%s\n\n", ECHO_BLANK);
- fprintf(script, "\"%s/vacuumdb\" %s--all --analyze-in-stages\n",
+#ifndef WIN32
+ fprintf(script, "\"%s/vacuumdb\" %s $VACUUMDB_OPTS --all --analyze-in-stages\n",
new_cluster.bindir, user_specification.data);
+#else
+ fprintf(script, "\"%s\\vacuumdb.exe\" %s %%VACUUMDB_OPTS%% --all --analyze-in-stages\n",
+ new_cluster.bindir, user_specification.data);
+#endif
/* Did we copy the free space files? */
if (GET_MAJOR_VERSION(old_cluster.major_version) < 804)
fprintf(script, "\"%s/vacuumdb\" %s--all\n", new_cluster.bindir,
--
2.17.2
analyze_new_cluster.sh
Description: application/shellscript
