drop postmaster symlink

On 11/23/22 02:52, Peter Eisentraut wrote:

A little while ago we discussed briefly over in the meson thread whether
we could remove the postmaster symlink [0]. The meson build system
currently does not install a postmaster symlink. (AFAICT, the MSVC
build system does not either.) So if we want to elevate the meson build
system, we either need to add the postmaster symlink, or remove it from
the other build system(s) as well. Seeing that it's been deprecated for
a long time, I propose we just remove it. See attached patches.

I am a big +1 on removing the symlink, however it is worth pointing out
that the PGDG RPMs still use the symlink in the included systemd service
file:

8<----------
ExecStart=/usr/pgsql-15/bin/postmaster -D ${PGDATA}
8<----------

--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Hi,

On Wed, 2022-11-23 at 09:18 -0500, Joe Conway wrote:

I am a big +1 on removing the symlink, however it is worth pointing
out
that the PGDG RPMs still use the symlink in the included systemd
service
file:

8<----------
ExecStart=/usr/pgsql-15/bin/postmaster -D ${PGDATA}

...and it helps us to find the "main" process a bit easily.

Regards,
--
Devrim Gündüz
Open Source Solution Architect, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR

Devrim =?ISO-8859-1?Q?G=FCnd=FCz?= <devrim@gunduz.org> writes:

...and it helps us to find the "main" process a bit easily.

Hmm, that's a nontrivial point perhaps. It's certain that this
will break some other people's start scripts too. On the whole,
is it really that hard to add the symlink to the meson build?

regards, tom lane

Hi,

On 2022-11-23 10:07:49 -0500, Tom Lane wrote:

Devrim =?ISO-8859-1?Q?G=FCnd=FCz?= <devrim@gunduz.org> writes:

...and it helps us to find the "main" process a bit easily.

Hmm, that's a nontrivial point perhaps. It's certain that this
will break some other people's start scripts too.

OTOH, postmaster has been deprecated for ~15 years.

On the whole, is it really that hard to add the symlink to the meson build?

No. Meson has a builtin command for it, just not in the meson version we're
currently requiring. We can create the symlink ourselves instead. The problem
is just detecting systems where we can't symlink and what to fall back to
there.

Greetings,

Andres Freund

On Wed, Nov 23, 2022 at 2:50 PM Andres Freund <andres@anarazel.de> wrote:

On 2022-11-23 10:07:49 -0500, Tom Lane wrote:

Devrim =?ISO-8859-1?Q?G=FCnd=FCz?= <devrim@gunduz.org> writes:

...and it helps us to find the "main" process a bit easily.

Hmm, that's a nontrivial point perhaps. It's certain that this
will break some other people's start scripts too.

OTOH, postmaster has been deprecated for ~15 years.

Yeah. Also, I don't think it's generally too hard to find the parent
process anyway, because at least on my system, the other ones end up
with ps display that looks like "postgres: logical replication
launcher" or whatever. The main process doesn't set the ps status
display, so that's the only one that shows a full path to the
executable in the ps status, which is how I usually spot it. That has
the advantage that it doesn't matter which name was used to launch it,
too.

I don't actually care very much whether we get rid of the postmaster
symlink or not, but if we aren't going to, we should stop calling it
deprecated. If 15 years isn't enough time to remove it, what ever will
be? I tend to think it's fairly pointless and perhaps also a bit
confusing, because the product is postgres not postmaster and people
can reasonably expect the binary name to match the product name. But
if we keep it, I don't think anything too dire will happen, either.

--
Robert Haas
EDB: http://www.enterprisedb.com

On 11/23/22 15:10, Robert Haas wrote:

On Wed, Nov 23, 2022 at 2:50 PM Andres Freund <andres@anarazel.de> wrote:

On 2022-11-23 10:07:49 -0500, Tom Lane wrote:

Devrim =?ISO-8859-1?Q?G=FCnd=FCz?= <devrim@gunduz.org> writes:

...and it helps us to find the "main" process a bit easily.

Hmm, that's a nontrivial point perhaps. It's certain that this
will break some other people's start scripts too.

