On Mon, Nov 2, 2020 at 2:23 PM Magnus Hagander <[email protected]> wrote:
>
> On Tue, Oct 27, 2020 at 12:40 PM Bruce Momjian <[email protected]> wrote:
> >
> > On Tue, Oct 27, 2020 at 12:35:56PM +0100, Peter Eisentraut wrote:
> > > On 2020-10-27 11:53, Bruce Momjian wrote:
> > > > On Tue, Oct 27, 2020 at 11:35:25AM +0100, Peter Eisentraut wrote:
> > > > > On 2020-10-06 12:26, Magnus Hagander wrote:
> > > > > > I went with the name --no-instructions to have the same name for
> > > > > > both
> > > > > > initdb and pg_upgrade. The downside is that "no-instructions" also
> > > > > > causes the scripts not to be written in pg_upgrade, which arguably
> > > > > > is a
> > > > > > different thing. We could go with "--no-instructions" and
> > > > > > "--no-scripts", but that would leave the parameters different. I
> > > > > > also
> > > > > > considered "--no-next-step", but that one didn't quite have the
> > > > > > right
> > > > > > ring to me. I'm happy for other suggestions on the parameter names.
> > > > >
> > > > > What scripts are left after we remove the analyze script, as
> > > > > discussed in a
> > > > > different thread?
> > > >
> > > > There is still delete_old_cluster.sh.
> > >
> > > Well, that one can trivially be replaced by a printed instruction, too.
> >
> > True. On my system is it simply:
> >
> > rm -rf '/u/pgsql.old/data'
> >
> > The question is whether the user is going to record the vacuumdb and rm
> > instructions that display at the end of the pg_upgrade run, or do we
> > need to write it down for them in script files.
>
> That assumes for example that you've had no extra tablespaces defined
> in it. And it assumes your config files are actually in the same
> directory etc.
>
> Now, pg_upgrade *could* create a script that "actually works" for most
> things, since it connects to the system and could then enumerate
> things like tablespaces and config file locations, and generate a
> script that actually uses that information.
>
> But getting that to cover things like removing/disabling systemd
> services or init jobs or whatever the platform uses can quickly get
> pretty complex I think...
>
> I wonder if we should just not do it and just say "you should delete
> the old cluster". Then we can leave it up to platform integrations to
> enhance that, based on their platform knowledge (such as for example
> knowing if the platform runs systemd or not). That would leave us with
> both pg_upgrade and initdb printing instructions, and not scripts, and
> solve that part of the issue.
>
> Thinking further, there has been one other thing that we've talked
> about before, which is to have pg_upgrade either run extension
> upgrades, or generate a script that would run extension upgrades. If
> we want to do that as form of a script, then we're bringing the
> scripts back into play again... But maybe that's actually an argument
> for *not* having pg_upgrade generate that script, but instead either
> do the extension upgrades automatically, or have a separate tool that
> one can run independent of pg_upgrade...
PFA a rebased version of this patch on top of what has happened since,
and changing the pg_upgrade parameter to be --no-scripts.
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
diff --git a/doc/src/sgml/ref/initdb.sgml b/doc/src/sgml/ref/initdb.sgml
index 385ac25150..995d78408e 100644
--- a/doc/src/sgml/ref/initdb.sgml
+++ b/doc/src/sgml/ref/initdb.sgml
@@ -275,6 +275,19 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--no-instructions</option></term>
+ <listitem>
+ <para>
+ By default, <command>initdb</command> will write instructions for how
+ to start the cluster at the end of its output. This option causes
+ those instructions to be left out. This is primarily intended for use
+ by tools that wrap <command>initdb</command> in platform specific
+ behavior, where those instructions are likely to be incorrect.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--pwfile=<replaceable>filename</replaceable></option></term>
<listitem>
diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml
index 92e1d09a55..6e70b8514d 100644
--- a/doc/src/sgml/ref/pgupgrade.sgml
+++ b/doc/src/sgml/ref/pgupgrade.sgml
@@ -230,6 +230,20 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--no-scripts</option></term>
+ <listitem>
+ <para>
+ By default, <command>pg_upgrade</command> will write instructions and
+ scripts for how to delete the old cluster at the end of its
+ output. This option causes those scripts and instructions to be left
+ out. This is primarily intended for use by tools that wrap
+ <command>pg_upgrade</command> in platform specific behavior, where
+ those instructions are likely to be incorrect.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-?</option></term>
<term><option>--help</option></term>
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index ee3bfa82f4..c1f742b1b4 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -139,6 +139,7 @@ static const char *authmethodhost = NULL;
static const char *authmethodlocal = NULL;
static bool debug = false;
static bool noclean = false;
+static bool noinstructions = false;
static bool do_sync = true;
static bool sync_only = false;
static bool show_setting = false;
@@ -2294,6 +2295,7 @@ usage(const char *progname)
printf(_(" -L DIRECTORY where to find the input files\n"));
printf(_(" -n, --no-clean do not clean up after errors\n"));
printf(_(" -N, --no-sync do not wait for changes to be written safely to disk\n"));
+ printf(_(" --no-instructions do not print instructions for next steps\n"));
printf(_(" -s, --show show internal settings\n"));
printf(_(" -S, --sync-only only sync data directory\n"));
printf(_("\nOther options:\n"));
@@ -2953,6 +2955,7 @@ main(int argc, char *argv[])
{"no-clean", no_argument, NULL, 'n'},
{"nosync", no_argument, NULL, 'N'}, /* for backwards compatibility */
{"no-sync", no_argument, NULL, 'N'},
+ {"no-instructions", no_argument, NULL, 13},
{"sync-only", no_argument, NULL, 'S'},
{"waldir", required_argument, NULL, 'X'},
{"wal-segsize", required_argument, NULL, 12},
@@ -3093,6 +3096,9 @@ main(int argc, char *argv[])
case 12:
str_wal_segment_size_mb = pg_strdup(optarg);
break;
+ case 13:
+ noinstructions = true;
+ break;
case 'g':
SetDataDirectoryCreatePerm(PG_DIR_MODE_GROUP);
break;
@@ -3243,34 +3249,40 @@ main(int argc, char *argv[])
"--auth-local and --auth-host, the next time you run initdb.\n"));
}
- /*
- * Build up a shell command to tell the user how to start the server
- */
- start_db_cmd = createPQExpBuffer();
+ if (!noinstructions)
+ {
+ /*
+ * Build up a shell command to tell the user how to start the server
+ */
+ start_db_cmd = createPQExpBuffer();
+
+ /* Get directory specification used to start initdb ... */
+ strlcpy(pg_ctl_path, argv[0], sizeof(pg_ctl_path));
+ canonicalize_path(pg_ctl_path);
+ get_parent_directory(pg_ctl_path);
+ /* ... and tag on pg_ctl instead */
+ join_path_components(pg_ctl_path, pg_ctl_path, "pg_ctl");
- /* Get directory specification used to start initdb ... */
- strlcpy(pg_ctl_path, argv[0], sizeof(pg_ctl_path));
- canonicalize_path(pg_ctl_path);
- get_parent_directory(pg_ctl_path);
- /* ... and tag on pg_ctl instead */
- join_path_components(pg_ctl_path, pg_ctl_path, "pg_ctl");
+ /* path to pg_ctl, properly quoted */
+ appendShellString(start_db_cmd, pg_ctl_path);
- /* path to pg_ctl, properly quoted */
- appendShellString(start_db_cmd, pg_ctl_path);
+ /* add -D switch, with properly quoted data directory */
+ appendPQExpBufferStr(start_db_cmd, " -D ");
+ appendShellString(start_db_cmd, pgdata_native);
- /* add -D switch, with properly quoted data directory */
- appendPQExpBufferStr(start_db_cmd, " -D ");
- appendShellString(start_db_cmd, pgdata_native);
+ /* add suggested -l switch and "start" command */
+ /* translator: This is a placeholder in a shell command. */
+ appendPQExpBuffer(start_db_cmd, " -l %s start", _("logfile"));
- /* add suggested -l switch and "start" command */
- /* translator: This is a placeholder in a shell command. */
- appendPQExpBuffer(start_db_cmd, " -l %s start", _("logfile"));
+ printf(_("\nSuccess. You can now start the database server using:\n\n"
+ " %s\n\n"),
+ start_db_cmd->data);
- printf(_("\nSuccess. You can now start the database server using:\n\n"
- " %s\n\n"),
- start_db_cmd->data);
+ destroyPQExpBuffer(start_db_cmd);
+ }
+ else
+ printf(_("\nSuccess.\n"));
- destroyPQExpBuffer(start_db_cmd);
success = true;
return 0;
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 548d648e8c..f1f3b7833d 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -57,6 +57,7 @@ parseCommandLine(int argc, char *argv[])
{"verbose", no_argument, NULL, 'v'},
{"clone", no_argument, NULL, 1},
{"index-collation-versions-unknown", no_argument, NULL, 2},
+ {"no-scripts", no_argument, NULL, 3},
{NULL, 0, NULL, 0}
};
@@ -68,6 +69,7 @@ parseCommandLine(int argc, char *argv[])
time_t run_time = time(NULL);
user_opts.transfer_mode = TRANSFER_MODE_COPY;
+ user_opts.noscripts = false;
os_info.progname = get_progname(argv[0]);
@@ -208,6 +210,10 @@ parseCommandLine(int argc, char *argv[])
user_opts.ind_coll_unknown = true;
break;
+ case 3:
+ user_opts.noscripts = true;
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
os_info.progname);
@@ -314,6 +320,7 @@ usage(void)
printf(_(" --clone clone instead of copying files to new cluster\n"));
printf(_(" --index-collation-versions-unknown\n"));
printf(_(" mark text indexes as needing to be rebuilt\n"));
+ printf(_(" --no-scripts do not generate scripts for next steps\n"));
printf(_(" -?, --help show this help, then exit\n"));
printf(_("\n"
"Before running pg_upgrade you must:\n"
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index e2253ecd5d..b653e7ce02 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -175,7 +175,10 @@ main(int argc, char **argv)
new_cluster.pgdata);
check_ok();
- create_script_for_old_cluster_deletion(&deletion_script_file_name);
+ if (!user_opts.noscripts)
+ {
+ create_script_for_old_cluster_deletion(&deletion_script_file_name);
+ }
issue_warnings_and_set_wal_level();
@@ -184,7 +187,10 @@ main(int argc, char **argv)
"Upgrade Complete\n"
"----------------\n");
- output_completion_banner(deletion_script_file_name);
+ if (!user_opts.noscripts)
+ {
+ output_completion_banner(deletion_script_file_name);
+ }
pg_free(deletion_script_file_name);
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index ee70243c2e..542bcd1f5e 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -293,6 +293,7 @@ typedef struct
int jobs; /* number of processes/threads to use */
char *socketdir; /* directory to use for Unix sockets */
bool ind_coll_unknown; /* mark unknown index collation versions */
+ bool noscripts; /* don't generate scripts for next steps */
} UserOpts;
typedef struct
