On April 21, 2015 09:34:51 PM Jan de Visser wrote:
> On April 21, 2015 09:01:14 PM Jan de Visser wrote:
> > On April 21, 2015 07:32:05 PM Payal Singh wrote:
... snip ...
>
> Urgh. It appears you are right. Will fix.
>
> jan
Attached a new attempt. This was one from the category 'I have no idea how
that ever worked", but whatever. For reference, this is how it looks for me
(magic man-behind-the-curtain postgresql.conf editing omitted):
jan@wolverine:~/Projects/postgresql$ initdb -D data
... Bla bla bla ...
jan@wolverine:~/Projects/postgresql$ pg_ctl -D data -l logfile start
server starting
jan@wolverine:~/Projects/postgresql$ tail -5 logfile
LOG: database system was shut down at 2015-04-21 22:03:33 EDT
LOG: database system is ready to accept connections
LOG: autovacuum launcher started
jan@wolverine:~/Projects/postgresql$ pg_ctl -D data reload
server signaled
jan@wolverine:~/Projects/postgresql$ tail -5 logfile
LOG: database system was shut down at 2015-04-21 22:03:33 EDT
LOG: database system is ready to accept connections
LOG: autovacuum launcher started
LOG: received SIGHUP, reloading configuration files
jan@wolverine:~/Projects/postgresql$ pg_ctl -D data reload
server signaled
pg_ctl: Reload of server with PID 14656 FAILED
Consult the server log for details.
jan@wolverine:~/Projects/postgresql$ tail -5 logfile
LOG: autovacuum launcher started
LOG: received SIGHUP, reloading configuration files
LOG: received SIGHUP, reloading configuration files
LOG: syntax error in file "/home/jan/Projects/postgresql/data/postgresql.conf"
line 1, near end of line
LOG: configuration file "/home/jan/Projects/postgresql/data/postgresql.conf"
contains errors; no changes were applied
jan@wolverine:~/Projects/postgresql$
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index a9f20ac..a7819d2 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1222,6 +1222,15 @@ PostmasterMain(int argc, char *argv[])
#endif
/*
+ * Update postmaster.pid with startup time as the last reload time:
+ */
+ {
+ char last_reload_info[32];
+ snprintf(last_reload_info, 32, "%ld %d", (long) MyStartTime, 1);
+ AddToDataDirLockFile(LOCK_FILE_LINE_LAST_RELOAD, last_reload_info);
+ }
+
+ /*
* Remember postmaster startup time
*/
PgStartTime = GetCurrentTimestamp();
@@ -2341,6 +2350,8 @@ static void
SIGHUP_handler(SIGNAL_ARGS)
{
int save_errno = errno;
+ bool reload_success;
+ char last_reload_info[32];
PG_SETMASK(&BlockSig);
@@ -2348,7 +2359,16 @@ SIGHUP_handler(SIGNAL_ARGS)
{
ereport(LOG,
(errmsg("received SIGHUP, reloading configuration files")));
- ProcessConfigFile(PGC_SIGHUP);
+ reload_success = ProcessConfigFile(PGC_SIGHUP);
+
+ /*
+ * Write the current time and the result of the reload to the
+ * postmaster.pid file.
+ */
+ snprintf(last_reload_info, 32, "%ld %d",
+ (long) time(NULL), reload_success);
+ AddToDataDirLockFile(LOCK_FILE_LINE_LAST_RELOAD, last_reload_info);
+
SignalChildren(SIGHUP);
SignalUnconnectedWorkers(SIGHUP);
if (StartupPID != 0)
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index c5e0fac..3162cd5 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -109,7 +109,7 @@ STRING \'([^'\\\n]|\\.|\'\')*\'
* All options mentioned in the configuration file are set to new values.
* If an error occurs, no values will be changed.
*/
-void
+bool
ProcessConfigFile(GucContext context)
{
bool error = false;
@@ -202,7 +202,7 @@ ProcessConfigFile(GucContext context)
* the config file.
*/
if (head == NULL)
- return;
+ return false;
}
/*
@@ -430,6 +430,7 @@ ProcessConfigFile(GucContext context)
* freed here.
*/
FreeConfigVariables(head);
+ return !error;
}
/*
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 80d7bc7..0ffe97b 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -73,6 +73,20 @@ typedef enum
RUN_AS_SERVICE_COMMAND
} CtlCommand;
+typedef struct
+{
+ pgpid_t pid;
+ char *datadir;
+ time_t startup_ts;
+ int port;
+ char *socketdir;
+ char *listenaddr;
+ unsigned long shmkey;
+ int shmid;
+ time_t reload_ts;
+ bool reload_ok;
+} PIDFileContents;
+
#define DEFAULT_WAIT 60
static bool do_wait = false;
@@ -153,6 +167,8 @@ static int CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo,
static pgpid_t get_pgpid(bool is_status_request);
static char **readfile(const char *path);
static void free_readfile(char **optlines);
+static PIDFileContents * get_pidfile_contents(const char *path);
+static void free_pidfile_contents(PIDFileContents *contents);
static int start_postmaster(void);
static void read_post_opts(void);
@@ -415,6 +431,78 @@ free_readfile(char **optlines)
}
/*
+ * Read and parse the contents of a postmaster.pid file. These contents are
+ * placed in a PIDFileContents struct, a pointer to which is returned. If the
+ * file could not be read NULL is returned.
+ */
+static PIDFileContents *
+get_pidfile_contents(const char *path)
+{
+ char **optlines = readfile(path);
+ PIDFileContents *result = NULL;
+ int lines;
+
+ if (optlines && optlines[0]) {
+ /*
+ * Count number of lines returned:
+ */
+ for (lines = 0; optlines[lines]; lines++);
+
+ result = (PIDFileContents *) pg_malloc0(sizeof(PIDFileContents));
+ result -> pid = atol(optlines[LOCK_FILE_LINE_PID - 1]);
+ result->datadir = (lines >= LOCK_FILE_LINE_DATA_DIR)
+ ? pg_strdup(optlines[LOCK_FILE_LINE_DATA_DIR - 1])
+ : NULL;
+ result->startup_ts = (lines >= LOCK_FILE_LINE_START_TIME)
+ ? atol(optlines[LOCK_FILE_LINE_START_TIME - 1])
+ : 0L;
+ result->port = (lines >= LOCK_FILE_LINE_PORT)
+ ? atoi(optlines[LOCK_FILE_LINE_PORT - 1])
+ : 0;
+ result->socketdir = (lines >= LOCK_FILE_LINE_SOCKET_DIR)
+ ? pg_strdup(optlines[LOCK_FILE_LINE_SOCKET_DIR - 1])
+ : NULL;
+ result->listenaddr = (lines >= LOCK_FILE_LINE_LISTEN_ADDR)
+ ? pg_strdup(optlines[LOCK_FILE_LINE_LISTEN_ADDR - 1])
+ : NULL;
+ if (lines >= LOCK_FILE_LINE_SHMEM_KEY) {
+ sscanf(optlines[LOCK_FILE_LINE_SHMEM_KEY - 1], "%9lu %9d",
+ &result->shmkey, &result->shmid);
+ } else {
+ result->shmkey = 0;
+ result->shmid = 0;
+ }
+ if (lines >= LOCK_FILE_LINE_LAST_RELOAD) {
+ char *ptr = strchr(optlines[LOCK_FILE_LINE_LAST_RELOAD - 1], ' ');
+ *ptr = 0;
+ result->reload_ts = atol(optlines[LOCK_FILE_LINE_LAST_RELOAD - 1]);
+ result->reload_ok = (bool) atoi(ptr+1);
+ *ptr = ' ';
+ } else {
+ result->reload_ts = 0;
+ result->reload_ok = 0;
+ }
+ free_readfile(optlines);
+ }
+ return result;
+}
+
+/*
+ * Free the memory allocated by get_pidfile_contents.
+ */
+static void
+free_pidfile_contents(PIDFileContents *contents)
+{
+ if (contents) {
+ pg_free(contents->datadir);
+ pg_free(contents->socketdir);
+ pg_free(contents->listenaddr);
+ free(contents);
+ }
+}
+
+
+/*
* start/test/stop routines
*/
@@ -1093,7 +1181,10 @@ do_restart(void)
static void
do_reload(void)
{
- pgpid_t pid;
+ pgpid_t pid;
+ PIDFileContents *current_contents;
+ PIDFileContents *new_contents = NULL;
+ int retries;
pid = get_pgpid(false);
if (pid == 0) /* no pid file */
@@ -1112,14 +1203,101 @@ do_reload(void)
exit(1);
}
+ /*
+ * Load the current contents of the postmaster.pid, so that after the
+ * SIGHUP we can compare the reload timestamp with the one currently in
+ * the file.
+ */
+ current_contents = get_pidfile_contents(pid_file);
+ if (!current_contents) {
+ write_stderr(_("%s: PID file \"%s\" can not be read\n"), progname, pid_file);
+ write_stderr(_("Is server running?\n"));
+ exit(1);
+ }
+
if (kill((pid_t) pid, sig) != 0)
{
write_stderr(_("%s: could not send reload signal (PID: %ld): %s\n"),
progname, pid, strerror(errno));
exit(1);
}
-
print_msg(_("server signaled\n"));
+
+ /*
+ * - Pre-9.6 servers don't write the last-reload timestamp, so we can
+ * only assume everything was OK.
+ * - If -W was specified on the command line, the user doesn't care
+ * about the result and we leave.
+ */
+ if ((current_contents->reload_ts == 0) || !do_wait) {
+ return;
+ }
+
+ /*
+ * Load the current contents of the postmaster.pid file, and check
+ * if the reload timestamp is newer than the one in the current
+ * contents loaded before the SIGHUP. If the file cannot be read, or
+ * the timestamp is not newer, the reload hasn't finished yet. In that
+ * case sleep and try again, for a maximum of 5 times.
+ */
+ for (retries = 0; retries < 5; retries++) {
+
+ /*
+ * Wait 1 second - the first time around to give the postmaster the
+ * opportunity to actually rewrite postmaster.pid.
+ */
+ pg_usleep(1000000); /* 1 sec */
+
+ /*
+ * free_pidfile_contents knows how to deal with NULL pointers
+ * and therefore this is safe the first time around:
+ */
+ free_pidfile_contents(new_contents);
+ new_contents = get_pidfile_contents(pid_file);
+ if (new_contents) {
+
+ /*
+ * We were able to read the postmaster.pid file:
+ */
+ if (new_contents->reload_ts > current_contents->reload_ts) {
+
+ /*
+ * The reload timestamp is newer that the "old" timestamp:
+ */
+ if (new_contents->reload_ok) {
+
+ /*
+ * The reload was successful:
+ */
+ break;
+ } else {
+
+ /*
+ * The reload failed, probably because of errors in the
+ * config file. The server will continue to run (with the
+ * old config!) but will fail to start the next time
+ * it is restarted.
+ */
+ write_stderr(_("%s: Reload of server with PID %ld FAILED\n"),
+ progname, pid);
+ write_stderr(_("Consult the server log for details.\n"));
+ break;
+ }
+ }
+ }
+ }
+ free_pidfile_contents(new_contents);
+ free_pidfile_contents(current_contents);
+
+ if (retries >= 5) {
+
+ /*
+ * We couldn't read a new postmaster.pid after 5 retries.
+ */
+ write_stderr(_("%s: Server with PID %ld non-responsive after reload request\n"),
+ progname, pid);
+ write_stderr(_("Consult the server log for details.\n"));
+ }
}
@@ -2348,6 +2526,7 @@ main(int argc, char **argv)
do_wait = false;
break;
case STOP_COMMAND:
+ case RELOAD_COMMAND:
do_wait = true;
break;
default:
@@ -2358,7 +2537,6 @@ main(int argc, char **argv)
if (ctl_command == RELOAD_COMMAND)
{
sig = SIGHUP;
- do_wait = false;
}
if (pg_data)
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index eacfccb..991405f 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -444,6 +444,7 @@ extern char *local_preload_libraries_string;
#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_LAST_RELOAD 8
extern void CreateDataDirLockFile(bool amPostmaster);
extern void CreateSocketLockFile(const char *socketfile, bool amPostmaster,
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index ff78b70..6db7bd7 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -338,7 +338,7 @@ extern void EmitWarningsOnPlaceholders(const char *className);
extern const char *GetConfigOption(const char *name, bool missing_ok,
bool restrict_superuser);
extern const char *GetConfigOptionResetString(const char *name);
-extern void ProcessConfigFile(GucContext context);
+extern bool ProcessConfigFile(GucContext context);
extern void InitializeGUCOptions(void);
extern bool SelectConfigFiles(const char *userDoption, const char *progname);
extern void ResetAllOptions(void);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
