On Sat, Jun 25, 2011 at 10:41 AM, Peter Geoghegan <pe...@2ndquadrant.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
