[ redirect to -hackers ] I wrote: >> Christophe Pettus <x...@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
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(); }