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

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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 14:23:54
Message-ID: 20150319142354.GB6061@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Mar 19, 2015 at 07:54:02AM -0400, Robert Haas wrote:
> On Wed, Mar 18, 2015 at 10:56 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > I have researched this issue originally reported in June of 2014 and
> > implemented a patch to ignore cancel while we are completing a commit.
> > I am not clear if this is the proper place for this code, though a
> > disable_timeout() call on the line above suggests I am close. :-)
>
> This would also disable cancel interrupts while running AFTER
> triggers, which seems almost certain to be wrong. TBH, I'm not sure
> why the existing HOLD_INTERRUPTS() in CommitTransaction() isn't
> already preventing this problem. Did you investigate that at all?

Yes, the situation is complex, and was suggested by the original poster.
The issue with CommitTransaction() is that it only _holds_ the signal
--- it doesn't clear it. Now, since there are very few
CHECK_FOR_INTERRUPTS() calls in the typical commit process flow, the
signal is normally erased. However, if log_duration or
log_min_duration_statement are set, they call ereport, which calls
errfinish(), which has a call to CHECK_FOR_INTERRUPTS().

First attached patch is more surgical and clears a possible cancel
request before we report the query duration in the logs --- this doesn't
affect any after triggers that might include CHECK_FOR_INTERRUPTS()
calls we want to honor.

Another approach would be to have CommitTransaction() clear any pending
cancel before it calls RESUME_INTERRUPTS(). The second attached patch
takes that approach, and also works.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Attachment Content-Type Size
cancel2.diff text/x-diff 583 bytes
cancel3.diff text/x-diff 501 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tatsuo Ishii 2015-03-19 14:31:21 Re: ERRCODE_T_R_DEADLOCK_DETECTED
Previous Message Thom Brown 2015-03-19 14:20:36 Re: GSoC 2015: Extra Jsonb functionality