Skip site navigation (1) Skip section navigation (2)

Re: [PATCH] V3: Idle in transaction cancellation

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-hackers(at)postgresql(dot)org
Cc: "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov>
Subject: Re: [PATCH] V3: Idle in transaction cancellation
Date: 2010-10-30 08:49:21
Message-ID: 201010301049.22354.andres@anarazel.de (view raw or flat)
Thread:
Lists: pgsql-hackers
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

Attachment: 0003-Enable-rethrowing-errors-which-occured-while-IDLE-IN.patch
Description: text/x-patch (10.6 KB)
Attachment: 0001-Support-transporting-flags-in-the-elevel-argument-of.patch
Description: text/x-patch (6.8 KB)
Attachment: 0002-Implement-cancellation-of-backends-in-IDLE-IN-TRANSA.patch
Description: text/x-patch (5.7 KB)

In response to

Responses

pgsql-hackers by date

Next:From: Andres FreundDate: 2010-10-30 09:05:17
Subject: Re: [RFC][PATCH]: CRC32 is limiting at COPY/CTAS/INSERT ... SELECT + speeding it up
Previous:From: Marti RaudseppDate: 2010-10-29 23:33:18
Subject: [PATCH] More Coccinelli cleanups

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group