From f13ea95f9e473a43ee4e1baeb94daaf83535d37c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 28 Jun 2017 17:31:24 -0400 Subject: [PATCH] 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 --- 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 | 273 +++++++++------------------- 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, 207 insertions(+), 232 deletions(-) create mode 100644 src/include/utils/pidfile.h diff --git a/src/backend/main/main.c b/src/backend/main/main.c index 09f99486e0..87b7d3bf65 100644 --- a/src/backend/main/main.c +++ b/src/backend/main/main.c @@ -169,7 +169,7 @@ main(int argc, char *argv[]) } if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-V") == 0) { - puts("postgres (PostgreSQL) " PG_VERSION); + fputs(PG_BACKEND_VERSIONSTR, stdout); exit(0); } diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c index 273d1313b0..e8cf6d3e93 100644 --- a/src/backend/port/sysv_shmem.c +++ b/src/backend/port/sysv_shmem.c @@ -38,6 +38,7 @@ #include "storage/ipc.h" #include "storage/pg_shmem.h" #include "utils/guc.h" +#include "utils/pidfile.h" /* diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 2874f635af..5f4dd68941 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -125,6 +125,7 @@ #include "utils/datetime.h" #include "utils/dynamic_loader.h" #include "utils/memutils.h" +#include "utils/pidfile.h" #include "utils/ps_status.h" #include "utils/timeout.h" #include "utils/varlena.h" @@ -1340,6 +1341,12 @@ PostmasterMain(int argc, char *argv[]) gettimeofday(&random_start_time, NULL); #endif + /* + * Report postmaster status in the postmaster.pid file, to allow pg_ctl to + * see what's happening. + */ + AddToDataDirLockFile(LOCK_FILE_LINE_PM_STATUS, PM_STATUS_STARTING); + /* * We're ready to rock and roll... */ @@ -2608,6 +2615,9 @@ pmdie(SIGNAL_ARGS) Shutdown = SmartShutdown; ereport(LOG, (errmsg("received smart shutdown request"))); + + /* Report status */ + AddToDataDirLockFile(LOCK_FILE_LINE_PM_STATUS, PM_STATUS_STOPPING); #ifdef USE_SYSTEMD sd_notify(0, "STOPPING=1"); #endif @@ -2663,6 +2673,9 @@ pmdie(SIGNAL_ARGS) Shutdown = FastShutdown; ereport(LOG, (errmsg("received fast shutdown request"))); + + /* Report status */ + AddToDataDirLockFile(LOCK_FILE_LINE_PM_STATUS, PM_STATUS_STOPPING); #ifdef USE_SYSTEMD sd_notify(0, "STOPPING=1"); #endif @@ -2727,6 +2740,9 @@ pmdie(SIGNAL_ARGS) Shutdown = ImmediateShutdown; ereport(LOG, (errmsg("received immediate shutdown request"))); + + /* Report status */ + AddToDataDirLockFile(LOCK_FILE_LINE_PM_STATUS, PM_STATUS_STOPPING); #ifdef USE_SYSTEMD sd_notify(0, "STOPPING=1"); #endif @@ -2872,6 +2888,8 @@ reaper(SIGNAL_ARGS) ereport(LOG, (errmsg("database system is ready to accept connections"))); + /* Report status */ + AddToDataDirLockFile(LOCK_FILE_LINE_PM_STATUS, PM_STATUS_READY); #ifdef USE_SYSTEMD sd_notify(0, "READY=1"); #endif @@ -5005,10 +5023,18 @@ sigusr1_handler(SIGNAL_ARGS) if (XLogArchivingAlways()) PgArchPID = pgarch_start(); -#ifdef USE_SYSTEMD + /* + * If we aren't planning to enter hot standby mode later, treat + * RECOVERY_STARTED as meaning we're out of startup, and report status + * accordingly. + */ if (!EnableHotStandby) + { + AddToDataDirLockFile(LOCK_FILE_LINE_PM_STATUS, PM_STATUS_STANDBY); +#ifdef USE_SYSTEMD sd_notify(0, "READY=1"); #endif + } pmState = PM_RECOVERY; } @@ -5024,6 +5050,8 @@ sigusr1_handler(SIGNAL_ARGS) ereport(LOG, (errmsg("database system is ready to accept read only connections"))); + /* Report status */ + AddToDataDirLockFile(LOCK_FILE_LINE_PM_STATUS, PM_STATUS_READY); #ifdef USE_SYSTEMD sd_notify(0, "READY=1"); #endif diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index 49a6afafe7..afbf8f8691 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -47,6 +47,7 @@ #include "utils/builtins.h" #include "utils/guc.h" #include "utils/memutils.h" +#include "utils/pidfile.h" #include "utils/syscache.h" #include "utils/varlena.h" @@ -1149,8 +1150,9 @@ TouchSocketLockFiles(void) * * Note: because we don't truncate the file, if we were to rewrite a line * with less data than it had before, there would be garbage after the last - * line. We don't ever actually do that, so not worth adding another kernel - * call to cover the possibility. + * line. While we could fix that by adding a truncate call, that would make + * the file update non-atomic, which we'd rather avoid. Therefore, callers + * should endeavor never to shorten a line once it's been written. */ void AddToDataDirLockFile(int target_line, const char *str) @@ -1193,18 +1195,25 @@ AddToDataDirLockFile(int target_line, const char *str) srcptr = srcbuffer; for (lineno = 1; lineno < target_line; lineno++) { - if ((srcptr = strchr(srcptr, '\n')) == NULL) - { - elog(LOG, "incomplete data in \"%s\": found only %d newlines while trying to add line %d", - DIRECTORY_LOCK_FILE, lineno - 1, target_line); - close(fd); - return; - } - srcptr++; + char *eol = strchr(srcptr, '\n'); + + if (eol == NULL) + break; /* not enough lines in file yet */ + srcptr = eol + 1; } memcpy(destbuffer, srcbuffer, srcptr - srcbuffer); destptr = destbuffer + (srcptr - srcbuffer); + /* + * Fill in any missing lines before the target line, in case lines are + * added to the file out of order. + */ + for (; lineno < target_line; lineno++) + { + if (destptr < destbuffer + sizeof(destbuffer)) + *destptr++ = '\n'; + } + /* * Write or rewrite the target line. */ diff --git a/src/bin/pg_ctl/Makefile b/src/bin/pg_ctl/Makefile index f5ec088c42..46f30bd46f 100644 --- a/src/bin/pg_ctl/Makefile +++ b/src/bin/pg_ctl/Makefile @@ -16,14 +16,12 @@ subdir = src/bin/pg_ctl top_builddir = ../../.. include $(top_builddir)/src/Makefile.global -override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS) - OBJS= pg_ctl.o $(WIN32RES) all: pg_ctl -pg_ctl: $(OBJS) | submake-libpq submake-libpgport - $(CC) $(CFLAGS) $(OBJS) $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X) +pg_ctl: $(OBJS) | submake-libpgport + $(CC) $(CFLAGS) $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X) install: all installdirs $(INSTALL_PROGRAM) pg_ctl$(X) '$(DESTDIR)$(bindir)/pg_ctl$(X)' diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index ad2a16f755..6f05790209 100644 --- a/src/bin/pg_ctl/pg_ctl.c +++ b/src/bin/pg_ctl/pg_ctl.c @@ -34,9 +34,7 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "getopt_long.h" -#include "libpq-fe.h" -#include "miscadmin.h" -#include "pqexpbuffer.h" +#include "utils/pidfile.h" /* PID can be negative for standalone backend */ typedef long pgpid_t; @@ -49,6 +47,12 @@ typedef enum IMMEDIATE_MODE } ShutdownMode; +typedef enum +{ + POSTMASTER_READY, + POSTMASTER_STILL_STARTING, + POSTMASTER_FAILED +} WaitPMResult; typedef enum { @@ -147,12 +151,12 @@ static int CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, #endif static pgpid_t get_pgpid(bool is_status_request); -static char **readfile(const char *path); +static char **readfile(const char *path, int *numlines); static void free_readfile(char **optlines); static pgpid_t start_postmaster(void); static void read_post_opts(void); -static PGPing test_postmaster_connection(pgpid_t pm_pid, bool do_checkpoint); +static WaitPMResult wait_for_postmaster(pgpid_t pm_pid, bool do_checkpoint); static bool postmaster_is_alive(pid_t pid); #if defined(HAVE_GETRLIMIT) && defined(RLIMIT_CORE) @@ -304,9 +308,14 @@ get_pgpid(bool is_status_request) /* * get the lines from a text file - return NULL if file can't be opened + * + * Trailing newlines are deleted from the lines (this is a change from pre-v10) + * + * *numlines is set to the number of line pointers returned; there is + * also an additional NULL pointer after the last real line. */ static char ** -readfile(const char *path) +readfile(const char *path, int *numlines) { int fd; int nlines; @@ -318,6 +327,8 @@ readfile(const char *path) int len; struct stat statbuf; + *numlines = 0; /* in case of failure or empty file */ + /* * Slurp the file into memory. * @@ -367,6 +378,7 @@ readfile(const char *path) /* set up the result buffer */ result = (char **) pg_malloc((nlines + 1) * sizeof(char *)); + *numlines = nlines; /* now split the buffer into lines */ linebegin = buffer; @@ -375,10 +387,13 @@ readfile(const char *path) { if (buffer[i] == '\n') { - int slen = &buffer[i] - linebegin + 1; + int slen = &buffer[i] - linebegin; char *linebuf = pg_malloc(slen + 1); memcpy(linebuf, linebegin, slen); + /* we already dropped the \n, but get rid of any \r too */ + if (slen > 0 && linebuf[slen - 1] == '\r') + slen--; linebuf[slen] = '\0'; result[n++] = linebuf; linebegin = &buffer[i + 1]; @@ -509,7 +524,7 @@ start_postmaster(void) /* - * Find the pgport and try a connection + * Wait for the postmaster to become ready. * * On Unix, pm_pid is the PID of the just-launched postmaster. On Windows, * it may be the PID of an ancestor shell process, so we can't check the @@ -522,168 +537,67 @@ start_postmaster(void) * Note that the checkpoint parameter enables a Windows service control * manager checkpoint, it's got nothing to do with database checkpoints!! */ -static PGPing -test_postmaster_connection(pgpid_t pm_pid, bool do_checkpoint) +static WaitPMResult +wait_for_postmaster(pgpid_t pm_pid, bool do_checkpoint) { - PGPing ret = PQPING_NO_RESPONSE; - char connstr[MAXPGPATH * 2 + 256]; int i; - /* if requested wait time is zero, return "still starting up" code */ - if (wait_seconds <= 0) - return PQPING_REJECT; - - connstr[0] = '\0'; - for (i = 0; i < wait_seconds * WAITS_PER_SEC; i++) { - /* Do we need a connection string? */ - if (connstr[0] == '\0') + char **optlines; + int numlines; + + /* + * Try to read the postmaster.pid file. If it's not valid, or if the + * status line isn't there yet, just keep waiting. + */ + if ((optlines = readfile(pid_file, &numlines)) != NULL && + numlines >= LOCK_FILE_LINE_PM_STATUS) { - /*---------- - * The number of lines in postmaster.pid tells us several things: - * - * # of lines - * 0 lock file created but status not written - * 2 pre-9.1 server, shared memory not created - * 3 pre-9.1 server, shared memory created - * 5 9.1+ server, ports not opened - * 6 9.1+ server, shared memory not created - * 7 9.1+ server, shared memory created - * - * This code does not support pre-9.1 servers. On Unix machines - * we could consider extracting the port number from the shmem - * key, but that (a) is not robust, and (b) doesn't help with - * finding out the socket directory. And it wouldn't work anyway - * on Windows. - * - * If we see less than 6 lines in postmaster.pid, just keep - * waiting. - *---------- - */ - char **optlines; - - /* Try to read the postmaster.pid file */ - if ((optlines = readfile(pid_file)) != NULL && - optlines[0] != NULL && - optlines[1] != NULL && - optlines[2] != NULL) - { - if (optlines[3] == NULL) - { - /* File is exactly three lines, must be pre-9.1 */ - write_stderr(_("\n%s: -w option is not supported when starting a pre-9.1 server\n"), - progname); - return PQPING_NO_ATTEMPT; - } - else if (optlines[4] != NULL && - optlines[5] != NULL) - { - /* File is complete enough for us, parse it */ - pgpid_t pmpid; - time_t pmstart; - - /* - * Make sanity checks. If it's for the wrong PID, or the - * recorded start time is before pg_ctl started, then - * either we are looking at the wrong data directory, or - * this is a pre-existing pidfile that hasn't (yet?) been - * overwritten by our child postmaster. Allow 2 seconds - * slop for possible cross-process clock skew. - */ - pmpid = atol(optlines[LOCK_FILE_LINE_PID - 1]); - pmstart = atol(optlines[LOCK_FILE_LINE_START_TIME - 1]); - if (pmstart >= start_time - 2 && -#ifndef WIN32 - pmpid == pm_pid -#else - /* Windows can only reject standalone-backend PIDs */ - pmpid > 0 -#endif - ) - { - /* - * OK, seems to be a valid pidfile from our child. - */ - int portnum; - char *sockdir; - char *hostaddr; - char host_str[MAXPGPATH]; - - /* - * Extract port number and host string to use. Prefer - * using Unix socket if available. - */ - portnum = atoi(optlines[LOCK_FILE_LINE_PORT - 1]); - sockdir = optlines[LOCK_FILE_LINE_SOCKET_DIR - 1]; - hostaddr = optlines[LOCK_FILE_LINE_LISTEN_ADDR - 1]; - - /* - * While unix_socket_directories can accept relative - * directories, libpq's host parameter must have a - * leading slash to indicate a socket directory. So, - * ignore sockdir if it's relative, and try to use TCP - * instead. - */ - if (sockdir[0] == '/') - strlcpy(host_str, sockdir, sizeof(host_str)); - else - strlcpy(host_str, hostaddr, sizeof(host_str)); - - /* remove trailing newline */ - if (strchr(host_str, '\n') != NULL) - *strchr(host_str, '\n') = '\0'; - - /* Fail if couldn't get either sockdir or host addr */ - if (host_str[0] == '\0') - { - write_stderr(_("\n%s: -w option cannot use a relative socket directory specification\n"), - progname); - return PQPING_NO_ATTEMPT; - } - - /* - * Map listen-only addresses to counterparts usable - * for establishing a connection. connect() to "::" - * or "0.0.0.0" is not portable to OpenBSD 5.0 or to - * Windows Server 2008, and connect() to "::" is - * additionally not portable to NetBSD 6.0. (Cygwin - * does handle both addresses, though.) - */ - if (strcmp(host_str, "*") == 0) - strcpy(host_str, "localhost"); - else if (strcmp(host_str, "0.0.0.0") == 0) - strcpy(host_str, "127.0.0.1"); - else if (strcmp(host_str, "::") == 0) - strcpy(host_str, "::1"); - - /* - * We need to set connect_timeout otherwise on Windows - * the Service Control Manager (SCM) will probably - * timeout first. - */ - snprintf(connstr, sizeof(connstr), - "dbname=postgres port=%d host='%s' connect_timeout=5", - portnum, host_str); - } - } - } + /* File is complete enough for us, parse it */ + pgpid_t pmpid; + time_t pmstart; /* - * Free the results of readfile. - * - * This is safe to call even if optlines is NULL. + * Make sanity checks. If it's for the wrong PID, or the recorded + * start time is before pg_ctl started, then either we are looking + * at the wrong data directory, or this is a pre-existing pidfile + * that hasn't (yet?) been overwritten by our child postmaster. + * Allow 2 seconds slop for possible cross-process clock skew. */ - free_readfile(optlines); + pmpid = atol(optlines[LOCK_FILE_LINE_PID - 1]); + pmstart = atol(optlines[LOCK_FILE_LINE_START_TIME - 1]); + if (pmstart >= start_time - 2 && +#ifndef WIN32 + pmpid == pm_pid +#else + /* Windows can only reject standalone-backend PIDs */ + pmpid > 0 +#endif + ) + { + /* + * OK, seems to be a valid pidfile from our child. Check the + * status line (this assumes a v10 or later server). + */ + char *pmstatus = optlines[LOCK_FILE_LINE_PM_STATUS - 1]; + + if (strcmp(pmstatus, PM_STATUS_READY) == 0 || + strcmp(pmstatus, PM_STATUS_STANDBY) == 0) + { + /* postmaster is done starting up */ + free_readfile(optlines); + return POSTMASTER_READY; + } + } } - /* If we have a connection string, ping the server */ - if (connstr[0] != '\0') - { - ret = PQping(connstr); - if (ret == PQPING_OK || ret == PQPING_NO_ATTEMPT) - break; - } + /* + * Free the results of readfile. + * + * This is safe to call even if optlines is NULL. + */ + free_readfile(optlines); /* * Check whether the child postmaster process is still alive. This @@ -697,14 +611,14 @@ test_postmaster_connection(pgpid_t pm_pid, bool do_checkpoint) int exitstatus; if (waitpid((pid_t) pm_pid, &exitstatus, WNOHANG) == (pid_t) pm_pid) - return PQPING_NO_RESPONSE; + return POSTMASTER_FAILED; } #else if (WaitForSingleObject(postmasterProcess, 0) == WAIT_OBJECT_0) - return PQPING_NO_RESPONSE; + return POSTMASTER_FAILED; #endif - /* No response, or startup still in process; wait */ + /* Startup still in process; wait, printing a dot once per second */ if (i % WAITS_PER_SEC == 0) { #ifdef WIN32 @@ -729,8 +643,8 @@ test_postmaster_connection(pgpid_t pm_pid, bool do_checkpoint) pg_usleep(USEC_PER_SEC / WAITS_PER_SEC); } - /* return result of last call to PQping */ - return ret; + /* out of patience; report that postmaster is still starting up */ + return POSTMASTER_STILL_STARTING; } @@ -764,14 +678,15 @@ read_post_opts(void) if (ctl_command == RESTART_COMMAND) { char **optlines; + int numlines; - optlines = readfile(postopts_file); + optlines = readfile(postopts_file, &numlines); if (optlines == NULL) { write_stderr(_("%s: could not read file \"%s\"\n"), progname, postopts_file); exit(1); } - else if (optlines[0] == NULL || optlines[1] != NULL) + else if (numlines != 1) { write_stderr(_("%s: option file \"%s\" must have exactly one line\n"), progname, postopts_file); @@ -779,14 +694,10 @@ read_post_opts(void) } else { - int len; char *optline; char *arg1; optline = optlines[0]; - /* trim off line endings */ - len = strcspn(optline, "\r\n"); - optline[len] = '\0'; /* * Are we at the first option, as defined by space and @@ -917,28 +828,23 @@ do_start(void) { print_msg(_("waiting for server to start...")); - switch (test_postmaster_connection(pm_pid, false)) + switch (wait_for_postmaster(pm_pid, false)) { - case PQPING_OK: + case POSTMASTER_READY: print_msg(_(" done\n")); print_msg(_("server started\n")); break; - case PQPING_REJECT: + case POSTMASTER_STILL_STARTING: print_msg(_(" stopped waiting\n")); print_msg(_("server is still starting up\n")); break; - case PQPING_NO_RESPONSE: + case POSTMASTER_FAILED: print_msg(_(" stopped waiting\n")); write_stderr(_("%s: could not start server\n" "Examine the log output.\n"), progname); exit(1); break; - case PQPING_NO_ATTEMPT: - print_msg(_(" failed\n")); - write_stderr(_("%s: could not wait for server because of misconfiguration\n"), - progname); - exit(1); } } else @@ -1319,15 +1225,16 @@ do_status(void) { char **optlines; char **curr_line; + int numlines; printf(_("%s: server is running (PID: %ld)\n"), progname, pid); - optlines = readfile(postopts_file); + optlines = readfile(postopts_file, &numlines); if (optlines != NULL) { for (curr_line = optlines; *curr_line != NULL; curr_line++) - fputs(*curr_line, stdout); + puts(*curr_line); /* Free the results of readfile */ free_readfile(optlines); @@ -1634,7 +1541,7 @@ pgwin32_ServiceMain(DWORD argc, LPTSTR *argv) if (do_wait) { write_eventlog(EVENTLOG_INFORMATION_TYPE, _("Waiting for server startup...\n")); - if (test_postmaster_connection(postmasterPID, true) != PQPING_OK) + if (wait_for_postmaster(postmasterPID, true) != POSTMASTER_READY) { write_eventlog(EVENTLOG_ERROR_TYPE, _("Timed out waiting for server startup\n")); pgwin32_SetServiceStatus(SERVICE_STOPPED); @@ -1655,7 +1562,7 @@ pgwin32_ServiceMain(DWORD argc, LPTSTR *argv) { /* * status.dwCheckPoint can be incremented by - * test_postmaster_connection(), so it might not start from 0. + * wait_for_postmaster(), so it might not start from 0. */ int maxShutdownCheckPoint = status.dwCheckPoint + 12; diff --git a/src/bin/pg_ctl/t/001_start_stop.pl b/src/bin/pg_ctl/t/001_start_stop.pl index df6158bd1f..9c3551f1a5 100644 --- a/src/bin/pg_ctl/t/001_start_stop.pl +++ b/src/bin/pg_ctl/t/001_start_stop.pl @@ -4,7 +4,7 @@ use warnings; use Config; use PostgresNode; use TestLib; -use Test::More tests => 17; +use Test::More tests => 19; my $tempdir = TestLib::tempdir; my $tempdir_short = TestLib::tempdir_short; @@ -32,12 +32,14 @@ else print $conf "listen_addresses = '127.0.0.1'\n"; } close $conf; -command_ok([ 'pg_ctl', 'start', '-D', "$tempdir/data" ], 'pg_ctl start'); +command_like([ 'pg_ctl', 'start', '-D', "$tempdir/data", + '-l', "$TestLib::log_path/001_start_stop_server.log" ], + qr/done.*server started/s, 'pg_ctl start'); # sleep here is because Windows builds can't check postmaster.pid exactly, # so they may mistake a pre-existing postmaster.pid for one created by the # postmaster they start. Waiting more than the 2 seconds slop time allowed -# by test_postmaster_connection prevents that mistake. +# by wait_for_postmaster() prevents that mistake. sleep 3 if ($windows_os); command_fails([ 'pg_ctl', 'start', '-D', "$tempdir/data" ], 'second pg_ctl start fails'); diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c index de5bfddf5c..cb77665cda 100644 --- a/src/bin/pg_upgrade/option.c +++ b/src/bin/pg_upgrade/option.c @@ -14,8 +14,8 @@ #include #endif -#include "miscadmin.h" #include "getopt_long.h" +#include "utils/pidfile.h" #include "pg_upgrade.h" diff --git a/src/common/config_info.c b/src/common/config_info.c index ad506be9ca..e0841a5af2 100644 --- a/src/common/config_info.c +++ b/src/common/config_info.c @@ -21,7 +21,6 @@ #endif #include "common/config_info.h" -#include "miscadmin.h" /* diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 21a77289f8..dad98de98d 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -28,8 +28,6 @@ #include "pgtime.h" /* for pg_time_t */ -#define PG_BACKEND_VERSIONSTR "postgres (PostgreSQL) " PG_VERSION "\n" - #define InvalidPid (-1) @@ -431,31 +429,6 @@ extern char *session_preload_libraries_string; extern char *shared_preload_libraries_string; extern char *local_preload_libraries_string; -/* - * As of 9.1, the contents of the data-directory lock file are: - * - * line # - * 1 postmaster PID (or negative of a standalone backend's PID) - * 2 data directory path - * 3 postmaster start timestamp (time_t representation) - * 4 port number - * 5 first Unix socket directory path (empty if none) - * 6 first listen_address (IP address or "*"; empty if no TCP port) - * 7 shared memory key (not present on Windows) - * - * Lines 6 and up are added via AddToDataDirLockFile() after initial file - * creation. - * - * The socket lock file, if used, has the same contents as lines 1-5. - */ -#define LOCK_FILE_LINE_PID 1 -#define LOCK_FILE_LINE_DATA_DIR 2 -#define LOCK_FILE_LINE_START_TIME 3 -#define LOCK_FILE_LINE_PORT 4 -#define LOCK_FILE_LINE_SOCKET_DIR 5 -#define LOCK_FILE_LINE_LISTEN_ADDR 6 -#define LOCK_FILE_LINE_SHMEM_KEY 7 - extern void CreateDataDirLockFile(bool amPostmaster); extern void CreateSocketLockFile(const char *socketfile, bool amPostmaster, const char *socketDir); diff --git a/src/include/port.h b/src/include/port.h index c2462167e4..b1ba645655 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -98,6 +98,9 @@ extern int find_my_exec(const char *argv0, char *retpath); extern int find_other_exec(const char *argv0, const char *target, const char *versionstr, char *retpath); +/* Doesn't belong here, but this is used with find_other_exec(), so... */ +#define PG_BACKEND_VERSIONSTR "postgres (PostgreSQL) " PG_VERSION "\n" + /* Windows security token manipulation (in exec.c) */ #ifdef WIN32 extern BOOL AddUserToTokenDacl(HANDLE hToken); diff --git a/src/include/utils/pidfile.h b/src/include/utils/pidfile.h new file mode 100644 index 0000000000..c3db4c46e3 --- /dev/null +++ b/src/include/utils/pidfile.h @@ -0,0 +1,55 @@ +/*------------------------------------------------------------------------- + * + * pidfile.h + * Declarations describing the data directory lock file (postmaster.pid) + * + * Portions Copyright (c) 1996-2017, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/utils/pidfile.h + * + *------------------------------------------------------------------------- + */ +#ifndef UTILS_PIDFILE_H +#define UTILS_PIDFILE_H + +/* + * As of Postgres 10, the contents of the data-directory lock file are: + * + * line # + * 1 postmaster PID (or negative of a standalone backend's PID) + * 2 data directory path + * 3 postmaster start timestamp (time_t representation) + * 4 port number + * 5 first Unix socket directory path (empty if none) + * 6 first listen_address (IP address or "*"; empty if no TCP port) + * 7 shared memory key (empty on Windows) + * 8 postmaster status (see values below) + * + * Lines 6 and up are added via AddToDataDirLockFile() after initial file + * creation; also, line 5 is initially empty and is changed after the first + * Unix socket is opened. + * + * Socket lock file(s), if used, have the same contents as lines 1-5, with + * line 5 being their own directory. + */ +#define LOCK_FILE_LINE_PID 1 +#define LOCK_FILE_LINE_DATA_DIR 2 +#define LOCK_FILE_LINE_START_TIME 3 +#define LOCK_FILE_LINE_PORT 4 +#define LOCK_FILE_LINE_SOCKET_DIR 5 +#define LOCK_FILE_LINE_LISTEN_ADDR 6 +#define LOCK_FILE_LINE_SHMEM_KEY 7 +#define LOCK_FILE_LINE_PM_STATUS 8 + +/* + * The PM_STATUS line may contain one of these values. All these strings + * must be the same length, per comments for AddToDataDirLockFile(). + * We pad with spaces as needed to make that true. + */ +#define PM_STATUS_STARTING "starting" /* still starting up */ +#define PM_STATUS_STOPPING "stopping" /* in shutdown sequence */ +#define PM_STATUS_READY "ready " /* ready for connections */ +#define PM_STATUS_STANDBY "standby " /* up, won't accept connections */ + +#endif /* UTILS_PIDFILE_H */ diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index 621238417d..0179844349 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -49,7 +49,7 @@ my @contrib_excludes = ( # Set of variables for frontend modules my $frontend_defines = { 'initdb' => 'FRONTEND' }; -my @frontend_uselibpq = ('pg_ctl', 'pg_upgrade', 'pgbench', 'psql', 'initdb'); +my @frontend_uselibpq = ('pg_upgrade', 'pgbench', 'psql', 'initdb'); my @frontend_uselibpgport = ( 'pg_archivecleanup', 'pg_test_fsync', 'pg_test_timing', 'pg_upgrade',