From: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
---|---|
To: | Peter Geoghegan <peter(at)2ndquadrant(dot)com> |
Cc: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>, Florian Pflug <fgp(at)phlo(dot)org> |
Subject: | Re: Latch implementation that wakes on postmaster death on both win32 and Unix |
Date: | 2011-06-30 06:36:17 |
Message-ID: | BANLkTinr8KecEkTug4tOaEAsfR1z0wA64A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Jun 25, 2011 at 10:41 AM, Peter Geoghegan <peter(at)2ndquadrant(dot)com> wrote:
> Attached is patch that addresses Fujii's third and most recent set of concerns.
Thanks for updating the patch!
> I think that Heikki is currently taking another look at my work,
> because he indicates in a new message to the list a short time ago
> that while reviewing my patch, he realised that there may be an
> independent problem with silent_mode. I will wait for his remarks
> before producing another version of the patch that incorporates those
> two small changes.
Yes, we should wait for the comments from Heikki. But, I have another
comments;
InitPostmasterDeathWatchHandle() can be static function because it's
used only in postmaster.c.
ReleasePostmasterDeathWatchHandle() can call ereport(FATAL) before
StartChildProcess() or BackendStartup() calls on_exit_reset() and resets
MyProcPid. This looks unsafe. If that ereport(FATAL) is unfortunately
called, a process other than postmaster would perform the postmaster's
proc-exit handlers. And that ereport(FATAL) would report wrong pid
when %p is specified in log_line_prefix. What about closing the pipe in
ClosePostmasterPorts() and removing ReleasePostmasterDeathWatchHandle()?
+ /*
+ * Set O_NONBLOCK to allow checking for the fd's presence with a select() call
+ */
+ if (fcntl(postmaster_alive_fds[POSTMASTER_FD_WATCH], F_SETFL, O_NONBLOCK))
+ {
+ ereport(FATAL,
+ (errcode_for_socket_access(),
+ errmsg("failed to set the postmaster death watching fd's flags: %m")));
+ }
I don't think that the pipe fd needs to be set to non-blocking mode
since we don't read or write on it.
http://developer.postgresql.org/pgdocs/postgres/error-style-guide.html
According to the error style guide, I think that it's better to change the
following messages:
+ errmsg( "pipe() call failed to create pipe to monitor postmaster
death: %m")));
"could not create pipe for monitoring postmaster death: %m"
+ errmsg("failed to close file descriptor associated with
postmaster death in child process")));
"could not close postmaster pipe: %m"
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | Jun Ishiduka | 2011-06-30 06:38:45 | Re: Online base backup from the hot-standby |
Previous Message | Pavel Stehule | 2011-06-30 06:36:01 | ToDo: list of active channels |