Horton

Thread

Commits

  1. Don't include sys/poll.h anymore.

  2. Remove select(2) backed latch implementation.

  1. Removing select(2) based latch (was Unportable implementation of background worker start)

    Andres Freund <andres@anarazel.de> — 2017-04-20T00:36:11Z

    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 '%no configure step for MSCV%'
        AND log_text NOT LIKE '%checking for poll... yes%' order by snapshot desc;
   name    |      snapshot
-----------+---------------------
 frogmouth | 2017-04-19 00:32:32
 jacana    | 2017-04-19 00:00:58
 narwhal   | 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

  2. Re: Removing select(2) based latch (was Unportable implementation of background worker start)

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-04-20T21:27:42Z

    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

  3. Re: Removing select(2) based latch (was Unportable implementation of background worker start)

    Andres Freund <andres@anarazel.de> — 2017-04-20T21:35:46Z

    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

  4. Re: Removing select(2) based latch (was Unportable implementation of background worker start)

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-04-20T21:50:39Z

    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

  5. Re: Removing select(2) based latch (was Unportable implementation of background worker start)

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-04-20T22:09:08Z

    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

  6. Re: Removing select(2) based latch (was Unportable implementation of background worker start)

    Robert Haas <robertmhaas@gmail.com> — 2017-04-23T15:50:55Z

    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

  7. Re: Removing select(2) based latch (was Unportable implementation of background worker start)

    Andres Freund <andres@anarazel.de> — 2017-04-23T23:13:08Z

    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