Re: "cancelling statement due to user request error" occurs but the transaction has committed.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Naoya Anzai <anzai-naoya(at)mxu(dot)nes(dot)nec(dot)co(dot)jp>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Akio Iwaasa <iwaasa(at)mxs(dot)nes(dot)nec(dot)co(dot)jp>
Subject: Re: "cancelling statement due to user request error" occurs but the transaction has committed.
Date: 2015-03-19 23:19:02
Message-ID: 13406.1426807142@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> On Thu, Mar 19, 2015 at 04:36:38PM -0400, Robert Haas wrote:
>> So, either way, what happens if the query cancel shows up just an
>> instant after you clear the flag?

> Oh, good point. This version handles that case addressing only the
> log_duration* block.

This is just moving the failure cases around, and not by very much either.

The core issue here, I think, is that xact.c only holds off interrupts
during what it considers to be the commit critical section --- which is
okay from the standpoint of transactional consistency. But the complaint
has to do with what the client perceives to have happened if a SIGINT
arrives somewhere between where xact.c has committed and where postgres.c
has reported the commit to the client. If we want to address that, I
think postgres.c needs to hold off interrupts for the entire duration from
just before CommitTransactionCommand() to just after ReadyForQuery().
That's likely to be rather messy, because there are so many code paths
there, especially when you consider error cases.

A possible way to do this without incurring unacceptably high risk of
breakage (in particular, ending up with interrupts still held off when
they shouldn't be any longer) is to assume that there should never be a
case where we reach ReadCommand() with interrupts still held off. Then
we could invent an additional interrupt primitive "RESET_INTERRUPTS()"
that forces InterruptHoldoffCount to zero (and, presumably, then does
a CHECK_FOR_INTERRUPTS()); then putting a HOLD_INTERRUPTS() before calling
CommitTransactionCommand() and a RESET_INTERRUPTS() before waiting for
client input would presumably be pretty safe. On the other hand, that
approach could easily mask interrupt holdoff mismatch bugs elsewhere in
the code base.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2015-03-19 23:27:16 Re: [PATCH] two-arg current_setting() with fallback
Previous Message David Christensen 2015-03-19 23:16:08 [PATCH] two-arg current_setting() with fallback