Re: [PATCH] V3: Idle in transaction cancellation

Hi,

On Tuesday 19 October 2010 16:18:29 Kevin Grittner wrote:
> Andres Freund <andres(at)anarazel(dot)de> wrote:
> > Here is a proposed patch which enables cancellation of $subject.
>
> Cool. Some enhancements we'd like to do to Serializable Snapshot
> Isolation (SSI), should the base patch make it in, would require
> this capability.
>
> > Currently it does *not* report any special error message to the
> > client if it
> >
> > starts sending commands in an (unbekownst to it) failed
> > transaction, but just the normal "25P02: current transaction is
> > aborted..." message.
> >
> > It shouldn't be hard to add that and I will propose a patch if
> > people would like it (I personally am not very interested, but I
> > can see people validly wanting it)
>
> For SSI purposes, it would be highly desirable to be able to set the
> SQLSTATE and message generated when the canceled transaction
> terminates.
Ok, I implemented that capability, but the patch feels somewhat wrong to me,
so its a separate patch on top the others:

* it intermingles logic between elog.c and postgres.c far too much - requiring
exporting variables which should not get exported. And it would still need
more to allow some sensible Assert()s. I.e. Assert(DoingCommandRead) would
need to be available in elog.c to avoid somebody using the re-throwing
capability out of place....

* You wont see an error if the next command after the IDLE in transaction is a
COMMIT/ROLLBACK. I don’t see any sensible way around that.

* the copied edata lives in TopMemoryContext and gets only reset in the next
reported error, after the re-raised error was reported.

That’s how it looks like to raise one such error right now:

error = ERROR
if (DoingCommandRead)
{
silent_error_while_idle = true;
error |= LOG_NO_CLIENT|LOG_RE_THROW_AFTER_SYNC;
}

...

ereport(error,
(errcode(ERRCODE_QUERY_CANCELED),
errmsg("canceling statement due to user request")));

One could mingle together LOG_NO_CLIENT|LOG_RE_THROW_AFTER_SYNC into a macro
and set silent_error_while_idle in elog.c, but I don’t see many callsites
coming up, so I think its better to be explicit.

Ill set this up for the next commitfest, I don't think I can do much more
without further input.

Andres