Re: dblink query interruptibility

On Wed, Nov 22, 2023 at 10:29 AM Noah Misch <noah(at)leadboat(dot)com> wrote:
>
> === Background
>
> Something as simple as the following doesn't respond to cancellation. In
> v15+, any DROP DATABASE will hang as long as it's running:
>
> SELECT dblink_exec(
> $$dbname='$$||current_database()||$$' port=$$||current_setting('port'),
> 'SELECT pg_sleep(15)');
>
> https://postgr.es/m/4B584C99.8090004@enterprisedb.com proposed a fix back in
> 2010. Latches and the libpqsrv facility have changed the server programming
> environment since that patch. The problem statement also came up here:
>
> On Thu, Dec 08, 2022 at 06:08:15PM -0800, Andres Freund wrote:
> > dblink.c uses a lot of other blocking libpq functions, which obviously also
> > isn't ok.
>
>
> === Key decisions
>
> This patch adds to libpqsrv facility.

I found that this patch was committed at d3c5f37dd5 and changed the
error message in postgres_fdw slightly. Here's an example:

#1. Begin a new transaction.
#2. Execute a query accessing to a foreign table, like SELECT * FROM
<foreign table>
#3. Terminate the *remote* session corresponding to the foreign table.
#4. Commit the transaction, and then currently the following error
message is output.

ERROR: FATAL: terminating connection due to administrator command
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
invalid socket

Previously, before commit d3c5f37dd5, the error message at #4 did not
include "invalid socket." Now, after the commit, it does. Is this
change intentional?

+ /* Consume whatever data is available from the socket */
+ if (PQconsumeInput(conn) == 0)
+ {
+ /* trouble; expect PQgetResult() to return NULL */
+ break;
+ }
+ }
+
+ /* Now we can collect and return the next PGresult */
+ return PQgetResult(conn);

This code appears to cause the change. When the remote session ends,
PQconsumeInput() returns 0 and marks conn->socket as invalid.
Subsequent PQgetResult() calls pqWait(), detecting the invalid socket
and appending "invalid socket" to the error message.

I think the "invalid socket" message is unsuitable in this scenario,
and PQgetResult() should not be called after PQconsumeInput() returns
0. Thought?

Regards,

--
Fujii Masao