Thread: drop postmaster symlink
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. [0]: https://www.postgresql.org/message-id/bfdf03c8-c24f-c5b1-474e-4c9a96210f46@enterprisedb.com
Attachment
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
Attachment
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 https://www.postgresql.org/message-id/20230107165942.748ccf4e%40slate.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 > https://www.postgresql.org/message-id/20230107165942.748ccf4e%40slate.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 > https://www.postgresql.org/message-id/20230107165942.748ccf4e%40slate.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.
When creating a private service for another instance of PostgreSQL I used the template of postgresql-15.service file installed into /usr/lib/systemd/system on Fedora 38 provided through the installation for postgres 15.3 from PGDG repositories.
There I noticed that the line ExecStart still uses the postmaster link.
I would propose to change it to
ExecStart=/usr/pgsql-15/bin/postgres -D $(PGDATA)
This change should apply also to back branches to avoid using deprecated links in PGDG software.
This seems to be necessesary on upcoming PG16.
(BTW: where is this all documented?)
Hans Buschmann