Prevent printing "next step instructions" in initdb and pg_upgrade
The attached patch adds a switch --no-instructions to initdb and
pg_upgrade, which prevents them from printing instructions about how to
start the cluster (initdb) or how to analyze and delete clusters
(pg_upgrade).
The use case for this is for example the debian or redhat package wrappers.
When these commands are run under those wrappers the printed instructions
are *wrong*. It's better in that case to exclude them, and let the wrapper
be responsible for printing the correct instructions.
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.
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
Attachments:
no-instructions.patchtext/x-patch; charset=US-ASCII; name=no-instructions.patchDownload
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 b59c5697a3..b4d0a5e8f1 100644
--- a/doc/src/sgml/ref/pgupgrade.sgml
+++ b/doc/src/sgml/ref/pgupgrade.sgml
@@ -215,6 +215,20 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--no-instructions</option></term>
+ <listitem>
+ <para>
+ By default, <command>pg_upgrade</command> will write instructions and
+ scripts for how to analyze the new and 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..e6b5200ba5 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 don't 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 aca1ee8b48..21e8e0283f 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -56,6 +56,7 @@ parseCommandLine(int argc, char *argv[])
{"socketdir", required_argument, NULL, 's'},
{"verbose", no_argument, NULL, 'v'},
{"clone", no_argument, NULL, 1},
+ {"no-instructions", no_argument, NULL, 2},
{NULL, 0, NULL, 0}
};
@@ -67,6 +68,7 @@ parseCommandLine(int argc, char *argv[])
time_t run_time = time(NULL);
user_opts.transfer_mode = TRANSFER_MODE_COPY;
+ user_opts.noinstructions = false;
os_info.progname = get_progname(argv[0]);
@@ -203,6 +205,10 @@ parseCommandLine(int argc, char *argv[])
user_opts.transfer_mode = TRANSFER_MODE_CLONE;
break;
+ case 2:
+ user_opts.noinstructions = true;
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
os_info.progname);
@@ -307,6 +313,7 @@ usage(void)
printf(_(" -v, --verbose enable verbose internal logging\n"));
printf(_(" -V, --version display version information, then exit\n"));
printf(_(" --clone clone instead of copying files to new cluster\n"));
+ printf(_(" --no-instructions don't print instructions 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 70194eb096..e5be7e4b75 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -176,8 +176,11 @@ main(int argc, char **argv)
new_cluster.pgdata);
check_ok();
- create_script_for_cluster_analyze(&analyze_script_file_name);
- create_script_for_old_cluster_deletion(&deletion_script_file_name);
+ if (!user_opts.noinstructions)
+ {
+ create_script_for_cluster_analyze(&analyze_script_file_name);
+ create_script_for_old_cluster_deletion(&deletion_script_file_name);
+ }
issue_warnings_and_set_wal_level();
@@ -186,8 +189,11 @@ main(int argc, char **argv)
"Upgrade Complete\n"
"----------------\n");
- output_completion_banner(analyze_script_file_name,
- deletion_script_file_name);
+ if (!user_opts.noinstructions)
+ {
+ output_completion_banner(analyze_script_file_name,
+ deletion_script_file_name);
+ }
pg_free(analyze_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 8b90cefbe0..cedc500dd2 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -292,6 +292,7 @@ typedef struct
transferMode transfer_mode; /* copy files or link them? */
int jobs; /* number of processes/threads to use */
char *socketdir; /* directory to use for Unix sockets */
+ bool noinstructions; /* don't print instructions about next steps */
} UserOpts;
typedef struct
2020年10月6日(火) 19:26 Magnus Hagander <magnus@hagander.net>:
The attached patch adds a switch --no-instructions to initdb and pg_upgrade, which prevents them from printing instructions about how to start the cluster (initdb) or how to analyze and delete clusters (pg_upgrade).
The use case for this is for example the debian or redhat package wrappers. When these commands are run under those wrappers the printed instructions are *wrong*. It's better in that case to exclude them, and let the wrapper be responsible for printing the correct instructions.
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.
As the switches are doing slightly different things (just omitting verbiage
versus omitting verbiage *and* not generating some script files) it
seems reasonable
not to try and shoehorn them into a using a unified but ambiguous name
name. Particularly as they're intended to be used in scripts themselves, so
it's not like it's important to create something that users can remember
easily for frequent use.
Alternatively, rather than describing what is not being done, the switch
could be called "--script-mode" or similar with a description along the
lines of "produces output suitable for execution by packaging scripts"
or something, and detail what's being omitted (or done differently)
in the documentation page.
Regards
Ian Barwick
--
EnterpriseDB: https://www.enterprisedb.com
On Tue, Oct 6, 2020 at 1:49 PM Ian Lawrence Barwick <barwick@gmail.com>
wrote:
2020年10月6日(火) 19:26 Magnus Hagander <magnus@hagander.net>:
The attached patch adds a switch --no-instructions to initdb and
pg_upgrade, which prevents them from printing instructions about how to
start the cluster (initdb) or how to analyze and delete clusters
(pg_upgrade).The use case for this is for example the debian or redhat package
wrappers. When these commands are run under those wrappers the printed
instructions are *wrong*. It's better in that case to exclude them, and let
the wrapper be responsible for printing the correct instructions.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.As the switches are doing slightly different things (just omitting verbiage
versus omitting verbiage *and* not generating some script files) it
seems reasonable
not to try and shoehorn them into a using a unified but ambiguous name
name. Particularly as they're intended to be used in scripts themselves, so
it's not like it's important to create something that users can remember
easily for frequent use.
True.
Alternatively, rather than describing what is not being done, the switch
could be called "--script-mode" or similar with a description along the
lines of "produces output suitable for execution by packaging scripts"
or something, and detail what's being omitted (or done differently)
in the documentation page.
Hmm, I'm less sure about that one. One could argue that this should then
also affect other things, like password prompting, to fulfill it's name.
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
Magnus Hagander <magnus@hagander.net> writes:
The use case for this is for example the debian or redhat package wrappers.
When these commands are run under those wrappers the printed instructions
are *wrong*. It's better in that case to exclude them, and let the wrapper
be responsible for printing the correct instructions.
Hm, does it matter? I think those wrappers send the output to /dev/null
anyway.
regards, tom lane
On Tue, Oct 6, 2020 at 4:31 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
The use case for this is for example the debian or redhat package
wrappers.
When these commands are run under those wrappers the printed instructions
are *wrong*. It's better in that case to exclude them, and let thewrapper
be responsible for printing the correct instructions.
Hm, does it matter? I think those wrappers send the output to /dev/null
anyway.
The debian ones don't, because they consider it useful information to the
user. I'd say that it is, especially in the case of pg_upgrade. (Less so
about initdb, but still a bit -- especially when creating secondary
clusters)
The redhat ones I believe sent it to a logfile, not to /dev/null. Meaning
it's not in your face, but it still contains incorrect instructions.
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
Magnus Hagander <magnus@hagander.net> writes:
On Tue, Oct 6, 2020 at 4:31 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Hm, does it matter? I think those wrappers send the output to /dev/null
anyway.
The debian ones don't, because they consider it useful information to the
user. I'd say that it is, especially in the case of pg_upgrade. (Less so
about initdb, but still a bit -- especially when creating secondary
clusters)
The redhat ones I believe sent it to a logfile, not to /dev/null. Meaning
it's not in your face, but it still contains incorrect instructions.
OK. FWIW, I'd vote for separate --no-instructions and --no-scripts
switches.
regards, tom lane
On Tue, Oct 6, 2020 at 11:06:13AM -0400, Tom Lane wrote:
Magnus Hagander <magnus@hagander.net> writes:
On Tue, Oct 6, 2020 at 4:31 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Hm, does it matter? I think those wrappers send the output to /dev/null
anyway.The debian ones don't, because they consider it useful information to the
user. I'd say that it is, especially in the case of pg_upgrade. (Less so
about initdb, but still a bit -- especially when creating secondary
clusters)
The redhat ones I believe sent it to a logfile, not to /dev/null. Meaning
it's not in your face, but it still contains incorrect instructions.OK. FWIW, I'd vote for separate --no-instructions and --no-scripts
switches.
Works for me.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
On Tue, Oct 06, 2020 at 11:22:11AM -0400, Bruce Momjian wrote:
On Tue, Oct 6, 2020 at 11:06:13AM -0400, Tom Lane wrote:
OK. FWIW, I'd vote for separate --no-instructions and --no-scripts
switches.Works for me.
+1.
--
Michael
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?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
On Tue, Oct 27, 2020 at 11:53 AM Bruce Momjian <bruce@momjian.us> 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.
Yeah, that's the one I was thinking of, but I was looking for something
more generic than explicitly saying --no-delete-script.
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
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.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
On Tue, Oct 27, 2020 at 12:40 PM Bruce Momjian <bruce@momjian.us> 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...
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
On Mon, Nov 2, 2020 at 2:23 PM Magnus Hagander <magnus@hagander.net> wrote:
On Tue, Oct 27, 2020 at 12:40 PM Bruce Momjian <bruce@momjian.us> 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/
Attachments:
no-instructions-2.patchtext/x-patch; charset=US-ASCII; name=no-instructions-2.patchDownload
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
On 02.11.2020 16:23, Magnus Hagander 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.
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.
Do we only care about .sh scripts? There are also reindex_hash.sql and
pg_largeobject.sqlin src/bin/pg_upgrade/version.c with instructions.
How should we handle them?
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
On Mon, Nov 9, 2020 at 2:18 PM Anastasia Lubennikova
<a.lubennikova@postgrespro.ru> wrote:
On 02.11.2020 16:23, Magnus Hagander 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.
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.Do we only care about .sh scripts? There are also reindex_hash.sql and pg_largeobject.sql in src/bin/pg_upgrade/version.c with instructions.
How should we handle them?
Oh, that's a good point. I had completely forgotten about those.
For consistency, one might say that those should be dropped as well.
But for usability that makes less sense. For the delete script, the
wrapper (that the switch is intended for) knows more than pg_upgrade
about how to delete it, so it can do a better job, and thus it makes
sense to silence it. But for something like these .sql (and also in
the previously mentioned case of extension upgrades), pg_upgrade knows
more and the only thing the wrapper could do is to reimplement the
same functionality, and maintain it.
I wonder if the best way forward might be to revert it back to being a
--no-instructions switch, remove the delete_old_cluster.sh script
completely and just replace it with instructions saying "you should
delete the old cluster", and then keep generating these scripts as
necessary?
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
On 2020-Nov-09, Magnus Hagander wrote:
But for usability that makes less sense. For the delete script, the
wrapper (that the switch is intended for) knows more than pg_upgrade
about how to delete it, so it can do a better job, and thus it makes
sense to silence it. But for something like these .sql (and also in
the previously mentioned case of extension upgrades), pg_upgrade knows
more and the only thing the wrapper could do is to reimplement the
same functionality, and maintain it.I wonder if the best way forward might be to revert it back to being a
--no-instructions switch, remove the delete_old_cluster.sh script
completely and just replace it with instructions saying "you should
delete the old cluster", and then keep generating these scripts as
necessary?
How about a switch like "--with-scripts=<list>" where the list can be
"all" to include everything (default), "none" to include nothing, or a
comma-separated list of things to include? (Also "--with-scripts=help"
to query which things are supported) So
"--with-scripts=reindex_hash,largeobject" omits the useless
delete_old_cluster script and keeps the ones you want.
Perhaps you could see it in the negative direction as more convenient,
so --suppress-scripts=delete_old_cluster
On Mon, Nov 9, 2020 at 3:29 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On 2020-Nov-09, Magnus Hagander wrote:
But for usability that makes less sense. For the delete script, the
wrapper (that the switch is intended for) knows more than pg_upgrade
about how to delete it, so it can do a better job, and thus it makes
sense to silence it. But for something like these .sql (and also in
the previously mentioned case of extension upgrades), pg_upgrade knows
more and the only thing the wrapper could do is to reimplement the
same functionality, and maintain it.I wonder if the best way forward might be to revert it back to being a
--no-instructions switch, remove the delete_old_cluster.sh script
completely and just replace it with instructions saying "you should
delete the old cluster", and then keep generating these scripts as
necessary?How about a switch like "--with-scripts=<list>" where the list can be
"all" to include everything (default), "none" to include nothing, or a
comma-separated list of things to include? (Also "--with-scripts=help"
to query which things are supported) So
"--with-scripts=reindex_hash,largeobject" omits the useless
delete_old_cluster script and keeps the ones you want.Perhaps you could see it in the negative direction as more convenient,
so --suppress-scripts=delete_old_cluster
What would be the actual use-case in this though?
I can see the --suppress-scripts=delete_old_cluster, but that's
basically the only usecase I can see for this. And for use in any
wrapper, that wrapper would have to know ahead of time what the
options would be... And if that's the only usecase, than this ends up
basically boiling down to the same as my suggestion just with a more
complex switch? :)
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
On 2020-Nov-09, Magnus Hagander wrote:
On Mon, Nov 9, 2020 at 3:29 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
How about a switch like "--with-scripts=<list>" where the list can be
"all" to include everything (default), "none" to include nothing, or a
comma-separated list of things to include? (Also "--with-scripts=help"
to query which things are supported) So
"--with-scripts=reindex_hash,largeobject" omits the useless
delete_old_cluster script and keeps the ones you want.Perhaps you could see it in the negative direction as more convenient,
so --suppress-scripts=delete_old_clusterWhat would be the actual use-case in this though?
The point is that we can do this just this time, and then in the future
we'll already have a mechanism for emitting or suppressing scripts that
are useful for some cases but not others. If I read you right, we
currenly have two cases: the user is either running it manually (in
which case you claim they want all scripts) or there's a wrapper (in
which case reindex_hash et al are useful, others are not). But what if,
say, we determined that it's a good idea to suggest to migrate BRIN
indexes from v1 to v2 for datatype X? If your tooling is prepared to
deal with that then it'll find the script useful, otherwise it's junk.
And we don't have to have a new option "--include-brin-upgrade" or
whatever; just add a new value to the suppressable script list.
On Mon, Nov 2, 2020 at 02:23:35PM +0100, Magnus Hagander wrote:
On Tue, Oct 27, 2020 at 12:40 PM Bruce Momjian <bruce@momjian.us> 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.
Uh, pg_upgrade does enumerate things like tablespaces in
create_script_for_old_cluster_deletion(). I think config file locations
are beyond the scope of what we want pg_upgrade to handle.
In summary, I think the vacuumdb --analyze is now a one-line command,
but the delete part can be complex and not easily typed.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
On Wed, Nov 11, 2020 at 4:53 PM Bruce Momjian <bruce@momjian.us> wrote:
On Mon, Nov 2, 2020 at 02:23:35PM +0100, Magnus Hagander wrote:
On Tue, Oct 27, 2020 at 12:40 PM Bruce Momjian <bruce@momjian.us> 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.Uh, pg_upgrade does enumerate things like tablespaces in
create_script_for_old_cluster_deletion(). I think config file locations
are beyond the scope of what we want pg_upgrade to handle.
Ah, that's right. Forgot that it did actually handle tablespaces.
But how would config file locations be beyond the scope? WIthout that,
it's incomplete for any user using that...
And for the vast majority of users, it would for example also have to
include the un-defining of a system service (systemd, sysvinit, or
whatever) as well, which would be even more out of scope by that
argument, yet at least as important in actually deleting the old
cluster...
In summary, I think the vacuumdb --analyze is now a one-line command,
but the delete part can be complex and not easily typed.
I definitely agree to that part -- my argument is that it's more
complex (or rather, more platform-specific) than can easily be put in
the script either. If it were to be replaced it wouldn't be withy "a
command",it would be with instructions basically saying "delete the
old cluster". Platform specific wrappers (debian, redhat, windows or
whatever) knows more and could do a more complete job, but trying to
make all that generic is very complicated.
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
On Wed, Nov 11, 2020 at 05:11:38PM +0100, Magnus Hagander wrote:
Uh, pg_upgrade does enumerate things like tablespaces in
create_script_for_old_cluster_deletion(). I think config file locations
are beyond the scope of what we want pg_upgrade to handle.Ah, that's right. Forgot that it did actually handle tablespaces.
But how would config file locations be beyond the scope? WIthout that,
it's incomplete for any user using that...
I would be worried the config file would somehow be shared between old
and new clusters. Potentially you could use the same pg_hba.conf for
old and new, and then deleting one would stop both. The cluster and
tablespace directories are clearly isolated to a single cluster. I am
not sure that is 100% true of config files, or if deletion permission
would be possible for those. It just seems too error-prone to attempt.
And for the vast majority of users, it would for example also have to
include the un-defining of a system service (systemd, sysvinit, or
whatever) as well, which would be even more out of scope by that
argument, yet at least as important in actually deleting the old
cluster...
Well, the question is what can the user reaonsably do. Finding all the
tablespace subdirecties is a pain, but managing systemctl seems like
something they would already have had to deal with. I am happy for
pg_upgrade to handle the mostly-internal parts and leave the user-facing
stuff to the user, which is what the pg_upgrade usage instructions do as
well.
In summary, I think the vacuumdb --analyze is now a one-line command,
but the delete part can be complex and not easily typed.I definitely agree to that part -- my argument is that it's more
complex (or rather, more platform-specific) than can easily be put in
the script either. If it were to be replaced it wouldn't be withy "a
command",it would be with instructions basically saying "delete the
old cluster". Platform specific wrappers (debian, redhat, windows or
whatever) knows more and could do a more complete job, but trying to
make all that generic is very complicated.
Agreed, but I don't see OS-specific instruction on how to delete the
tablespaces as reasonable --- that should be done by pg_upgrade, and we
have had almost no complaints about that.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
On Wed, Nov 11, 2020 at 11:21:22AM -0500, Bruce Momjian wrote:
On Wed, Nov 11, 2020 at 05:11:38PM +0100, Magnus Hagander wrote:
In summary, I think the vacuumdb --analyze is now a one-line command,
but the delete part can be complex and not easily typed.I definitely agree to that part -- my argument is that it's more
complex (or rather, more platform-specific) than can easily be put in
the script either. If it were to be replaced it wouldn't be withy "a
command",it would be with instructions basically saying "delete the
old cluster". Platform specific wrappers (debian, redhat, windows or
whatever) knows more and could do a more complete job, but trying to
make all that generic is very complicated.Agreed, but I don't see OS-specific instruction on how to delete the
tablespaces as reasonable --- that should be done by pg_upgrade, and we
have had almost no complaints about that.
Stepping back, I have always felt there was need for a wrapper program
to setup pg_upgrade, and clean up the old cluster on finish, though no
one has created such a tool yet.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
On Wed, Nov 11, 2020 at 5:38 PM Bruce Momjian <bruce@momjian.us> wrote:
On Wed, Nov 11, 2020 at 11:21:22AM -0500, Bruce Momjian wrote:
On Wed, Nov 11, 2020 at 05:11:38PM +0100, Magnus Hagander wrote:
In summary, I think the vacuumdb --analyze is now a one-line command,
but the delete part can be complex and not easily typed.I definitely agree to that part -- my argument is that it's more
complex (or rather, more platform-specific) than can easily be put in
the script either. If it were to be replaced it wouldn't be withy "a
command",it would be with instructions basically saying "delete the
old cluster". Platform specific wrappers (debian, redhat, windows or
whatever) knows more and could do a more complete job, but trying to
make all that generic is very complicated.Agreed, but I don't see OS-specific instruction on how to delete the
tablespaces as reasonable --- that should be done by pg_upgrade, and we
have had almost no complaints about that.Stepping back, I have always felt there was need for a wrapper program
to setup pg_upgrade, and clean up the old cluster on finish, though no
one has created such a tool yet.
Sure they have, it's just platform specific.
pg_upgradecluster on debian is a great example :)
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
On Wed, Nov 11, 2020 at 05:39:10PM +0100, Magnus Hagander wrote:
On Wed, Nov 11, 2020 at 5:38 PM Bruce Momjian <bruce@momjian.us> wrote:
On Wed, Nov 11, 2020 at 11:21:22AM -0500, Bruce Momjian wrote:
On Wed, Nov 11, 2020 at 05:11:38PM +0100, Magnus Hagander wrote:
In summary, I think the vacuumdb --analyze is now a one-line command,
but the delete part can be complex and not easily typed.I definitely agree to that part -- my argument is that it's more
complex (or rather, more platform-specific) than can easily be put in
the script either. If it were to be replaced it wouldn't be withy "a
command",it would be with instructions basically saying "delete the
old cluster". Platform specific wrappers (debian, redhat, windows or
whatever) knows more and could do a more complete job, but trying to
make all that generic is very complicated.Agreed, but I don't see OS-specific instruction on how to delete the
tablespaces as reasonable --- that should be done by pg_upgrade, and we
have had almost no complaints about that.Stepping back, I have always felt there was need for a wrapper program
to setup pg_upgrade, and clean up the old cluster on finish, though no
one has created such a tool yet.Sure they have, it's just platform specific.
pg_upgradecluster on debian is a great example :)
Yes, true. Ideally it would be nice to have a cross-platform wrapper
around pg_upgrade.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
On 2020-11-09 13:05, Magnus Hagander wrote:
PFA a rebased version of this patch on top of what has happened since,
and changing the pg_upgrade parameter to be --no-scripts.
It seems were are still finding out more nuances about pg_upgrade, but
looking at initdb for moment, I think the solution for wrapper scripts
is to just run initdb with >dev/null. Or maybe if that looks a bit too
hackish, a --quiet option that turns everything on stdout off.
I think initdb has gotten a bit too chatty over time. I think if it
printed nothing on stdout by default and the current output would be
some kind of verbose or debug mode, we wouldn't really lose much. With
that in mind, I'm a bit concerned about adding options (and thus
documentation surface area etc.) to select exactly which slice of the
chattiness to omit.
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/
On Fri, Nov 20, 2020 at 4:46 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 2020-11-09 13:05, Magnus Hagander wrote:
PFA a rebased version of this patch on top of what has happened since,
and changing the pg_upgrade parameter to be --no-scripts.It seems were are still finding out more nuances about pg_upgrade, but
looking at initdb for moment, I think the solution for wrapper scripts
is to just run initdb with >dev/null. Or maybe if that looks a bit too
hackish, a --quiet option that turns everything on stdout off.I think initdb has gotten a bit too chatty over time. I think if it
printed nothing on stdout by default and the current output would be
some kind of verbose or debug mode, we wouldn't really lose much. With
that in mind, I'm a bit concerned about adding options (and thus
documentation surface area etc.) to select exactly which slice of the
chattiness to omit.
I agree that it's getting unnecessarily chatty, but things like the
locale that it has detected I think is very useful information to
output. Though I guess the same could be said for a few other things,
but does it *ever' pick anything other than 128Mb/100 for example? :)
The main difference between them is that some information is
informational but unnecessary, but the "next steps instructions" are
*incorrect* in most cases when executed by a wrapper. I'd argue that
even if we show them only with --verbose, we should still have a way
of not outputing the information that's going to be incorrect for the
end user.
I think it boils down to that today the output from initdb is entirely
geared towards people running initdb directly and starting their
server manually, and very few people outside the actual PostgreSQL
developers ever do that. But there are still a lot of people who run
initdb through their wrapper manually (for redhat you have to do that,
for debian you only have to do it if you're creating a secondary
cluster but that still a pretty common operation).
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
On Tue, Nov 24, 2020 at 01:32:45PM +0100, Magnus Hagander wrote:
I think it boils down to that today the output from initdb is entirely
geared towards people running initdb directly and starting their
server manually, and very few people outside the actual PostgreSQL
developers ever do that. But there are still a lot of people who run
initdb through their wrapper manually (for redhat you have to do that,
for debian you only have to do it if you're creating a secondary
cluster but that still a pretty common operation).
I think the big issue is that pg_upgrade not only output progress
messages, but created files in the current directory, while initdb, by
definition, is creating files in PGDATA.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
On Tue, Nov 24, 2020 at 3:12 PM Bruce Momjian <bruce@momjian.us> wrote:
On Tue, Nov 24, 2020 at 01:32:45PM +0100, Magnus Hagander wrote:
I think it boils down to that today the output from initdb is entirely
geared towards people running initdb directly and starting their
server manually, and very few people outside the actual PostgreSQL
developers ever do that. But there are still a lot of people who run
initdb through their wrapper manually (for redhat you have to do that,
for debian you only have to do it if you're creating a secondary
cluster but that still a pretty common operation).I think the big issue is that pg_upgrade not only output progress
messages, but created files in the current directory, while initdb, by
definition, is creating files in PGDATA.
To be clear, my comments above were primarily about initdb, not
pg_upgrade, as that's what Peter was commenting on as well.
pg_upgrade is a somewhat different but also interesting case. I think
the actual progress output is more interesting in pg_upgrade as it's
more likely to take measurable amounts of time. Whereas in initdb,
it's actually the "detected parameter values" that are the most
interesting parts.
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
On Tue, Nov 24, 2020 at 04:05:26PM +0100, Magnus Hagander wrote:
pg_upgrade is a somewhat different but also interesting case. I think
the actual progress output is more interesting in pg_upgrade as it's
more likely to take measurable amounts of time. Whereas in initdb,
it's actually the "detected parameter values" that are the most
interesting parts.
Originally, initdb did take some time for each step.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
On 2020-11-24 13:32, Magnus Hagander wrote:
I think it boils down to that today the output from initdb is entirely
geared towards people running initdb directly and starting their
server manually, and very few people outside the actual PostgreSQL
developers ever do that. But there are still a lot of people who run
initdb through their wrapper manually (for redhat you have to do that,
for debian you only have to do it if you're creating a secondary
cluster but that still a pretty common operation).
Perhaps it's worth asking whom the advice applies to then. You suggest
it's mostly developers. I for one am still grumpy that in 9.5 we
removed the variant of the hint that suggested "postgres -D ..." instead
of pg_ctl. I used to copy and paste that a lot. The argument back then
was that the hint should target end users, not developers. I doubt that
under the current circumstances, running pg_ctl start from the console
is really appropriate advice for a majority of users. (For one thing,
systemd will kill it when you log out.) I don't know what better advice
would be, though. Maybe we need to add some kind of text adventure game
into initdb.
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/
On Wed, Nov 25, 2020 at 9:29 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 2020-11-24 13:32, Magnus Hagander wrote:
I think it boils down to that today the output from initdb is entirely
geared towards people running initdb directly and starting their
server manually, and very few people outside the actual PostgreSQL
developers ever do that. But there are still a lot of people who run
initdb through their wrapper manually (for redhat you have to do that,
for debian you only have to do it if you're creating a secondary
cluster but that still a pretty common operation).Perhaps it's worth asking whom the advice applies to then. You suggest
it's mostly developers. I for one am still grumpy that in 9.5 we
removed the variant of the hint that suggested "postgres -D ..." instead
of pg_ctl. I used to copy and paste that a lot. The argument back then
was that the hint should target end users, not developers. I doubt that
under the current circumstances, running pg_ctl start from the console
is really appropriate advice for a majority of users. (For one thing,
systemd will kill it when you log out.) I don't know what better advice
I guess one option could be to just remove it, unconditionally. And
assume that any users who is running it manually read that in docs
somewhere that tells them what to do next, and that any user who's
running it under a wrapper will have the wrapper set it up?
would be, though. Maybe we need to add some kind of text adventure game
into initdb.
I do like this idea though...
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
I guess one option could be to just remove it, unconditionally. And
assume that any users who is running it manually read that in docs
somewhere that tells them what to do next, and that any user who's
running it under a wrapper will have the wrapper set it up?
I could get behind that, personally. (1) I think most end-users don't
run initdb by hand anymore. (2) The message is barely useful; what
it mostly does is to distract your attention from the slightly
more useful info printed ahead of it.
would be, though. Maybe we need to add some kind of text adventure game
into initdb.
I do like this idea though...
+1
regards, tom lane
On Wed, Nov 25, 2020 at 10:33:09AM -0500, Tom Lane wrote:
Magnus Hagander <magnus@hagander.net> writes:
I guess one option could be to just remove it, unconditionally. And
assume that any users who is running it manually read that in docs
somewhere that tells them what to do next, and that any user who's
running it under a wrapper will have the wrapper set it up?I could get behind that, personally. (1) I think most end-users don't
run initdb by hand anymore. (2) The message is barely useful; what
it mostly does is to distract your attention from the slightly
more useful info printed ahead of it.
I think the question is not how many people who run initdb in any form
need the instructions to start Postgres, but how many who are actually
seeing the initdb output need those instructions. If someone isn't
seeing the initdb output, it doesn't matter what we output.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
On Wed, Nov 25, 2020 at 04:25:56PM +0100, Magnus Hagander wrote:
On Wed, Nov 25, 2020 at 9:29 AM Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
Perhaps it's worth asking whom the advice applies to then. You suggest
it's mostly developers. I for one am still grumpy that in 9.5 we
removed the variant of the hint that suggested "postgres -D ..." instead
of pg_ctl. I used to copy and paste that a lot. The argument back then
was that the hint should target end users, not developers. I doubt that
under the current circumstances, running pg_ctl start from the console
is really appropriate advice for a majority of users. (For one thing,
systemd will kill it when you log out.) I don't know what better adviceI guess one option could be to just remove it, unconditionally. And
assume that any users who is running it manually read that in docs
somewhere that tells them what to do next, and that any user who's
running it under a wrapper will have the wrapper set it up?
Hmm. I don't think that users running manually initdb will read the
documentation if we don't show directly a reference to them. FWIW, I
agree with Peter that it would have been nice to keep around the 9.5
hint, and I would wish that the last one remains around. I agree,
however, that there is value in having a switch that suppresses them.
would be, though. Maybe we need to add some kind of text adventure game
into initdb.I do like this idea though...
+1.
--
MichaelAfter pondering this again, I think we can go with initdb
--no-instructions, as in your patch.
As a minor nitpick, I would leave out the
else
printf(_("\nSuccess.\n"));
in the --no-instructions case.
(I don't know where the pg_upgrade part of this discussion is right now.)
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/
On Thu, Jan 7, 2021 at 11:53 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
After pondering this again, I think we can go with initdb
--no-instructions, as in your patch.As a minor nitpick, I would leave out the
else
printf(_("\nSuccess.\n"));in the --no-instructions case.
OK, thanks. I have applied it as such, with that message moved inside
the if statement.
(I don't know where the pg_upgrade part of this discussion is right now.)
Yeah, I think that one turned into quite a different discussion. I
think it's clear to say that the part of this patch related to
pg_upgrade was rejected - I'll drop that one. I think pg_upgrade
scripts has to be thought about as a completely separate thing, so
let's leave that for another thread.
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
On 2021-01-17 14:38, Magnus Hagander wrote:
On Thu, Jan 7, 2021 at 11:53 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:After pondering this again, I think we can go with initdb
--no-instructions, as in your patch.As a minor nitpick, I would leave out the
else
printf(_("\nSuccess.\n"));in the --no-instructions case.
OK, thanks. I have applied it as such, with that message moved inside
the if statement.
It appears that there is an extra blank line in the initdb output before
"Success" now.
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/
On Wed, Feb 3, 2021 at 4:21 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 2021-01-17 14:38, Magnus Hagander wrote:
On Thu, Jan 7, 2021 at 11:53 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:After pondering this again, I think we can go with initdb
--no-instructions, as in your patch.As a minor nitpick, I would leave out the
else
printf(_("\nSuccess.\n"));in the --no-instructions case.
OK, thanks. I have applied it as such, with that message moved inside
the if statement.It appears that there is an extra blank line in the initdb output before
"Success" now.
Oops, clearly it does.
That said, the full output is:
"""
Success. You can now start the database server using:
bin/pg_ctl -D /tmp/data -l logfile start
Success.
"""
Isn't the whole "Success." at the end redundant here, and we should
just end the message after the pg_ctl command? So not just the extra
newline, but the whole thing?
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
On Sun, Feb 7, 2021 at 11:21:05AM +0100, Magnus Hagander wrote:
It appears that there is an extra blank line in the initdb output before
"Success" now.Oops, clearly it does.
That said, the full output is:
"""
Success. You can now start the database server using:bin/pg_ctl -D /tmp/data -l logfile start
Success.
"""Isn't the whole "Success." at the end redundant here, and we should
just end the message after the pg_ctl command? So not just the extra
newline, but the whole thing?
Agreed.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
Bruce Momjian <bruce@momjian.us> writes:
On Sun, Feb 7, 2021 at 11:21:05AM +0100, Magnus Hagander wrote:
Isn't the whole "Success." at the end redundant here, and we should
just end the message after the pg_ctl command? So not just the extra
newline, but the whole thing?
Agreed.
+1
regards, tom lane
I wrote:
Bruce Momjian <bruce@momjian.us> writes:
On Sun, Feb 7, 2021 at 11:21:05AM +0100, Magnus Hagander wrote:
Isn't the whole "Success." at the end redundant here, and we should
just end the message after the pg_ctl command? So not just the extra
newline, but the whole thing?
Agreed.
+1
In fact, I just noticed that none of our back branches print that
extra "Success." either. So that's a bug, is what it is.
regards, tom lane