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

On Thu, Mar 19, 2015 at 06:59:20PM -0300, Alvaro Herrera wrote:
> Robert Haas wrote:
> > On Thu, Mar 19, 2015 at 10:23 AM, Bruce Momjian <bruce@momjian.us> wrote:
>
> > > 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.
> >
> > So, either way, what happens if the query cancel shows up just an
> > instant after you clear the flag?
>
> I don't understand why aren't interrupts held until after the commit is
> done -- including across the mentioned ereports.

Uh, I think Robert was thinking of pre-commit triggers at the top of
CommitTransaction() that might take a long time and we might want to
cancel.  In fact, he is right that mid-way into CommitTransaction(),
after those pre-commit triggers, we do HOLD_INTERRUPTS(), then set our
clog bit and continue to the bottom of that function.  What is happening
is that we don't _clear_ the cancel bit and log_duration is finding the
cancel.

In an ideal world, we would clear the client cancel in
CommitTransaction() and when we do log_duration*, and the attached patch
now does that.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +