From: Petr Jelinek <petr.jelinek@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: pgsql-hackers@postgreSQL.org, Andrew Dunstan <andrew@dunslane.net>
Subject: Re: [COMMITTERS] pgsql: Use asynchronous connect API in libpqwalreceiver
Date: Sat, 4 Mar 2017 07:45:58 +0100
Message-ID: <4144256b-dc4f-892c-a720-71f6e7b9ac87@2ndquadrant.com> (raw)
In-Reply-To: <19815.1488600693@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>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
On 04/03/17 05:11, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
>> On 3/3/17 19:16, Tom Lane wrote:
>>> Peter Eisentraut <peter_e@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 <pjmodos@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.cindex 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-hackersAttachments:
[text/plain] 0001-Reorder-the-asynchronous-libpq-calls-for-replication.patch (1.9K, ../4144256b-dc4f-892c-a720-71f6e7b9ac87@2ndquadrant.com/2-0001-Reorder-the-asynchronous-libpq-calls-for-replication.patch)
download | inline diff:
From 7478c898954099c79c0a34743d2292f740120009 Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@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.cindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: petr.jelinek@2ndquadrant.com, tgl@sss.pgh.pa.us, peter.eisentraut@2ndquadrant.com, andrew@dunslane.net
Subject: Re: [COMMITTERS] pgsql: Use asynchronous connect API in libpqwalreceiver
In-Reply-To: <4144256b-dc4f-892c-a720-71f6e7b9ac87@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox