Re: Latch implementation that wakes on postmaster death on both win32 and Unix

On Mon, Jun 20, 2011 at 1:00 AM, Peter Geoghegan <peter@2ndquadrant.com> wrote:
> I took another look at this this evening, and realised that my
> comments could be a little clearer.
>
> Attached revision cleans them up a bit.

Since I'm not familiar with Windows, I haven't read the code related
to Windows. But
the followings are my comments on your patch.

+        if (wakeEvents & WL_POSTMASTER_DEATH)
+        {
+            FD_SET(postmaster_alive_fds[POSTMASTER_FD_WATCH], &input_mask);
+            if (postmaster_alive_fds[POSTMASTER_FD_WATCH] > hifd)
+                hifd = postmaster_alive_fds[POSTMASTER_FD_WATCH];
+        }        hifd = selfpipe_readfd;

'hifd' should be initialized to 'selfpipe_readfd' before the above
'if' block. Otherwise,
'hifd = postmaster_alive_fds[POSTMASTER_FD_WATCH]' might have no effect.

+            time_t         curtime = time(NULL);
+            unsigned int timeout_secs  = (unsigned int) PGARCH_AUTOWAKE_INTERVAL -
+                    (unsigned int) (curtime - last_copy_time);
+            WaitLatch(&mainloop_latch, WL_LATCH_SET | WL_TIMEOUT |
WL_POSTMASTER_DEATH, timeout_secs * 1000000L);

Why does the archive still need to wake up periodically?

+    flags |= FNONBLOCK;
+    if (fcntl(postmaster_alive_fds[POSTMASTER_FD_WATCH], F_SETFL, FNONBLOCK))

Is the variable 'flag' really required? It's not used by fcntl() to
set the fd nonblocking.

Is FNONBLOCK equal to O_NONBLOCK? If yes, we should use O_NONBLOCK
for the sake of consistency? In other code (e.g., noblock.c), O_NONBLOCK is used
rather than FNONBLOCK.

+            WaitLatchOrSocket(&MyWalSnd->latch,
+                              WL_LATCH_SET | WL_SOCKET_READABLE | (pq_is_send_pending()?
WL_SOCKET_WRITEABLE:0) |  WL_TIMEOUT,
+                              MyProcPort->sock,

I think that it's worth that walsender checks the postmaster death event. No?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center