public inbox for [email protected]
help / color / mirror / Atom feedFrom: Fujii Masao <[email protected]>
To: [email protected]
Subject: Issue in postgres_fdw causing unnecessary wait for cancel request reply
Date: Wed, 12 Apr 2023 03:36:01 +0900
Message-ID: <[email protected]> (raw)
Hi,
When using postgres_fdw, in the event of a local transaction being
aborted while a query is running on a remote server,
postgres_fdw sends a cancel request to the remote server.
However, if PQgetCancel() returned NULL and no cancel request was issued,
I found that postgres_fdw could still wait for the reply to
the cancel request, causing unnecessary wait time with a 30 second timeout.
For example, the following queries can reproduce the issue:
----------------------------
create extension postgres_fdw;
create server loopback foreign data wrapper postgres_fdw options (tcp_user_timeout 'a');
create user mapping for public server loopback;
create view t as select 1 as i from pg_sleep(100);
create foreign table ft (i int) server loopback options (table_name 't');
select * from ft;
Press Ctrl-C while running the above SELECT query.
----------------------------
Attached patch fixes this issue. It ensures that postgres_fdw only waits
for a reply if a cancel request is actually issued. Additionally,
it improves PQgetCancel() to set error messages in certain error cases,
such as when out of memory occurs and malloc() fails. Moreover,
it enhances postgres_fdw to report a warning message when PQgetCancel()
returns NULL, explaining the reason for the NULL value.
Thought?
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From 26293ade92ce6fa0bbafac4a95f634e485f4aab6 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 12 Apr 2023 02:17:56 +0900
Subject: [PATCH v1] postgres_fdw: Fix bug causing unnecessary wait for cancel
request reply.
In the event of a local transaction being aborted while a query is
running on a remote server in postgres_fdw, the extension sends
a cancel request to the remote server. However, if PQgetCancel()
returned NULL and no cancel request was issued, postgres_fdw
could still wait for the reply to the cancel request, causing unnecessary
wait time with a 30 second timeout.
To fix this issue, this commit ensures that postgres_fdw only waits for
a reply if a cancel request was actually issued. Additionally,
this commit improves PQgetCancel() to set error messages in certain
error cases, such as when out of memory happens and malloc() fails.
Moreover, this commit enhances postgres_fdw to report a warning
message when PQgetCancel() returns NULL, explaining the reason for
the NULL value.
---
contrib/postgres_fdw/connection.c | 8 ++++++++
src/interfaces/libpq/fe-connect.c | 6 ++++++
2 files changed, 14 insertions(+)
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 2969351e9a..e3c79158d8 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -1344,6 +1344,14 @@ pgfdw_cancel_query_begin(PGconn *conn)
}
PQfreeCancel(cancel);
}
+ else
+ {
+ ereport(WARNING,
+ (errcode(ERRCODE_CONNECTION_FAILURE),
+ errmsg("could not send cancel request: %s",
+ pchomp(PQerrorMessage(conn)))));
+ return false;
+ }
return true;
}
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 40fef0e2c8..eeb4d9a745 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -4703,11 +4703,17 @@ PQgetCancel(PGconn *conn)
return NULL;
if (conn->sock == PGINVALID_SOCKET)
+ {
+ libpq_append_conn_error(conn, "connection is not open");
return NULL;
+ }
cancel = malloc(sizeof(PGcancel));
if (cancel == NULL)
+ {
+ libpq_append_conn_error(conn, "out of memory");
return NULL;
+ }
memcpy(&cancel->raddr, &conn->raddr, sizeof(SockAddr));
cancel->be_pid = conn->be_pid;
--
2.40.0
Attachments:
[text/plain] v1-0001-postgres_fdw-Fix-bug-causing-unnecessary-wait-for.patch (2.2K, ../[email protected]/2-v1-0001-postgres_fdw-Fix-bug-causing-unnecessary-wait-for.patch)
download | inline diff:
From 26293ade92ce6fa0bbafac4a95f634e485f4aab6 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 12 Apr 2023 02:17:56 +0900
Subject: [PATCH v1] postgres_fdw: Fix bug causing unnecessary wait for cancel
request reply.
In the event of a local transaction being aborted while a query is
running on a remote server in postgres_fdw, the extension sends
a cancel request to the remote server. However, if PQgetCancel()
returned NULL and no cancel request was issued, postgres_fdw
could still wait for the reply to the cancel request, causing unnecessary
wait time with a 30 second timeout.
To fix this issue, this commit ensures that postgres_fdw only waits for
a reply if a cancel request was actually issued. Additionally,
this commit improves PQgetCancel() to set error messages in certain
error cases, such as when out of memory happens and malloc() fails.
Moreover, this commit enhances postgres_fdw to report a warning
message when PQgetCancel() returns NULL, explaining the reason for
the NULL value.
---
contrib/postgres_fdw/connection.c | 8 ++++++++
src/interfaces/libpq/fe-connect.c | 6 ++++++
2 files changed, 14 insertions(+)
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 2969351e9a..e3c79158d8 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -1344,6 +1344,14 @@ pgfdw_cancel_query_begin(PGconn *conn)
}
PQfreeCancel(cancel);
}
+ else
+ {
+ ereport(WARNING,
+ (errcode(ERRCODE_CONNECTION_FAILURE),
+ errmsg("could not send cancel request: %s",
+ pchomp(PQerrorMessage(conn)))));
+ return false;
+ }
return true;
}
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 40fef0e2c8..eeb4d9a745 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -4703,11 +4703,17 @@ PQgetCancel(PGconn *conn)
return NULL;
if (conn->sock == PGINVALID_SOCKET)
+ {
+ libpq_append_conn_error(conn, "connection is not open");
return NULL;
+ }
cancel = malloc(sizeof(PGcancel));
if (cancel == NULL)
+ {
+ libpq_append_conn_error(conn, "out of memory");
return NULL;
+ }
memcpy(&cancel->raddr, &conn->raddr, sizeof(SockAddr));
cancel->be_pid = conn->be_pid;
--
2.40.0
view thread (2+ messages)
reply
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: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox