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

Enterprise / PostgreSQL - Tom Lane [sss.pgh.pa.us] - 28 June 2017 17:31 EDT

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

f13ea95 Change pg_ctl to detect server-ready by watching status in postmaster.pid.
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(-)

Upstream: git.postgresql.org


  • Share