On 04/03/17 05:11, Tom Lane wrote:
> Peter Eisentraut <peter.eisentr...@2ndquadrant.com> writes:
>> On 3/3/17 19:16, Tom Lane wrote:
>>> Peter Eisentraut <pete...@gmx.net> writes:
>>>> Use asynchronous connect API in libpqwalreceiver
> 
>>> Buildfarm member bowerbird has been failing in the pg_rewind test since
>>> this patch went in.  It looks like it's failing to complete connections
>>> from the standby; which suggests that something platform-specific is
>>> missing from this commit, but I dunno what.
> 
>> Hmm, I wonder how widely tested the async connection API is on Windows
>> at all.  I only see bowerbird and jacana running bin-check on Windows.
> 
> Yeah, I was wondering if this is just exposing a pre-existing bug.
> However, the "normal" path operates by repeatedly invoking PQconnectPoll
> (cf. connectDBComplete) so it's not immediately obvious how such a bug
> would've escaped detection.
> 

I can see one difference though (I didn't see this code before) and that
is, the connectDBComplete starts with waiting for socket to become
writable and only then calls PQconnectPoll, while my patch starts with
PQconnectPoll call. And I see following comment in connectDBstart
>       /*
>        * The code for processing CONNECTION_NEEDED state is in 
> PQconnectPoll(),
>        * so that it can easily be re-executed if needed again during the
>        * asynchronous startup process.  However, we must run it once here,
>        * because callers expect a success return from this routine to mean 
> that
>        * we are in PGRES_POLLING_WRITING connection state.
>        */

So I guess I implemented it wrong in a subtle way that breaks on windows.

If that's the case, the attached should fix it, but I have no way of
testing it on windows, I can only say that it still works on my machine
so at least it hopefully does not make things worse.

-- 
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services
From 7478c898954099c79c0a34743d2292f740120009 Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmo...@pjmodos.net>
Date: Sat, 4 Mar 2017 07:43:17 +0100
Subject: [PATCH] Reorder the asynchronous libpq calls for replication
 connection

---
 .../replication/libpqwalreceiver/libpqwalreceiver.c   | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c 
b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 048d2aa..b24eb3f 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -113,7 +113,7 @@ libpqrcv_connect(const char *conninfo, bool logical, const 
char *appname,
                                 char **err)
 {
        WalReceiverConn *conn;
-       PostgresPollingStatusType status;
+       PostgresPollingStatusType status = PGRES_POLLING_WRITING;
        const char *keys[5];
        const char *vals[5];
        int                     i = 0;
@@ -155,12 +155,14 @@ libpqrcv_connect(const char *conninfo, bool logical, 
const char *appname,
                return NULL;
        }
 
-       /* Poll connection. */
-       do
+       /*
+        * Poll connection until we have OK or FAILED status.
+        *
+        * Note the initial state after PQconnectStartParams is
+        * PGRES_POLLING_WRITING.
+        */
+       while (status != PGRES_POLLING_OK && status != PGRES_POLLING_FAILED)
        {
-               /* Determine current state of the connection. */
-               status = PQconnectPoll(conn->streamConn);
-
                /* Sleep a bit if waiting for socket. */
                if (status == PGRES_POLLING_READING ||
                        status == PGRES_POLLING_WRITING)
@@ -189,8 +191,9 @@ libpqrcv_connect(const char *conninfo, bool logical, const 
char *appname,
                                CHECK_FOR_INTERRUPTS();
                }
 
-               /* Otherwise loop until we have OK or FAILED status. */
-       } while (status != PGRES_POLLING_OK && status != PGRES_POLLING_FAILED);
+               /* Determine new state of the connection. */
+               status = PQconnectPoll(conn->streamConn);
+       }
 
        if (PQstatus(conn->streamConn) != CONNECTION_OK)
        {
-- 
2.7.4

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

Reply via email to