[COMMITTERS] pgsql: Change pg_ctl to detect server-ready by watching status inpostm - Mailing list pgsql-committers

Change pg_ctl to detect server-ready by watching status in postmaster.pid.

Traditionally, "pg_ctl start -w" has waited for the server to become
ready to accept connections by attempting a connection once per second.
That has the major problem that connection issues (for instance, a
kernel packet filter blocking traffic) can't be reliably told apart
from server startup issues, and the minor problem that if server startup
isn't quick, we accumulate "the database system is starting up" spam
in the server log.  We've hacked around many of the possible connection
issues, but it resulted in ugly and complicated code in pg_ctl.c.

In commit c61559ec3, I changed the probe rate to every tenth of a second.
That prompted Jeff Janes to complain that the log-spam problem had become
much worse.  In the ensuing discussion, Andres Freund pointed out that
we could dispense with connection attempts altogether if the postmaster
were changed to report its status in postmaster.pid, which "pg_ctl start"
already relies on being able to read.  This patch implements that, teaching
postmaster.c to report a status string into the pidfile at the same
state-change points already identified as being of interest for systemd
status reporting (cf commit 7d17e683f).  pg_ctl no longer needs to link
with libpq at all; all its functions now depend on reading server files.

In support of this, teach AddToDataDirLockFile() to allow addition of
postmaster.pid lines in not-necessarily-sequential order.  This is needed
on Windows where the SHMEM_KEY line will never be written at all.  We still
have the restriction that we don't want to truncate the pidfile; document
the reasons for that a bit better.

Also, fix the pg_ctl TAP tests so they'll notice if "start -w" mode
is broken --- before, they'd just wait out the sixty seconds until
the loop gives up, and then report success anyway.  (Yes, I found that
out the hard way.)

While at it, arrange for pg_ctl to not need to #include miscadmin.h;
as a rather low-level backend header, requiring that to be compilable
client-side is pretty dubious.  This requires moving the #define's
associated with the pidfile into a new header file, and moving
PG_BACKEND_VERSIONSTR someplace else.  For lack of a clearly better
"someplace else", I put it into port.h, beside the declaration of
find_other_exec(), since most users of that macro are passing the value to
find_other_exec().  (initdb still depends on miscadmin.h, but at least
pg_ctl and pg_upgrade no longer do.)

In passing, fix main.c so that PG_BACKEND_VERSIONSTR actually defines the
output of "postgres -V", which remarkably it had never done before.

Discussion: https://postgr.es/m/CAMkU=1xJW8e+CTotojOMBd-yzUvD0e_JZu2xHo=MnuZ4__m7Pg@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/f13ea95f9e473a43ee4e1baeb94daaf83535d37c

Modified Files
--------------
src/backend/main/main.c             |   2 +-
src/backend/port/sysv_shmem.c       |   1 +
src/backend/postmaster/postmaster.c |  30 +++-
src/backend/utils/init/miscinit.c   |  29 ++--
src/bin/pg_ctl/Makefile             |   6 +-
src/bin/pg_ctl/pg_ctl.c             | 265 ++++++++++++------------------------
src/bin/pg_ctl/t/001_start_stop.pl  |   8 +-
src/bin/pg_upgrade/option.c         |   2 +-
src/common/config_info.c            |   1 -
src/include/miscadmin.h             |  27 ----
src/include/port.h                  |   3 +
src/include/utils/pidfile.h         |  55 ++++++++
src/tools/msvc/Mkvcbuild.pm         |   2 +-
13 files changed, 203 insertions(+), 228 deletions(-)