Subject: Re: [COMMITTERS] pgsql: Use asynchronous connect API in
 libpqwalreceiver
To: Tom Lane <tgl@sss.pgh.pa.us>,
 Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
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>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
 pgsql-hackers@postgresql.org
From: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Message-ID: <5b6a6d6d-fb45-0afb-2e95-5600063c3dbd@2ndquadrant.com>
Date: Fri, 17 Mar 2017 16:54:03 +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: <10458.1489763266@sss.pgh.pa.us>
Content-Type: multipart/mixed;
 boundary="------------5D0F87609BC1FFB284B7AAAA"
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

This is a multi-part message in MIME format.
--------------5D0F87609BC1FFB284B7AAAA
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit

On 17/03/17 16:07, Tom Lane wrote:
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
>> I have confirmed on jacana Petr's observation that adding a timeout to
>> the WaitLatchOrSocket cures the problem.
> 
> Does anyone have a theory as to why that cures the problem?
> 

I now have theory and even PoC patch for it.

The long wait of WaitLatchOrSocket happens after connection state
switches from CONNECTION_STARTED to CONNECTION_MADE in both cases the
return value of PQconnectPoll is PGRES_POLLING_WRITING so we wait for
FD_WRITE.

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.

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.

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

--------------5D0F87609BC1FFB284B7AAAA
Content-Type: text/x-patch;
 name="fix-wait-for-writable-socket-on-windows.patch"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
 filename="fix-wait-for-writable-socket-on-windows.patch"

diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latc=
h.c
index ea7f930..300b866 100644
--- a/src/backend/storage/ipc/latch.c
+++ b/src/backend/storage/ipc/latch.c
@@ -1401,6 +1401,37 @@ WaitEventSetWaitBlock(WaitEventSet *set, int cur_tim=
eout,
 			WaitEventAdjustWin32(set, cur_event);
 			cur_event->reset =3D false;
 		}
+
+		if (cur_event->events & WL_SOCKET_WRITEABLE)
+		{
+			char		c;
+			WSABUF		buf;
+			DWORD		sent;
+			int			r;
+
+			buf.buf =3D &c;
+			buf.len =3D 0;
+
+			r =3D WSASend(cur_event->fd, &buf, 1, &sent, 0, NULL, NULL);
+			if (r =3D=3D 0)
+			{
+				occurred_events->pos =3D cur_event->pos;
+				occurred_events->user_data =3D cur_event->user_data;
+				occurred_events->events =3D WL_SOCKET_WRITEABLE;
+				occurred_events->fd =3D cur_event->fd;
+				return 1;
+			}
+			else
+			{
+				if (WSAGetLastError() !=3D WSAEWOULDBLOCK)
+					/*
+					 * Not completed, and not just "would block", so an error
+					 * occurred
+					 */
+					elog(ERROR, "failed writability check on socket: error code %u",
+						 WSAGetLastError());
+			}
+		}
 	}
=20
 	/*

--------------5D0F87609BC1FFB284B7AAAA
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


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

--------------5D0F87609BC1FFB284B7AAAA--