Subject: Re: [COMMITTERS] pgsql: Use asynchronous connect API in
 libpqwalreceiver
To: Tom Lane <tgl@sss.pgh.pa.us>
References: <E1cjo1g-0000mr-ST@gemulon.postgresql.org>
 <19403.1488586564@sss.pgh.pa.us>
 <34d98fbf-9195-04d2-b85a-634d6952ffdf@2ndquadrant.com>
 <19815.1488600693@sss.pgh.pa.us>
 <4144256b-dc4f-892c-a720-71f6e7b9ac87@2ndquadrant.com>
 <dcef4616-21af-5ec0-4113-d409e5ddbf86@2ndquadrant.com>
 <3ffe98f7-d2ed-65c6-51f3-4d97ffd6e312@2ndquadrant.com>
 <e31d81e2-392a-dca0-a700-e28b583bd2d3@2ndQuadrant.com>
 <34591606-976a-bbee-8eda-778c1e1fc52f@2ndQuadrant.com>
 <10458.1489763266@sss.pgh.pa.us>
 <5b6a6d6d-fb45-0afb-2e95-5600063c3dbd@2ndquadrant.com>
 <13398.1489768097@sss.pgh.pa.us>
Cc: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>,
 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
 pgsql-hackers@postgresql.org
From: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Message-ID: <dd5d7eae-cc0a-88a1-193f-88893b6963da@2ndquadrant.com>
Date: Fri, 17 Mar 2017 17:36:00 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.7.0
MIME-Version: 1.0
In-Reply-To: <13398.1489768097@sss.pgh.pa.us>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On 17/03/17 17:28, Tom Lane wrote:
> Petr Jelinek <petr.jelinek@2ndquadrant.com> writes:
>> Now the documentation for WSAEventSelect says "The FD_WRITE network
>> event is handled slightly differently. An FD_WRITE network event is
>> recorded when a socket is first connected with a call to the connect,
>> ConnectEx, WSAConnect, WSAConnectByList, or WSAConnectByName function or
>> when a socket is accepted with accept, AcceptEx, or WSAAccept function
>> and then after a send fails with WSAEWOULDBLOCK and buffer space becomes
>> available. Therefore, an application can assume that sends are possible
>> starting from the first FD_WRITE network event setting and lasting until
>> a send returns WSAEWOULDBLOCK. After such a failure the application will
>> find out that sends are again possible when an FD_WRITE network event is
>> recorded and the associated event object is set."
> 
>> But while PQconnectPoll does connect() before setting connection status
>> to CONNECTION_STARTED and returns  PGRES_POLLING_WRITING so the FD_WRITE
>> eventually happens, it does not do any writes in the code block that
>> switches to  CONNECTION_MADE and PGRES_POLLING_WRITING. That means
>> FD_WRITE event is never recorded as per quoted documentation.
> 
> Ah-hah!  Great detective work.
> 
>> Then what remains is why it works in libpq. If you look at
>> pgwin32_select which is eventually called by libpq code, it actually
>> handles the situation by trying empty WSASend on any socket that is
>> supposed to wait for FD_WRITE and only calling the
>> WaitForMultipleObjectsEx when WSASend failed with WSAEWOULDBLOCK, when
>> the WSASend succeeds it immediately returns ok.
> 
>> So I did something similar in attached for WaitEventSetWaitBlock() and
>> it indeed solves the issue my windows test machine. Now the
>> coding/placement probably isn't the best (and there are no comments),
>> but maybe somebody will find proper place for this now that we know the
>> cause.
> 
> Yeah, I'm afraid we had better do something more or less like this.
> It's interesting to speculate about whether WaitEventSet could keep
> additional state that would avoid the need for a dummy send() every
> time, but that seems like material for new development not a bug fix.
> 
> In any case, a quick review of the code suggests that there are no
> performance-critical places where we wait for WL_SOCKET_WRITEABLE
> unless we've already detected that we have to wait for the network;
> so the dummy send() isn't going to do anything except consume a
> few more CPU cycles before we get to the point of blocking.  It may
> not be worth worrying about.
> 

Thanks, now that I look at it again I think we might need to do cycle
with the occurred_events and returned_events and not return on first
find since theoretically there can be multiple sockets if this gets
called directly and not via WaitLatchOrSocket(). I don't have mental
power to finish it up right now though, sorry for that.

> I'll add some comments and push this.  Does everyone agree that
> this had better get back-patched, too?
> 

+1

-- 
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers