Обсуждение: [HACKERS] Removing select(2) based latch (was Unportable implementation ofbackground worker start)
On 2017-04-19 20:06:05 -0400, Tom Lane wrote: > > BTW, we IIRC had discussed removing the select() backed latch > > implementation in this release. I'll try to dig up that discussion. > > Might be sensible. Even my pet dinosaurs have poll(2). I can't find the discussion anymore, but I'm fairly sure we did discuss that. So here's a new one. > We should check the buildfarm to see if the select() implementation is > being tested at all. I verified it's currently not (unless I made a mistake): pgbfprod=> select bs.name, snapshot FROM buildsystems bs, lateral ( SELECT * FROM build_status_log bsl WHERE bs.name = bsl.sysname AND log_stage = 'configure.log' ORDER BY bsl.sysname, bsl.snapshot desc, bsl.log_stage limit 1) bsl2 WHERE log_text NOT LIKE '%noconfigure step for MSCV%' AND log_text NOT LIKE '%checking for poll... yes%' order by snapshot desc; name | snapshot -----------+---------------------frogmouth | 2017-04-19 00:32:32jacana | 2017-04-19 00:00:58narwhal | 2017-04-18 07:00:01 (3 rows) and those three animals are windows, and thus use the windows implementation. This actually seems to suggest that no animal, including dead ones, didn't have poll(2) support at the time of their last run... - Andres
Andres Freund <andres@anarazel.de> writes:
> On 2017-04-19 20:06:05 -0400, Tom Lane wrote:
>> We should check the buildfarm to see if the select() implementation is
>> being tested at all.
> I verified it's currently not (unless I made a mistake):
I did my own checking, and concur that only the MinGW buildfarm members
are reporting lacking poll(2) or poll.h. Since they also report lacking
sys/select.h, they must be falling through to the WAIT_USE_WIN32
implementation. So the WAIT_USE_SELECT implementation is going untested
in the buildfarm, and has been since before the WaitEventSet API existed
(I checked buildfarm records back to the start of 2016).
Aside from that empirical observation, we can make a standards-compliance
argument, which is that both poll(2) and poll.h are required by the Single
Unix Spec v2 (a/k/a POSIX 1997), which is what we've been taking as our
baseline requirement for Unix platforms for ten or fifteen years now.
In short: yeah, let's nuke the WAIT_USE_SELECT implementation.
It's dead code and it's unlikely to get resurrected.
BTW, noting that SUSv2 specifies <poll.h> not <sys/poll.h>, I wonder
whether we couldn't drop configure's test for the latter along with
the
#ifdef HAVE_SYS_POLL_H
#include <sys/poll.h>
#endif
stanzas we have in a couple of places. But that's a separate issue.
regards, tom lane
Hi, On 2017-04-20 17:27:42 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2017-04-19 20:06:05 -0400, Tom Lane wrote: > >> We should check the buildfarm to see if the select() implementation is > >> being tested at all. > > > I verified it's currently not (unless I made a mistake): > > I did my own checking, and concur that only the MinGW buildfarm members > are reporting lacking poll(2) or poll.h. Since they also report lacking > sys/select.h, they must be falling through to the WAIT_USE_WIN32 > implementation. So the WAIT_USE_SELECT implementation is going untested > in the buildfarm, and has been since before the WaitEventSet API existed > (I checked buildfarm records back to the start of 2016). Similarly with the SELECT based unix_latch.c code before... > In short: yeah, let's nuke the WAIT_USE_SELECT implementation. > It's dead code and it's unlikely to get resurrected. Ok, cool. v10 or wait till v11? I see very little reason to wait personally. > BTW, noting that SUSv2 specifies <poll.h> not <sys/poll.h>, I wonder > whether we couldn't drop configure's test for the latter along with > the > > #ifdef HAVE_SYS_POLL_H > #include <sys/poll.h> > #endif > > stanzas we have in a couple of places. But that's a separate issue. Seems like a good idea, a quick select confirms there's no !win buildfarm animals without poll.h. Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2017-04-20 17:27:42 -0400, Tom Lane wrote:
>> In short: yeah, let's nuke the WAIT_USE_SELECT implementation.
>> It's dead code and it's unlikely to get resurrected.
> Ok, cool. v10 or wait till v11? I see very little reason to wait
> personally.
I feel no need to wait on that. Code removal is not a "new feature".
regards, tom lane
I wrote:
> I did my own checking, and concur that only the MinGW buildfarm members
> are reporting lacking poll(2) or poll.h. Since they also report lacking
> sys/select.h, they must be falling through to the WAIT_USE_WIN32
> implementation.
BTW, another amusing thing I just noted is that given a machine too old
to have poll(2), the existing coding would most likely fail outright,
because <sys/select.h> post-dates poll(2). SUSv2 specifies including
<sys/time.h> to get select(2); apparently POSIX invented <sys/select.h>
around 2001. gaur/pademelon indeed lacks <sys/select.h>, so it could
never have reached the WAIT_USE_SELECT implementation without some
readjustment of that #ifdef nest.
regards, tom lane
On Thu, Apr 20, 2017 at 5:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Andres Freund <andres@anarazel.de> writes: >> On 2017-04-20 17:27:42 -0400, Tom Lane wrote: >>> In short: yeah, let's nuke the WAIT_USE_SELECT implementation. >>> It's dead code and it's unlikely to get resurrected. > >> Ok, cool. v10 or wait till v11? I see very little reason to wait >> personally. > > I feel no need to wait on that. Code removal is not a "new feature". One can imagine a situation in which code removal seemed to carry a risk of destabilizing something, but the change under discussion here seems more likely to improve stability rather than to regress anything. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hi, On 2017-04-20 17:27:42 -0400, Tom Lane wrote: > In short: yeah, let's nuke the WAIT_USE_SELECT implementation. > It's dead code and it's unlikely to get resurrected. Done. > BTW, noting that SUSv2 specifies <poll.h> not <sys/poll.h>, I wonder > whether we couldn't drop configure's test for the latter along with > the > > #ifdef HAVE_SYS_POLL_H > #include <sys/poll.h> > #endif > > stanzas we have in a couple of places. But that's a separate issue. Done, too, in a separate commit. - Andres