On Thu, Mar 17, 2016 at 10:53 AM, Andres Freund <andres@anarazel.de> wrote:
>> I wonder if there's a way to refactor this code to avoid having so
>> much cut-and-paste duplication.
>
> I guess you mean WaitEventSetWait() and WaitEventAdjust*? I've tried,
> and my attempt ended up look nearly unreadable, because of the number of
> ifdefs. I've not found a good attempt. Which is sad, because adding back
> select support is going to increase the duplication further :( - but
> it's also further away from poll etc. (different type of timestamp,
> entirely different way of returming events).
I was more thinking of stuff like this:
+ /*
+ * We expect an EPOLLHUP when the remote end is closed, but
+ * because we don't expect the pipe to become readable or to have
+ * any errors either, treat those cases as postmaster death, too.
+ *
+ * According to the select(2) man page on Linux, select(2) may
+ * spuriously return and report a file descriptor as readable,
+ * when it's not; and presumably so can epoll_wait(2). It's not
+ * clear that the relevant cases would ever apply to the
+ * postmaster pipe, but since the consequences of falsely
+ * returning WL_POSTMASTER_DEATH could be pretty unpleasant, we
+ * take the trouble to positively verify EOF with
+ * PostmasterIsAlive().
+ */
>> 0 at the top of the loop and skip it forthwith if so.
>
> You mean in WaitEventSetWait()? There's
> else if (rc == 0)
> {
> break;
> }
> which is the timeout case. There should never be any other case of
> returning 0 elements?
No, I meant if (cur_event->events == 0) continue;
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company