statement_timeout vs DECLARE CURSOR
Hi,
We've encountered some unexpected behavior with statement_timeout not cancelling a query in DECLARE CURSOR, but only if the DECLARE CURSOR is outside of a transaction:
xof=# select version();
version
-------------------------------------------------------------------------------------------------------------------
PostgreSQL 13.4 on x86_64-apple-darwin19.6.0, compiled by Apple clang version 12.0.0 (clang-1200.0.32.29), 64-bit
(1 row)
xof=# set statement_timeout = '1s';
SET
xof=# \timing
Timing is on.
xof=# select * from (with test as (select pg_sleep(10), current_timestamp as cur_time) select 1 from test ) as slp;
ERROR: canceling statement due to statement timeout
Time: 1000.506 ms (00:01.001)
xof=# declare x no scroll cursor with hold for select * from (with test as (select pg_sleep(10), current_timestamp as cur_time) select 1 from test ) as slp;
DECLARE CURSOR
Time: 10001.929 ms (00:10.002)
xof=#
but:
xof=# set statement_timeout = '1s';
SET
xof=# \timing
Timing is on.
xof=# begin;
BEGIN
Time: 0.161 ms
xof=*# declare x no scroll cursor with hold for select * from (with test as (select pg_sleep(10), current_timestamp as cur_time) select 1 from test ) as slp;
DECLARE CURSOR
Time: 0.949 ms
xof=*# fetch all from x;
ERROR: canceling statement due to statement timeout
Time: 1000.520 ms (00:01.001)
xof=!# abort;
ROLLBACK
Time: 0.205 ms
xof=#
On Sep 27, 2021, at 10:42, Christophe Pettus <xof@thebuild.com> wrote:
We've encountered some unexpected behavior with statement_timeout not cancelling a query in DECLARE CURSOR, but only if the DECLARE CURSOR is outside of a transaction:
A bit more poking revealed the reason: The ON HOLD cursor's query is executed at commit time (which is, logically, not interruptible), but that's all wrapped in the single statement outside of a transaction.
Christophe Pettus <xof@thebuild.com> writes:
On Sep 27, 2021, at 10:42, Christophe Pettus <xof@thebuild.com> wrote:
We've encountered some unexpected behavior with statement_timeout not cancelling a query in DECLARE CURSOR, but only if the DECLARE CURSOR is outside of a transaction:
A bit more poking revealed the reason: The ON HOLD cursor's query is executed at commit time (which is, logically, not interruptible), but that's all wrapped in the single statement outside of a transaction.
Hmm ... seems like a bit of a UX failure. I wonder why we don't persist
such cursors before we get into the uninterruptible part of COMMIT.
regards, tom lane
I wrote:
Christophe Pettus <xof@thebuild.com> writes:
A bit more poking revealed the reason: The ON HOLD cursor's query is executed at commit time (which is, logically, not interruptible), but that's all wrapped in the single statement outside of a transaction.
Hmm ... seems like a bit of a UX failure. I wonder why we don't persist
such cursors before we get into the uninterruptible part of COMMIT.
Oh, I see the issue. It's not that that part of COMMIT isn't
interruptible; you can control-C out of it just fine. The problem
is that finish_xact_command() disarms the statement timeout before
starting CommitTransactionCommand at all.
We could imagine pushing the responsibility for that down into
xact.c, allowing it to happen after CommitTransaction has finished
running user-defined code. But it seems like a bit of a mess
because there are so many other code paths there. Not sure how
to avoid future bugs-of-omission.
regards, tom lane
[ redirect to -hackers ]
I wrote:
Christophe Pettus <xof@thebuild.com> writes:
A bit more poking revealed the reason: The ON HOLD cursor's query is executed at commit time (which is, logically, not interruptible), but that's all wrapped in the single statement outside of a transaction.
Hmm ... seems like a bit of a UX failure. I wonder why we don't persist
such cursors before we get into the uninterruptible part of COMMIT.
Oh, I see the issue. It's not that that part of COMMIT isn't
interruptible; you can control-C out of it just fine. The problem
is that finish_xact_command() disarms the statement timeout before
starting CommitTransactionCommand at all.
We could imagine pushing the responsibility for that down into
xact.c, allowing it to happen after CommitTransaction has finished
running user-defined code. But it seems like a bit of a mess
because there are so many other code paths there. Not sure how
to avoid future bugs-of-omission.
Actually ... maybe it needn't be any harder than the attached?
This makes it possible for a statement timeout interrupt to occur
anytime during CommitTransactionCommand, but I think
CommitTransactionCommand had better be able to protect itself
against that anyway, for a couple of reasons:
1. It's not significantly different from a query-cancel interrupt,
which surely could arrive during that window.
2. COMMIT-within-procedures already exposes us to statement timeout
during COMMIT.
regards, tom lane
Attachments:
include-commit-processing-in-statement-timeout.patchtext/x-diff; charset=us-ascii; name=include-commit-processing-in-statement-timeout.patchDownload
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 0775abe35d..9ec96f2453 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2713,9 +2713,6 @@ start_xact_command(void)
static void
finish_xact_command(void)
{
- /* cancel active statement timeout after each command */
- disable_statement_timeout();
-
if (xact_started)
{
CommitTransactionCommand();
@@ -2733,6 +2730,9 @@ finish_xact_command(void)
xact_started = false;
}
+
+ /* cancel active statement timeout after each command */
+ disable_statement_timeout();
}