OTOH, postmaster has been deprecated for ~15 years.

Yeah. Also, I don't think it's generally too hard to find the parent
process anyway, because at least on my system, the other ones end up
with ps display that looks like "postgres: logical replication
launcher" or whatever. The main process doesn't set the ps status
display, so that's the only one that shows a full path to the
executable in the ps status, which is how I usually spot it. That has
the advantage that it doesn't matter which name was used to launch it,
too.

Same here

I don't actually care very much whether we get rid of the postmaster
symlink or not, but if we aren't going to, we should stop calling it
deprecated. If 15 years isn't enough time to remove it, what ever will
be? I tend to think it's fairly pointless and perhaps also a bit
confusing, because the product is postgres not postmaster and people
can reasonably expect the binary name to match the product name. But
if we keep it, I don't think anything too dire will happen, either.

FWIW, the reason I took note of the postmaster symlink in the first
place a few years ago was because selinux treats execution of programs
from symlinks differently than from actual files.

--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Andres Freund <andres@anarazel.de> writes:

On 2022-11-23 10:07:49 -0500, Tom Lane wrote:

On the whole, is it really that hard to add the symlink to the meson build?

No. Meson has a builtin command for it, just not in the meson version we're
currently requiring. We can create the symlink ourselves instead. The problem
is just detecting systems where we can't symlink and what to fall back to
there.

This isn't a hill I want to die on, either way. But "it's a bit
more complicated in meson" seems like a poor reason for changing
the user-visible installed fileset.

regards, tom lane

Hi,

On 2022-11-23 15:48:04 -0500, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2022-11-23 10:07:49 -0500, Tom Lane wrote:

On the whole, is it really that hard to add the symlink to the meson build?

No. Meson has a builtin command for it, just not in the meson version we're
currently requiring. We can create the symlink ourselves instead. The problem
is just detecting systems where we can't symlink and what to fall back to
there.

This isn't a hill I want to die on, either way. But "it's a bit
more complicated in meson" seems like a poor reason for changing
the user-visible installed fileset.

I wouldn't even have thought about proposing dropping the symlink if it hadn't
been deprecated forever, and I suspect Peter wouldn't have either...

I think this is a bit more more complicated than "changing the user-visible
installed fileset" because we didn't have logic to create 'postmaster' on
windows before, afaik the only OS without reliable symlink support.

Anyway, my current thinking is to have dumb OS dependent behaviour and create
a symlink everywhere but windows, where we'd just copy the file.

Or we could just continue to not install 'postmaster' on windows, because of
the potential confusion that 'postmaster.exe' differing from 'postgres.exe'
could cause.

Greetings,

Andres Freund

On 23 Nov 2022, at 21:10, Robert Haas <robertmhaas@gmail.com> wrote:

I don't actually care very much whether we get rid of the postmaster
symlink or not, but if we aren't going to, we should stop calling it
deprecated. If 15 years isn't enough time to remove it, what ever will
be?

+1. If we actively add support for something then it isn't really all that
deprecated IMHO.

--
Daniel Gustafsson https://vmware.com/

Hello,

This is a review of Peter's 2 patches. I see only 1 small problem.

+++

Looking at the documentation, a "postmaster" in the glossary is
defined as the controlling process. This works; it needs to be called
something. There is still a postmaster.pid (etc.) in the data
directory.

The word "postmaster" (case insensitive) shows up 84 times in the
documentation. I looked at all of these.

I see a possible problem at line 1,412 of runtime.sgml, the "Linux
Memory Overcommit" section. It talks about the postmaster's startup
script invoking the postmaster. It might, possibly, be better to say
that "postgres" is invoked, that being the name of the invoked program.
There's a similar usage at line 1,416. On the other hand, the existing
text makes it quite clear what is going on and there's a lot to be said
for consistently using the word "postmaster". I mention this only in
case somebody deems it significant. Perhaps there's a way to use
markup, identifying "postgres" as a program with a name in the file
system, to make things more clear. Most likely, nobody will care.

In doc/src/sgml/ref/allfiles.sgml at line 222 there is an ENTITY
defined which references the deleted postmaster.sgml file. Since I
did a maintainer-clean, and the patch deletes the postmaster.sgml
file, and I don't see any references to the entity in the docs, I
believe that this line should be removed. (In other words, I don't
think this file is automatically maintained.)

After applying the patches the documentation seems to build just fine.

I have not run contrib/start-scripts/{freebsd,linux}, but the patch
looks fine and the result of applying the patch looks fine, and the
patch is a one-line simple replacement of "postmaster" with "postgres"
so I expect no problems.

The modification to src/port/path.c is in a comment, so it will surely
not affect anything at runtime. And the changed comment looks right
to me.

There's thousands of lines of comments in the code where "postmaster"
(case insensitive) shows up after applying the patches. I've not
reviewed any of these. Or looked for odd variable names, or
references in the code to the postmaster binary - by name, etc. I'm
not sure where it would make sense to look for such problems.

Aside from the "allfiles.sgml" problem, I see no reason why these 2
patches should not be applied. As mentioned by others, I don't have
strong feelings about getting rid of the postmaster symlink. But I
did pick this patch to review because I remember, once upon a time,
being slightly confused by a "postmaster" in the process list.

Regards,

Karl <kop@karlpinc.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

"Karl O. Pinc" <kop@karlpinc.com> writes:

This is a review of Peter's 2 patches. I see only 1 small problem.

Looking at the documentation, a "postmaster" in the glossary is
defined as the controlling process. This works; it needs to be called
something. There is still a postmaster.pid (etc.) in the data
directory.

The word "postmaster" (case insensitive) shows up 84 times in the
documentation. I looked at all of these.

Hmm ... I thought this patch was about getting rid of the
admittedly-obsolete installed symlink. If it's trying to get rid of
the "postmaster" terminology for our parent process, I'm very strongly
against that, either as regards to code or documentation.

regards, tom lane

On 1/7/23 18:38, Tom Lane wrote:

"Karl O. Pinc" <kop@karlpinc.com> writes:

This is a review of Peter's 2 patches. I see only 1 small problem.

Looking at the documentation, a "postmaster" in the glossary is
defined as the controlling process. This works; it needs to be called
something. There is still a postmaster.pid (etc.) in the data
directory.

The word "postmaster" (case insensitive) shows up 84 times in the
documentation. I looked at all of these.

Hmm ... I thought this patch was about getting rid of the
admittedly-obsolete installed symlink.

That was my understanding too.

If it's trying to get rid of the "postmaster" terminology for our
parent process, I'm very strongly against that, either as regards to
code or documentation.

+1

--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Sat, 07 Jan 2023 18:38:25 -0500
Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Karl O. Pinc" <kop@karlpinc.com> writes:

This is a review of Peter's 2 patches. I see only 1 small problem.

Looking at the documentation, a "postmaster" in the glossary is
defined as the controlling process. This works; it needs to be
called something. There is still a postmaster.pid (etc.) in the
data directory.

The word "postmaster" (case insensitive) shows up 84 times in the
documentation. I looked at all of these.

Hmm ... I thought this patch was about getting rid of the
admittedly-obsolete installed symlink. If it's trying to get rid of
the "postmaster" terminology for our parent process, I'm very strongly
against that, either as regards to code or documentation.

No. It's about getting rid of the symlink. I was grepping around
to look for use of the symlink, started with the glossary just
to be sure, and saw that "postmaster" is consistently (I think)
used to refer to the controlling, parent, process. And wrote
down what I was doing and what I found as I went along. And then
sent out my notes.

Sorry for the confusion.

Regards,

Karl <kop@karlpinc.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

On Sat, 7 Jan 2023 19:33:38 -0500
Joe Conway <mail@joeconway.com> wrote:

On 1/7/23 18:38, Tom Lane wrote:

"Karl O. Pinc" <kop@karlpinc.com> writes:

This is a review of Peter's 2 patches. I see only 1 small
problem.

The small problem is a reference to a deleted file.

Regards,

Karl <kop@karlpinc.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

