From: Alexander Lakhin <exclusion@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16678: The ecpg connect/test5 test sometimes fails on Windows
Date: Sun, 25 Oct 2020 00:00:00 +0300
Message-ID: <c550c1b0-45e2-9f71-1472-f3f22bbc702a@gmail.com> (raw)
In-Reply-To: <383222.1603561145@sss.pgh.pa.us>
References: <16678-253e48d34dc0c376@postgresql.org>
<434311.1603135136@sss.pgh.pa.us>
<f558fb39-ce9e-a516-d7c3-fc94fb581711@gmail.com>
<383222.1603561145@sss.pgh.pa.us>
24.10.2020 20:39, Tom Lane wrote:
> Alexander Lakhin <exclusion@gmail.com> writes:
>> So I've also tested on Windows the following version:
>> secure_close(MyProcPort);
>> shutdown(MyProcPort->sock, SD_SEND);
>> for(;;) {
>> char buffer[1000];
>> int res = recv(MyProcPort->sock, buffer, 1000, 0);
>> if (res <= 0)
>> break;
>> }
>> closesocket(MyProcPort->sock);
>> And it works too.
> I'm afraid this cure is probably worse than the disease, because
> now the backend's exiting is held hostage by whether the client
> closes its socket (and the resulting FIN reaches us, which it
> might not if there's a network problem).
On a next level of this game we could make something like that:
secure_close(MyProcPort);
timeout = 1000; // 1 sec
setsockopt(MyProcPort->sock, SOL_SOCKET, SO_RCVTIMEO,
(char*)&timeout, sizeof(timeout));
shutdown(MyProcPort->sock, SD_SEND);
for(;;) {
char buffer[1000];
int res = recv(MyProcPort->sock, buffer, 1000, 0);
if (res <= 0) {
break;
}
}
closesocket(MyProcPort->sock);
(I can't get this timeout working yet.)
But I'm inclined to stay on the previous level with "shutdown &&
closesocket" as recommended for server side:
https://docs.microsoft.com/en-us/windows/win32/winsock/graceful-shutdown-linger-options-and-socket-c...
Please look at the draft patch.
By the way, the delay added to pqReadData():
pg_usleep(10000L); // Wait for 10 ms.
makes the src/test/recovery test fail on Linux too (pg_ctl can't stop
the primary node).
So it seems that slow reading from a socket breaks things not only on
Windows and the fix should be more comprehensive.
Best regards,
Alexander
Attachments:
[text/x-patch] graceful_socket_shutdown.patch (1.5K, ../c550c1b0-45e2-9f71-1472-f3f22bbc702a@gmail.com/2-graceful_socket_shutdown.patch)
download | inline diff:diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.cindex ac986c0505..df681df103 100644--- a/src/backend/libpq/pqcomm.c+++ b/src/backend/libpq/pqcomm.c@@ -285,16 +285,26 @@ socket_close(int code, Datum arg)
* call this, so this is safe when interrupting BackendInitialize().
*/
secure_close(MyProcPort);
+#ifdef WIN32+ /*+ * Shutdown and close the socket explicitly to allow clients to receive+ * a message sent just before the backend exit, e.g., a FATAL error.+ * Otherwise Windows resets the connection and doesn't transmit data+ * when the process dies.+ */+ shutdown(MyProcPort->sock, SD_BOTH);+ closesocket(MyProcPort->sock);+#endif
/*
- * Formerly we did an explicit close() here, but it seems better to- * leave the socket open until the process dies. This allows clients- * to perform a "synchronous close" if they care --- wait till the- * transport layer reports connection closure, and you can be sure the- * backend has exited.+ * Formerly we did an explicit close() here on other OSes too, but it+ * seems better to leave the socket open until the process dies.+ * This allows clients to perform a "synchronous close" if they care+ * --- wait till the transport layer reports connection closure,+ * and you can be sure the backend has exited.
*
- * We do set sock to PGINVALID_SOCKET to prevent any further I/O,- * though.+ * We do set sock to PGINVALID_SOCKET to prevent any further I/O+ * in any case.
*/
MyProcPort->sock = PGINVALID_SOCKET;
}
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-bugs@postgresql.org
Cc: exclusion@gmail.com, tgl@sss.pgh.pa.us, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16678: The ecpg connect/test5 test sometimes fails on Windows
In-Reply-To: <c550c1b0-45e2-9f71-1472-f3f22bbc702a@gmail.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