Thread: 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.
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.
Attachment
Re: Prevent printing "next step instructions" in initdb and pg_upgrade
From
Ian Lawrence Barwick
Date:
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 instructionsabout 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 wrappersthe printed instructions are *wrong*. It's better in that case to exclude them, and let the wrapper be responsiblefor 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 gowith "--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 <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 the wrapper
> 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 <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
Attachment
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.
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/
Attachment
Re: Prevent printing "next step instructions" in initdb and pg_upgrade
From
Anastasia Lubennikova
Date:
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.
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.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.
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.cwith 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_cluster > > What 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 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? 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. -- Michael
Attachment
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. (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