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

On Wed, Sep 10, 2014 at 08:10:45PM -0400, Bruce Momjian wrote:
> On Tue, Jun 10, 2014 at 10:30:24AM -0400, Robert Haas wrote:
> > On Tue, Jun 10, 2014 at 10:18 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > > Robert Haas <robertmhaas@gmail.com> writes:
> > >> I don't agree with this analysis.  If the connection is closed after
> > >> the client sends a COMMIT and before it gets a response, then the
> > >> client must indeed be smart enough to figure out whether or not the
> > >> commit happened.  But if the server sends a response, the client
> > >> should be able to rely on that response being correct.  In this case,
> > >> an ERROR is getting sent but the transaction is getting committed;
> > >> yuck.  I'm not sure whether the fix is right, but this definitely
> > >> seems like a bug.
> > >
> > > In general, the only way to avoid that sort of behavior for a post-commit
> > > error would be to PANIC ... and even then, the transaction got committed,
> > > which might not be the expectation of a client that got an error message,
> > > even if it said PANIC.  So this whole area is a minefield, and the only
> > > attractive thing we can do is to try to reduce the number of errors that
> > > can get thrown post-commit.  We already, for example, do not treat
> > > post-commit file unlink failures as ERROR, though we surely would prefer
> > > to do that.
> >
> > We could treated it as a lost-communication scenario.  The appropriate
> > recovery actions from the client's point of view are identical.
> >
> > > So from this standpoint, redefining SIGINT as not throwing an error when
> > > we're in post-commit seems like a good idea.  I'm not endorsing any
> > > details of the patch here, but the 20000-foot view seems generally sound.
> >
> > Cool, that makes sense to me also.
>
> Did we ever do anything about this?

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.  :-)
(The disable_timeout disables internal timeouts, but it doesn't disable
cancels coming from the client.)

The first patch is for testing and adds a sleep(5) to the end of the
TRUNCATE command, to give the tester time to press Control-C from psql,
and enables log_duration so the cancel is checked.

The second patch is the patch that disables cancel when we are in the
process of committing;  before:

    test=> CREATE TABLE test(x INT);
    CREATE TABLE
    test=> INSERT INTO test VALUES (3);
    INSERT 0 1
    test=> TRUNCATE test;
    ^CCancel request sent
-->    ERROR:  canceling statement due to user request
    test=> SELECT * FROM test;
     x
    ---
    (0 rows)

and with both patches:

    test=> CREATE TABLE test(x INT);
    CREATE TABLE
    test=> INSERT INTO test VALUES (3);
    INSERT 0 1
    test=> TRUNCATE test;
    ^CCancel request sent
-->    TRUNCATE TABLE
    test=> SELECT * FROM test;
     x
    ---
    (0 rows)

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

  + Everyone has their own god. +