On Sat, 7 Jan 2023 19:56:08 -0600
"Karl O. Pinc" <kop@karlpinc.com> wrote:

On Sat, 07 Jan 2023 18:38:25 -0500
Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Karl O. Pinc" <kop@karlpinc.com> writes:

This is a review of Peter's 2 patches. I see only 1 small
problem. ...

Hmm ... I thought this patch was about getting rid of the
admittedly-obsolete installed symlink. ...

No. It's about getting rid of the symlink.

The only way I could think of to review a patch
that removes something is to report all the places
I looked where a reference to the symlink might be.
Then report what I found each place I looked and
report if there's a problem, or might be.

That way the committer knows where I didn't look if there's
more that needs removing.

Apologies that this was not clear.

Regards,

Karl <kop@karlpinc.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

On Sat, 7 Jan 2023 22:29:35 -0600
"Karl O. Pinc" <kop@karlpinc.com> wrote:

The only way I could think of to review a patch
that removes something is to report all the places
I looked where a reference to the symlink might be.

I forgot to report that I also tried a `make install`
and 'make uninstall`, with no problems.

FWIW, I suspect the include/backend/postmaster/ directory
would today be named include/backend/postgres/. Now
would be the time to change the name, but I don't see
it being worth the work. And while such a change
wouldn't break pg, that kind of change would break something
for somebody.

Regards,

Karl <kop@karlpinc.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

On 23.11.22 21:32, Joe Conway wrote:

Yeah. Also, I don't think it's generally too hard to find the parent
process anyway, because at least on my system, the other ones end up
with ps display that looks like "postgres: logical replication
launcher" or whatever. The main process doesn't set the ps status
display, so that's the only one that shows a full path to the
executable in the ps status, which is how I usually spot it. That has
the advantage that it doesn't matter which name was used to launch it,
too.

I think it is a problem that one of the most widely used packagings of
PostgreSQL uses techniques that are directly contradicting the
PostgreSQL documentation and are also inconsistent with other widely
used packagings. Users might learn this "trick" but then can't reuse it
elsewhere, and conversely those who come from other systems might not be
able to reuse their scripts. That is annoying.

FWIW, the reason I took note of the postmaster symlink in the first
place a few years ago was because selinux treats execution of programs
from symlinks differently than from actual files.

This is another such case, where knowledge about selinux configuration
cannot be transported between Linux distributions.

I almost feel that issues like this make a stronger case for removing
the postmaster symlink than if it hadn't actually been in use, since the
removal would serve to unify the landscape for the benefit of users.

On 1/12/23 12:00, Peter Eisentraut wrote:

On 23.11.22 21:32, Joe Conway wrote:

Yeah. Also, I don't think it's generally too hard to find the parent
process anyway, because at least on my system, the other ones end up
with ps display that looks like "postgres: logical replication
launcher" or whatever. The main process doesn't set the ps status
display, so that's the only one that shows a full path to the
executable in the ps status, which is how I usually spot it. That has
the advantage that it doesn't matter which name was used to launch it,
too.

I think it is a problem that one of the most widely used packagings of
PostgreSQL uses techniques that are directly contradicting the
PostgreSQL documentation and are also inconsistent with other widely
used packagings. Users might learn this "trick" but then can't reuse it
elsewhere, and conversely those who come from other systems might not be
able to reuse their scripts. That is annoying.

FWIW, the reason I took note of the postmaster symlink in the first
place a few years ago was because selinux treats execution of programs
from symlinks differently than from actual files.

This is another such case, where knowledge about selinux configuration
cannot be transported between Linux distributions.

I almost feel that issues like this make a stronger case for removing
the postmaster symlink than if it hadn't actually been in use, since the
removal would serve to unify the landscape for the benefit of users.

To be clear, I am completely in agreement with you about removing the
symlink. I just wanted to be sure Devrim was alerted because I knew he
had a strong opinion on this topic ;-)

--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Hi,

On Thu, 2023-01-12 at 13:35 -0500, Joe Conway wrote:

To be clear, I am completely in agreement with you about removing the
symlink. I just wanted to be sure Devrim was alerted because I knew
he had a strong opinion on this topic ;-)

