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 |
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 |