Red Hat's own packages, thus their users may be unhappy about that,
too. They also call postmaster directly.

Regsards,
--
Devrim Gündüz
Open Source Solution Architect, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR

On 12.01.23 20:11, Devrim Gündüz wrote:

On Thu, 2023-01-12 at 13:35 -0500, Joe Conway wrote:

To be clear, I am completely in agreement with you about removing the
symlink. I just wanted to be sure Devrim was alerted because I knew
he had a strong opinion on this topic ;-)

Red Hat's own packages, thus their users may be unhappy about that,
too. They also call postmaster directly.

Devrim,

Apart from your concerns, it appears there is consensus for making this
change. The RPM packaging scripts can obviously be fixed easily for
this. Do you have an objection to making this change?

Hi,

On Wed, 2023-01-25 at 08:54 +0100, Peter Eisentraut wrote:

Apart from your concerns, it appears there is consensus for making
this change.  The RPM packaging scripts can obviously be fixed
easily for this.  Do you have an objection to making this change?

I'm inclined to create the symlink in the RPMs to make users' lives
(and my life) easier. So, no objection from here.

Regards,

--
Devrim Gündüz
Open Source Solution Architect, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR

Hello,

Somehow I missed the email changing the status of this back
to "needs review".

Buried in
/messages/by-id/20230107165942.748ccf4e@slate.karlpinc.com
is the one change I see that should be made.

In doc/src/sgml/ref/allfiles.sgml at line 222 there is an ENTITY
defined which references the deleted postmaster.sgml file.

This line needs to be removed and the
0002-Don-t-install-postmaster-symlink-anymore.patch
updated. (Unless there's some magic going on
with the various allfiles.sgml files of which I am
not aware.)

If this is fixed I see no other problems.

Regards,

Karl <kop@karlpinc.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

On 26.01.23 01:03, Karl O. Pinc wrote:

Buried in
/messages/by-id/20230107165942.748ccf4e@slate.karlpinc.com
is the one change I see that should be made.

In doc/src/sgml/ref/allfiles.sgml at line 222 there is an ENTITY
defined which references the deleted postmaster.sgml file.

This line needs to be removed and the
0002-Don-t-install-postmaster-symlink-anymore.patch
updated. (Unless there's some magic going on
with the various allfiles.sgml files of which I am
not aware.)

If this is fixed I see no other problems.

Good find. Committed with this additional change.

On Wed, 25 Jan 2023 18:03:25 -0600
"Karl O. Pinc" <kop@k

Buried in
/messages/by-id/20230107165942.748ccf4e@slate.karlpinc.com
is the one change I see that should be made.

In doc/src/sgml/ref/allfiles.sgml at line 222 there is an ENTITY
defined which references the deleted postmaster.sgml file.

This line needs to be removed and the
0002-Don-t-install-postmaster-symlink-anymore.patch
updated. (Unless there's some magic going on
with the various allfiles.sgml files of which I am
not aware.)

If this is fixed I see no other problems.

Buried in the same email, and I apologize for not mentioning
this, is one other bit of documentation text that might
or might not need attention.

I see a possible problem at line 1,412 of runtime.sgml

This says:

in the postmaster's startup script just before invoking the postmaster.

Depending on how this is read, it could be interpreted to mean
that a "postmaster" binary is invoked. It might be more clear
to write: ... just before invoking <command>postgres</command>.

Or it might not be worth bothering about; at this point, probably
not, but I thought you might want the heads-up anyway.

Regards,

Karl <kop@karlpinc.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

On 26.01.23 19:36, Karl O. Pinc wrote:

I see a possible problem at line 1,412 of runtime.sgml

This says:

in the postmaster's startup script just before invoking the postmaster.

Depending on how this is read, it could be interpreted to mean
that a "postmaster" binary is invoked. It might be more clear
to write: ... just before invoking <command>postgres</command>.

Or it might not be worth bothering about; at this point, probably
not, but I thought you might want the heads-up anyway.

Good find. I have adjusted that, and a few more nearby.